New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
IDLE: add base Query dialog with ttk widgets #71567
Comments
The custom popup entry dialogs in config_sec and config_help have a common structure and overlapping code. When one hits OK and the entry is not valid, both display a specific message and let the user edit the entry (or Cancel). File => Load Module, on the other hand, uses tkSimpleDialog.askstring. No error checking is done before the box disappears. If there is an error, even a simple misspelling, one must start over. It is really annoying to use. I want to replace this with a custom popup that would be the same as the one in config_sec, except for title, prompt, and validation function. If the name given has multiple conponents, such as idlelib.idelhelp.help_xyz, I would like to report which is the first invalid component (going from left to right). I am therefore adding a new module, query.py, with a new Query class that will be the baseclass for three derived classes. The partial patch renames config_sec.py (formerly configSectionNameDialog.py) to query.py. The corresponding test_config_sec) becomes test_query. It splits class GetCfgSectionNameDialog into a base class Query and a subclass SectionName. Query is responsible for the popup, and used ttk widgets as appropriate. SectionName overrides the entry validation function entry_ok (formerly name_ok). The test functions have been similarly split. What remains are ModuleName and HelpSource subclasses, corresponding tests, and integration with the software that would use them. |
New changeset 4796d7fb00c5 by Terry Jan Reedy in branch 'default': |
Pushed with minor changes. I decided not to wait until I added more subclasses because file renames with post rename changes do not survive shelving. Reloading from the diff requires 'rediscovering' the renames. |
You can use True and False instead of TRUE and FALSE. You can import constants with "from tkinter.constants import *". The benefit of using constants instead of string literals is early failing in case of typo. |
Thanks, I will switch to True and False. I am aware of the possibility of separately importing the constants, but like some others, I prefer the strings. Compile-time checking is a good point, especially with no run tests. In this case, test_query has 100% coverage of query.py. Minor question: While trying to use the new widget (instead of tkSimpledialog) in (editor.EditorWindow).open_module I had problems which I am trying to pin down by taking smaller steps. As part of this, I tried this minimal code that uses Query. from idlelib import query
from tkinter import Tk
root = Tk()
name = query.Query(root, "Open Module",
"Enter the name of a Python module\n"
"to search on sys.path and open:",).result
print(name) It works fine except that entry.focus_set() is not working for me with this call (or else it is not staying set) even though it works fine when configdialog calls the SectionName subclass. I know Windows is more troublesome than *nix about focus, but do you have any idea why calling Query this way, instead of as in the committed patch, would make a difference? |
Revised load_module and new query.ModuleName work, including when invoked by open_class_browser. Tests are need for both. One problem was that using withdrawn root rather than a text as parent caused query.Query to not appear, so wait_window could not be terminated. |
Added tests for ModuleName. Will recheck and push. |
(Misdirected notice copied here) |
Next patch should finish issue except for a couple of minor questions. |
Serhiy, thank you for the review. I intend to follow PEP-8 as best as possible in the code I touch. I also fixed some of the comments. I did a bit of refactoring that make it trivial to check for reuse of help source names. See new issue bpo-27465. All that is needed is for configdialog to pass a used_names argument. I already changed the HelpSource htest to do so. I have tested the configdialog changes by running it with live IDLE. I intend to push after a final check. |
New changeset 66fe8d9eae6c by Terry Jan Reedy in branch 'default': |
Patch posted had code that belongs to bpo-27465 and was deleted before pushing. |
test_click_help_source fails on OS X: ====================================================================== Traceback (most recent call last):
File "/Library/Frameworks/Python.framework/Versions/3.6/lib/python3.6/idlelib/idle_test/test_query.py", line 389, in test_click_help_source
Equal(dialog.result, ('__test__', __file__))
AssertionError: Tuples differ: ('__test__', 'file:///Library/Frameworks/Python.framewo[58 chars].py') != ('__test__', '/Library/Frameworks/Python.framework/Vers[51 chars].py') First differing element 1: ('__test__',
+ '/Library/Frameworks/Python.framework/Versions/3.6/lib/python3.6/idlelib/idle_test/test_query.py') |
The Mac failure comes from this piece of code: if sys.platform == 'darwin':
path = self.result[1]
if path.startswith(('www', 'file:', 'http:', https:')):
pass
else:
# Mac Safari insists on using the URI form for local files
self.result = list(self.result)
self.result[1] = "file://" + path Before I do anything, The code that opens the supplementary help file or url is elsewhere, but it could be changed if outdated on the Mac. |
New changeset 8f37d772f71f by Terry Jan Reedy in branch 'default': |
While still curious if the Mac specific code is correct, I changed test to match the code. Someone please check by running at least test_idle after pulling. |
With 8f37d772f71f, the test no longer fails. Thanks! |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: