msg269154 - (view) |
Author: Terry J. Reedy (terry.reedy) * |
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) |
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) * |
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) * |
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) * |
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) * |
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) * |
Date: 2016-07-03 22:21 |
Added tests for ModuleName. Will recheck and push.
|
msg269777 - (view) |
Author: Terry J. Reedy (terry.reedy) * |
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) * |
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) * |
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) |
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) * |
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) * |
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) * |
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) |
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) * |
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) * |
Date: 2016-08-10 19:39 |
With 8f37d772f71f, the test no longer fails. Thanks!
|
|
Date |
User |
Action |
Args |
2022-04-11 14:58:32 | admin | set | github: 71567 |
2019-03-21 18:17:20 | terry.reedy | set | components:
+ IDLE |
2016-08-10 19:39:59 | ned.deily | set | status: open -> closed resolution: fixed messages:
+ msg272367
stage: needs patch -> resolved |
2016-08-10 17:30:26 | terry.reedy | set | messages:
+ msg272348 |
2016-08-10 17:16:43 | python-dev | set | messages:
+ msg272345 |
2016-08-09 07:18:59 | terry.reedy | set | nosy:
+ markroseman
|
2016-08-09 06:00:03 | terry.reedy | set | messages:
+ msg272216 |
2016-08-08 22:01:27 | ned.deily | set | status: closed -> open
nosy:
+ ned.deily messages:
+ msg272198
resolution: fixed -> (no value) stage: resolved -> needs patch |
2016-07-08 04:32:00 | terry.reedy | set | status: open -> closed resolution: fixed messages:
+ msg269973
stage: commit review -> resolved |
2016-07-08 04:26:35 | python-dev | set | messages:
+ msg269972 |
2016-07-08 02:37:12 | terry.reedy | set | files:
+ query-helpsource2.diff
messages:
+ msg269967 |
2016-07-08 02:10:03 | terry.reedy | link | issue27465 dependencies |
2016-07-07 03:44:27 | terry.reedy | set | files:
+ query-helpsource.diff
messages:
+ msg269920 stage: test needed -> commit review |
2016-07-04 03:32:51 | terry.reedy | set | messages:
+ msg269777 |
2016-07-03 22:21:35 | terry.reedy | set | files:
+ query4.diff
messages:
+ msg269774 |
2016-06-28 02:33:18 | terry.reedy | set | files:
+ query3.diff
messages:
+ msg269415 stage: needs patch -> test needed |
2016-06-28 00:19:35 | terry.reedy | set | messages:
+ msg269411 stage: patch review -> needs patch |
2016-06-27 04:57:28 | serhiy.storchaka | set | nosy:
+ serhiy.storchaka messages:
+ msg269347
|
2016-06-27 02:08:06 | terry.reedy | set | files:
+ query2.diff
messages:
+ msg269338 |
2016-06-27 02:05:26 | python-dev | set | nosy:
+ python-dev messages:
+ msg269337
|
2016-06-24 04:15:17 | terry.reedy | create | |