Skip to content
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

Closed
terryjreedy opened this issue Jun 24, 2016 · 17 comments
Closed

IDLE: add base Query dialog with ttk widgets #71567

terryjreedy opened this issue Jun 24, 2016 · 17 comments
Assignees
Labels
topic-IDLE type-feature A feature request or enhancement

Comments

@terryjreedy
Copy link
Member

BPO 27380
Nosy @terryjreedy, @ned-deily, @roseman, @serhiy-storchaka
Files
  • query.diff
  • query2.diff
  • query3.diff
  • query4.diff
  • query-helpsource.diff
  • query-helpsource2.diff
  • 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:

    assignee = 'https://github.com/terryjreedy'
    closed_at = <Date 2016-08-10.19:39:59.202>
    created_at = <Date 2016-06-24.04:15:17.216>
    labels = ['expert-IDLE', 'type-feature']
    title = 'IDLE: add base Query dialog with ttk widgets'
    updated_at = <Date 2019-03-21.18:17:20.620>
    user = 'https://github.com/terryjreedy'

    bugs.python.org fields:

    activity = <Date 2019-03-21.18:17:20.620>
    actor = 'terry.reedy'
    assignee = 'terry.reedy'
    closed = True
    closed_date = <Date 2016-08-10.19:39:59.202>
    closer = 'ned.deily'
    components = ['IDLE']
    creation = <Date 2016-06-24.04:15:17.216>
    creator = 'terry.reedy'
    dependencies = []
    files = ['43526', '43554', '43570', '43618', '43651', '43658']
    hgrepos = []
    issue_num = 27380
    keywords = ['patch']
    message_count = 17.0
    messages = ['269154', '269337', '269338', '269347', '269411', '269415', '269774', '269777', '269920', '269967', '269972', '269973', '272198', '272216', '272345', '272348', '272367']
    nosy_count = 5.0
    nosy_names = ['terry.reedy', 'ned.deily', 'markroseman', 'python-dev', 'serhiy.storchaka']
    pr_nums = []
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue27380'
    versions = ['Python 3.6']

    @terryjreedy
    Copy link
    Member Author

    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.

    @terryjreedy terryjreedy self-assigned this Jun 24, 2016
    @terryjreedy terryjreedy added the type-feature A feature request or enhancement label Jun 24, 2016
    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Jun 27, 2016

    New changeset 4796d7fb00c5 by Terry Jan Reedy in branch 'default':
    Issue bpo-27380: IDLE: add base Query dialog, with ttk widgets and subclass
    https://hg.python.org/cpython/rev/4796d7fb00c5

    @terryjreedy
    Copy link
    Member Author

    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.

    @serhiy-storchaka
    Copy link
    Member

    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.

    @terryjreedy
    Copy link
    Member Author

    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?

    @terryjreedy
    Copy link
    Member Author

    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.

    @terryjreedy
    Copy link
    Member Author

    Added tests for ModuleName. Will recheck and push.

    @terryjreedy
    Copy link
    Member Author

    (Misdirected notice copied here)
    New changeset 78a3d3700233 by Terry Jan Reedy in branch 'default':
    bpo-27437: Add query.ModuleName and use it for file => Load Module.
    https://hg.python.org/cpython/rev/78a3d3700233

    @terryjreedy
    Copy link
    Member Author

    Next patch should finish issue except for a couple of minor questions.

    @terryjreedy
    Copy link
    Member Author

    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.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Jul 8, 2016

    New changeset 66fe8d9eae6c by Terry Jan Reedy in branch 'default':
    Issue bpo-27380: IDLE: add query.HelpSource class and tests.
    https://hg.python.org/cpython/rev/66fe8d9eae6c

    @terryjreedy
    Copy link
    Member Author

    Patch posted had code that belongs to bpo-27465 and was deleted before pushing.

    @ned-deily
    Copy link
    Member

    test_click_help_source fails on OS X:

    ======================================================================
    FAIL: test_click_help_source (idlelib.idle_test.test_query.HelpsourceGuiTest)
    ----------------------------------------------------------------------

    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:
    'file:///Library/Frameworks/Python.framewo[57 chars]y.py'
    '/Library/Frameworks/Python.framework/Vers[50 chars]y.py'

    ('__test__',

    • 'file:///Library/Frameworks/Python.framework/Versions/3.6/lib/python3.6/idlelib/idle_test/test_query.py')
      ? -------

    + '/Library/Frameworks/Python.framework/Versions/3.6/lib/python3.6/idlelib/idle_test/test_query.py')

    @ned-deily ned-deily reopened this Aug 8, 2016
    @terryjreedy
    Copy link
    Member Author

    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,
    is the comment right about needing to add "file://" for Safari?
    is Safari still the default and/or correct way to open a local file?

    The code that opens the supplementary help file or url is elsewhere, but it could be changed if outdated on the Mac.

    @python-dev
    Copy link
    Mannequin

    python-dev mannequin commented Aug 10, 2016

    New changeset 8f37d772f71f by Terry Jan Reedy in branch 'default':
    Issue bpo-27380: For test_query on Mac, adjust one expected result.
    https://hg.python.org/cpython/rev/8f37d772f71f

    @terryjreedy
    Copy link
    Member Author

    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.

    @ned-deily
    Copy link
    Member

    With 8f37d772f71f, the test no longer fails. Thanks!

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    topic-IDLE type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants