classification
Title: IDLE: add base Query dialog with ttk widgets
Type: enhancement Stage: resolved
Components: IDLE Versions: Python 3.6
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: terry.reedy Nosy List: markroseman, ned.deily, python-dev, serhiy.storchaka, terry.reedy
Priority: normal Keywords: patch

Created on 2016-06-24 04:15 by terry.reedy, last changed 2019-03-21 18:17 by terry.reedy. This issue is now closed.

Files
File name Uploaded Description Edit
query.diff terry.reedy, 2016-06-24 04:15 review
query2.diff terry.reedy, 2016-06-27 02:08 review
query3.diff terry.reedy, 2016-06-28 02:33 review
query4.diff terry.reedy, 2016-07-03 22:21 review
query-helpsource.diff terry.reedy, 2016-07-07 03:44 review
query-helpsource2.diff terry.reedy, 2016-07-08 02:37 review
Messages (17)
msg269154 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2016-06-24 04:15
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.
msg269337 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-06-27 02:05
New changeset 4796d7fb00c5 by Terry Jan Reedy in branch 'default':
Issue #27380: IDLE: add base Query dialog, with ttk widgets and subclass
https://hg.python.org/cpython/rev/4796d7fb00c5
msg269338 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2016-06-27 02:08
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.
msg269347 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-06-27 04:57
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.
msg269411 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2016-06-28 00:19
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?
msg269415 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2016-06-28 02:33
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.
msg269774 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2016-07-03 22:21
Added tests for ModuleName.  Will recheck and push.
msg269777 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2016-07-04 03:32
(Misdirected notice copied here)
New changeset 78a3d3700233 by Terry Jan Reedy in branch 'default':
Issue 27437: Add query.ModuleName and use it for file => Load Module.
https://hg.python.org/cpython/rev/78a3d3700233
msg269920 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2016-07-07 03:44
Next patch should finish issue except for a couple of minor questions.
msg269967 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2016-07-08 02:37
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 #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.
msg269972 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-07-08 04:26
New changeset 66fe8d9eae6c by Terry Jan Reedy in branch 'default':
Issue #27380: IDLE: add query.HelpSource class and tests.
https://hg.python.org/cpython/rev/66fe8d9eae6c
msg269973 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2016-07-08 04:32
Patch posted had code that belongs to #27465 and was deleted before pushing.
msg272198 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2016-08-08 22:01
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')
msg272216 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2016-08-09 06:00
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.
msg272345 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2016-08-10 17:16
New changeset 8f37d772f71f by Terry Jan Reedy in branch 'default':
Issue #27380: For test_query on Mac, adjust one expected result.
https://hg.python.org/cpython/rev/8f37d772f71f
msg272348 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2016-08-10 17:30
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.
msg272367 - (view) Author: Ned Deily (ned.deily) * (Python committer) Date: 2016-08-10 19:39
With 8f37d772f71f, the test no longer fails.  Thanks!
History
Date User Action Args
2019-03-21 18:17:20terry.reedysetcomponents: + IDLE
2016-08-10 19:39:59ned.deilysetstatus: open -> closed
resolution: fixed
messages: + msg272367

stage: needs patch -> resolved
2016-08-10 17:30:26terry.reedysetmessages: + msg272348
2016-08-10 17:16:43python-devsetmessages: + msg272345
2016-08-09 07:18:59terry.reedysetnosy: + markroseman
2016-08-09 06:00:03terry.reedysetmessages: + msg272216
2016-08-08 22:01:27ned.deilysetstatus: closed -> open

nosy: + ned.deily
messages: + msg272198

resolution: fixed -> (no value)
stage: resolved -> needs patch
2016-07-08 04:32:00terry.reedysetstatus: open -> closed
resolution: fixed
messages: + msg269973

stage: commit review -> resolved
2016-07-08 04:26:35python-devsetmessages: + msg269972
2016-07-08 02:37:12terry.reedysetfiles: + query-helpsource2.diff

messages: + msg269967
2016-07-08 02:10:03terry.reedylinkissue27465 dependencies
2016-07-07 03:44:27terry.reedysetfiles: + query-helpsource.diff

messages: + msg269920
stage: test needed -> commit review
2016-07-04 03:32:51terry.reedysetmessages: + msg269777
2016-07-03 22:21:35terry.reedysetfiles: + query4.diff

messages: + msg269774
2016-06-28 02:33:18terry.reedysetfiles: + query3.diff

messages: + msg269415
stage: needs patch -> test needed
2016-06-28 00:19:35terry.reedysetmessages: + msg269411
stage: patch review -> needs patch
2016-06-27 04:57:28serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg269347
2016-06-27 02:08:06terry.reedysetfiles: + query2.diff

messages: + msg269338
2016-06-27 02:05:26python-devsetnosy: + python-dev
messages: + msg269337
2016-06-24 04:15:17terry.reedycreate