[LinuxCNC/linuxcnc PR#534] Add toggle to display/hide pyVCP panel

#1 – jepler 于 2018-11-26

Thanks for your pull request!

I think this is a fine idea in general, but I am concerned about users who do not have VCP panels in their AXIS configurations.

I didn’t actually test, but my guess is that with this patch, for users without VCP panels:

– [ ] The new menu item will be shown, but it would be better not to show it
– [ ] Similar for the new item in the keyboard quick reference
– [ ] Actually choosing the menu item or pressing the shortcut is likely to cause an exception, as the global variable vcp_frame is never defined.

Can you test a configuration without VCP? If pressing ctrl-e causes an exception to be shown to the user, it definitely needs to be re-worked to fix this part at least. Ideally, the other two items would also be addressed as well.

#2 – chaotix- 于 2018-11-26

You are right about two and a half points. Only pressing the keyboard shortcut will not generate an exception because ctrl-e will not be bound if there is no panel.

I added the menu option as an afterthought and did not think about vcp_frame not being defined anymore when using the menu, so yes it will throw an exception. That will be easy to fix.

I’m not really familiar with Tkintern, but I will see if I can find a fix for not showing the menu item and the shortcut in the quick reference.

#3 – jepler 于 2018-11-26

Thanks, I appreciate you taking the time to look into it and improve this pull request.

#4 – jepler 于 2018-11-26

In axis.py you can see how items are sometimes deleted from menus, look for the widgets.menuview.delete calls. However, this approach is becoming a maintenance problem as the effect of a call like ...delete(0,5) conditional on if lathebacktool...else means that the numeric value for deleting the vcp menu item can’t be chosen as a simple hard-coded number ☹️

#5 – chaotix- 于 2018-11-26

Thanks for your help.

Yeah. I figured out how to delete an entry with a fixed position, but I have not found a method yet to delete a entry based on the label or what the label should be like.

According to https://stackoverflow.com/questions/21906815/tkinter-menu-delete-item deleting by label should be possible, but using “Show pyVCP pan_el” does not work. Neither does adding a “-label foo” to axis.tcl and then calling .delete(“foo”).

But I have figured out how to add a shortcut to the quick reference dynamically. Although it will always be at the last position in the right column as I just append it to help2.

#6 – chaotix- 于 2018-11-26

OK. I got it. I have to drop the “_”, so it just has to be “Show pyVCP panel”.

Should I use a hack like ("Show pyVCP panel").replace("_", "") so it just has to be translated once?

#7 – jepler 于 2018-11-26

@chaotix- that seems fine, each translation should also have exactly one “_” in it to note the position of the keyboard accelerator.

#8 – jepler 于 2018-11-26

(there’s something more complicated you could have done involving going down to the Tcl level and calling [extract_text] to ensure you use the same logic that parses the accelerator, but that’s strictly unneeded here and would have complicated the patch)