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.

Bah, the initial version contained a silly bug in
EditorWindow.goto_line_event. Please remove it, and use this version
instead.

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)

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.

Please provide a single patch file.

uploaded a single comprehensive patch file

After more testing, I discovered a bug which broke Goto Line. Attaching
a fixed patch.

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.

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.

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.

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.

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.

> 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.

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.

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.

Bump as IDLE is now being actively developed.

I have not looked at the details, so cannot currently comment as to whether the idea should be pursued or dropped. Perhaps when Roger is done with overt bugs he will take a look and offer an opinion.

Attached is a refreshed version of the patch against current 2.7 tip.  I did a quick visual comparison with the original but make no guarantees that it is totally accurate.  I have no plans to work on it further myself.

The patch does two things.

1. It replaces the existing direct rebinding of messagebox functions as methods, such as
    self.showerror = tkMessageBox.showerror
with binding of a double wrapping of the functions. The middle layer is useless and only serves to allow use of the decorator syntax. The 'old-fashioned' method of wrapping by function call should be used.
    self.showerror = _tk_dialog_wrapper(tkMessageBox.showerror)
A single customized wrapping for the existing bindings seems like it might be a good idea.

I do not understand the 'motivation' (of the existing and revised method bindings) of making Idle extensions 'cross-IDE'. How so? What IDEs other than Idle are we concerned with?

The patch replaces both parent=self.text and master=self.text in current calls with parent=self.text in the wrapper. This seems to assume that parent==master in these contexts. I gather that this is nearly always the case.
( http://mail.python.org/pipermail/tutor/2010-June/076550.html
based on Grayson's book) But I wonder if the choice could ever make a difference.

2. Currently, EditorWindow methods sometimes call a message function as a message module attribute and sometimes as one of the EditorWindow method. The patch makes these calls consistently be methods calls (to the wrappers). It also changes call in classes that are EditorWindow subclasses or containers. (askokcancel is addded to EditorWindow for patches to some of these other classes.)

My current Idle concern is testing. The current inconsistency in EditorWindow is not good for that, so I will want to fix that one way or the other in any case. For automated non-gui testing, the message functions (or the message module) need to be replaced with mocks that a) save the args somewhere where they can be later retrieved and b) return without graphics system and user interaction. For modules with non-editor classes, this means monkey-patching the module. (I have done this once for one test.) I will experiment with also monkey-patching editor windows.

I suggest that we close this issue. IMO the potential benefit is too small relative to the work required, and the interest is too low.

I would rather leave this open for the moment as an information resource.  I wrote query.py with a base class and subclasses to standardize asking users for input.  So some of the patch may be obsolete, but it does have a catalog of popup uses.

The patch consistently replaces tkMessagebox.showerror with widget.showerror.  I have done that in several places, although widget.showerror may just be messagebox.showerror.  At least some of my motivation was making testing easier, by adding a mock to the test instance instead of temporarily monkeypatching messagebox.  If a custom Showerror class were defined in, for instant, query.py, "self.showerror = tkMessagebox.showerror" could easily be replaced by "self.showerror = query.Showerror.

I also have some interest in consistently handling focus in response to key presses.  (There was recently a request for this on idle-dev.)