|
msg62214 - (view) |
Author: Tal Einat (taleinat)  |
Date: 2008-02-09 00:10 |
In many places in the code, tkMessageBox dialogs were being used
directly, with the master (parent) widget being set explicitly to the
EditorWindow's text widget. Only in some cases was the focus being set
to the text widget afterwards, although in most this is the Right Thing
To Do.
This patch adds a decorator which wraps a tkMessage box (or similar)
function to use the EditorWindow's text widget as a parent by default
and set focus back to it afterwards. This is used to wrap the showerror
and ask* methods. It also changes the code to use these methods wherever
appropriate.
All in all, the change in functionality is that the dialogs' parent
widget will always be the text widget, and that focus will always be
returned to it. This makes the interface more consistent. As a bonus, a
lot of repetitive cruft is removed from the code, and writing extensions
is made another bit simpler.
|
|
msg62216 - (view) |
Author: Tal Einat (taleinat)  |
Date: 2008-02-09 01:54 |
Bah, the initial version contained a silly bug in
EditorWindow.goto_line_event. Please remove it, and use this version
instead.
|
|
msg62219 - (view) |
Author: Tal Einat (taleinat)  |
Date: 2008-02-09 11:20 |
Going through the code more thoroughly, found additional places where
the standardized dialogs should be used. Also fixed remaining places
where these dialogs are used but the parent widget was still being
explicitly set to be the text widget, which is now the default.
(should be easy to apply both patches in any order without conflicts)
|
|
msg62270 - (view) |
Author: Kurt B. Kaiser (kbk) *  |
Date: 2008-02-11 02:42 |
I would greatly appreciate it if you would slow down and test your
patches and use them for an extended period of time (preferably with
some other people trying them) before submitting them. It's quite
aggravating to start working on one, and even have it checked in, only
to have to start over again. It doubles the work and makes me
unwilling to address your patches. How much time have you given the
second version? Deleting both patches, please provide a combination
patch
after you've tested it and thought about it for a few days.
|
|
msg65885 - (view) |
Author: Kurt B. Kaiser (kbk) *  |
Date: 2008-04-27 15:00 |
Please provide a single patch file.
|
|
msg65977 - (view) |
Author: Tal Einat (taleinat)  |
Date: 2008-04-29 18:23 |
uploaded a single comprehensive patch file
|
|
msg67848 - (view) |
Author: Tal Einat (taleinat)  |
Date: 2008-06-08 21:13 |
After more testing, I discovered a bug which broke Goto Line. Attaching
a fixed patch.
|
|
msg76619 - (view) |
Author: Tal Einat (taleinat)  |
Date: 2008-11-29 18:26 |
Attaching a new patch against a more recent revision (resolved minor
conflict).
Only 3 extremely minor changes from previous patch: fixed two more
places where the parent was being passed explicitly (no longer required)
and changed two tabs -> spaces.
I've done some fairly thorough manual testing of this, and have been
using it without any issues for almost six months.
|
|
msg91183 - (view) |
Author: Guilherme Polo (gpolo) *  |
Date: 2009-08-02 15:11 |
I have reviewed the latest patch now, it is nice in general but I
dislike the idea of increasing even more EditorWindow. I would really
prefer to put this somewhere else, like.. idlelib/dialogs.py. Also, I'm
inclined to remove this master/parent hiding and continue making it
explicit.
|
|
msg91197 - (view) |
Author: Tal Einat (taleinat)  |
Date: 2009-08-02 19:22 |
The whole point of this patch is to make the EditorWindow's Text widget
the default master/parent for its dialogs!
IMO this makes sense, since this is a reasonable and intuitive default
for such methods of the EditorWindow object. I understand the "explicit
is better than implicit" argument, but that doesn't mean that having a
reasonable default value is always wrong. In this case having a default
value both simplifies the code considerably and makes sense, so IMO it
is justified.
Also, please note that the patch actually reduces the size of
EditorWindow.py (both character and line counts) and cleans up the code
in many places. Yes, the definition of the relevant methods is not
trivial, but it is in just one area of the code.
|
|
msg91206 - (view) |
Author: Guilherme Polo (gpolo) *  |
Date: 2009-08-02 20:54 |
I thought the main point of this issue was to define standard functions,
methods, classes (or something in that sense), in order to make the
creation of tk dialogs uniform across IDLE.
Right now it seems reasonable to keep the parent hidden, but I think it
may not help much for maintaining. I really consider a good idea to
define explicitly who is the parent of a dialog, it wasn't an attempt to
refer to the phrase you cited.
I trust you it reduces the size on that file, I really didn't count how
many lines were removed and how many were added, but these dialogs can
be used anywhere around IDLE so I don't see why keeping them inside
EditorWindow is a good idea. It would reduce even more (some few lines)
EditorWindow's size doing so, and it would make more sense as I see.
|
|
msg109873 - (view) |
Author: Tal Einat (taleinat)  |
Date: 2010-07-10 14:41 |
Because of the method of implementation, a dialog can be created without specifying a parent. There was at least one instance of this in IDLE before my patch. This is a bug, since the behavior of the dialog in this instance is obviously incorrect (the user can switch focus back to the parent while the dialog is still open).
I found that making the EditorWindow the default parent was the best way to both fix the bug, make the code more readable, and avoid replicating this bug in the future. Feel free to write up a different patch or change mine if you feel so strongly about the default parent.
The process of discussing this patch has taken far, far too long. I can't stand such discussions spanning months (and years!). Please just accept this patch or close it. I'm fed up with this.
|
|
msg109913 - (view) |
Author: Guilherme Polo (gpolo) *  |
Date: 2010-07-10 19:32 |
> Tal Einat added the comment:
>
> ...
>
> The process of discussing this patch has taken far, far too long. I can't stand such discussions spanning months (and years!). Please just accept this patch or close it. I'm fed up with this.
>
If the patch were good enough for everyone then it could have been
committed about 2 years ago, but it is not the case apparently (kbk
also raised some points earlier). Also, the effective discussion has
taken maybe some hours and this large extra time shows this issue is
not important enough to be noticed and taken proper care (and I'm
mostly sure you know this situation isn't exclusive to this
issue/Python/other (closed or open) projects). Said that, I don't see
the reason to just accept the patch and close the issue if no one else
fully backs up your patch.
I understand you may want to no longer follow the issue, and I can't
blame you because I've been mostly away from Python development these
months.
|
|
msg109942 - (view) |
Author: Tal Einat (taleinat)  |
Date: 2010-07-10 23:06 |
At this point I would prefer that this patch just be rejected rather than continue discussing it.
I can understand IDLE being a low priority, but someone could have just stated that to begin with and I wouldn't have bothered working for hours to work up a patch and discuss its merits.
Guilherme, I appreciate your taking an interest in this and obviously you have only helped move this forward. My frustration is with the devs allowing me to waste hours of my time, while requesting that I spend even more time testing and consolidating patches before submission, and then proceeding to ignore my work completely.
|
|
msg110628 - (view) |
Author: Mark Lawrence (BreamoreBoy) |
Date: 2010-07-18 10:28 |
For the record there has been a sizeable thread on python-dev in the last few days regarding IDLE. As a result it is extremely likely that IDLE outstanding issues will be picked up, so please leave this issue open.
|
|
| Date |
User |
Action |
Args |
| 2010-07-18 17:51:29 | terry.reedy | set | files:
- IDLE_standardize_dialogs.080609.patch |
| 2010-07-18 17:51:02 | terry.reedy | set | files:
- IDLE_standardize_dialogs.080429.patch |
| 2010-07-18 10:28:37 | BreamoreBoy | set | nosy:
+ terry.reedy, BreamoreBoy messages:
+ msg110628
|
| 2010-07-10 23:06:52 | taleinat | set | messages:
+ msg109942 |
| 2010-07-10 19:32:45 | gpolo | set | messages:
+ msg109913 |
| 2010-07-10 14:41:38 | taleinat | set | messages:
+ msg109873 |
| 2010-07-10 06:38:47 | terry.reedy | set | versions:
+ Python 3.2, - Python 2.6 |
| 2009-08-02 20:54:09 | gpolo | set | messages:
+ msg91206 |
| 2009-08-02 19:22:18 | taleinat | set | messages:
+ msg91197 |
| 2009-08-02 15:11:27 | gpolo | set | messages:
+ msg91183 |
| 2009-04-26 22:15:47 | ajaksu2 | set | nosy:
+ gpolo
stage: patch review |
| 2008-11-29 18:26:58 | taleinat | set | files:
+ IDLE_standardize_dialogs.081129.patch messages:
+ msg76619 |
| 2008-06-08 22:08:44 | taleinat | set | files:
+ IDLE_standardize_dialogs.080609.patch |
| 2008-06-08 22:07:37 | taleinat | set | files:
- IDLE_standardize_dialogs.080609.patch |
| 2008-06-08 21:13:43 | taleinat | set | files:
+ IDLE_standardize_dialogs.080609.patch messages:
+ msg67848 |
| 2008-04-29 18:23:15 | taleinat | set | files:
+ IDLE_standardize_dialogs.080429.patch messages:
+ msg65977 |
| 2008-04-27 15:00:44 | kbk | set | messages:
+ msg65885 |
| 2008-02-11 02:42:44 | kbk | set | files:
- IDLE_standardize_dialogs.080209_2.patch |
| 2008-02-11 02:42:39 | kbk | set | files:
- IDLE_standardize_dialogs.080209.patch |
| 2008-02-11 02:42:30 | kbk | set | messages:
+ msg62270 |
| 2008-02-09 11:20:34 | taleinat | set | files:
+ IDLE_standardize_dialogs.080209_2.patch messages:
+ msg62219 |
| 2008-02-09 02:17:49 | christian.heimes | set | files:
- IDLE_standardize_dialogs.080208.patch |
| 2008-02-09 02:17:43 | christian.heimes | set | priority: normal assignee: kbk type: enhancement keywords:
+ patch versions:
- Python 2.5 |
| 2008-02-09 01:54:42 | taleinat | set | files:
+ IDLE_standardize_dialogs.080209.patch messages:
+ msg62216 |
| 2008-02-09 00:10:20 | taleinat | create | |