Terry J. Reedy added the comment: Your ping got me to look at the doc for tk_optionMenu, which is a function rather than a widget (http://www.tcl.tk/man/tcl8.6/TkCmd/optionMenu.htm), the code for tkinter.OptionMenu (the only accurate doc I know of), which is a widget with the function API, the code for DynOptionMenu(OptionMenu), which is both too general-purpose and too special-cased, the doc (and a bit of code) for ttk.Combobox, and finally your patch. Conclusion: I really want to replace DynOptionMenu with Combobox, but not with the current patch.
Micro-issue: it is too late to add anything to tkinter. In any case, it should be a separate issue, with possible a check for anything else that is missing. IDLE can easily define the constant if needed, though I prefer literal strings myself. Real issue: no tests. Tests for a new class, if any, can await finalization of the API. However, I want to have automated tests for configdialog before making substantive changes. (Justin, what OS are you working on?) A plus for the patch is that aside from inessential name changes, it is nearly a drop-in replacement. I could *almost* be persuaded that it would be safe to just do it. However, this is a result of perpetuating what I consider to be an awful API. I will come back to this, but on to the methods. Enable/disable: I am not a fan of trivial half-line functions. "Cb.enable()" would be the same as "Cb['state'] = 'readonly', Beside this, the cb for font size should perhaps eventually be 'normal' so users can type in a font size. The current patch will not apply cleanly because I since augmented the list with sizes needed for code projection on classroom screens. That itself was a substitute for entry or other adjustmen method. On_select: I believe "self.variable.set(self.get())" should be redundant, so that '<<ComboboxSelected>>' can be bound directly to command when there is one (currently only one case, but I might want to change this. Reset: passing a display string should not be options, and IDLE always passes one. 'self.variable.set(selection)' should be redundant. This leaves two lines to set the entry and the option list. For one or two boxes, a wrapper function would not be worthwhile. But for multiple instances, it might be, whether as helper function or wrapper class method. .__init__: IDLE never passes anything for '*values' and always passes None for 'value'. This ends up as the default '' for the current value. Both parameters and the corresponding lines can go. So can 'self.enable': the initial state should be added to kwargs. If 'variable' is needed, it should be passed in the normal widget way: 'textvariable=mystringvar'. 'self.set(variable.get())' is redundant because 'variable is initially '' and so it the entry. But are textvariables needed? Not for setting and getting Combobox entry values. IDLE uses them to trace changes to the current selection. But with Combobox, the change functions could be bound to '<<ComboboxSelected>>' instead of the Var tracer. I think I would prefer this, as the tracer bindings have to be explicitly removed separately from .destroy(). Do I want this binding combined with initialization? Currently the change handler bindings are done in one function. There is a lot of duplication, and I am working to factor some of it out. (But I need tests before applying.) So I may want to leave them together, which means no wrapper class. Testing a dialog requires two things: simulating user actions and capturing the result. The first depends on the widget, so it makes sense to focus on testing one widget class (like Combobox) rather than, say, one pane with mixed widget types. The second requires a mock that captures the changes sent to the config database when [Ok] or [Apply] are invoked. ---------- resolution: duplicate -> stage: resolved -> test needed versions: +Python 3.7 _______________________________________ Python tracker <rep...@bugs.python.org> <http://bugs.python.org/issue27755> _______________________________________ _______________________________________________ Python-bugs-list mailing list Unsubscribe: https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com
