calltip_w.py code is partly based on tooltip.py. The latter is currently unused. But it is needed, with changes, for squeezer (bpo-1529353). So I would like to see if we can make an improved Tooltip class that can be used or subclassed by both.

The current subclasses should be deleted unless actually used. Docstrings should be added to what remains. Then test_tooltip, with 100% coverage, should be added. If needed, code can be changed to make testing easier.

I've started work on this. My progress can be followed on the bpo-33839 branch on my GitHub repo: https://github.com/taleinat/cpython/tree/bpo-33839

The refactoring is done, manual testing with the htests works, and I've added automated tests with ~92% code coverage.

Terry, I'd be happy for your review!

Attaching coverage report for tooltip.py.

The uncovered lines are required to avoid exceptions when closing a windows with a tooltip shown. These are not currently reproduced by test_tooltip.py because my attempts to exercise them in automated tests have all failed so far.

The TclError exceptions are reproduced by the htest in tooltip.py. The ValueError exception, however, I've only seen happen when actually running IDLE, since it is raised by the MultiCall module.

IMO it's not worth the effort to achieve 100% automated test coverage here. It's Terry's decision to make, though.

What do you think of changing 'ToolTip' to 'Tooltip'? The me, the extra 'T' is over-aggressive camel-casing that impedes rather than aids reading.

test_tooltip.py was missing the unittest invocation at the end, so I could not the tests from an editor. Without it, coverage on my Win 10 was 24%. With it, 91% (same missing coverage).

There are cases where 100% is close enough to impossible to just move on. I agree that the misse here are not necessarily showstoppers. But I like to check whether the failure does not suggest a code change.

TclError from widget.destroy suggests that we *might* be able to cleanup better.

"The TclError exceptions are reproduced by the htest in tooltip.py. ":
With the code as is? Or if the try-except is removed?

ValueError: where?

Other comments in the review.

As I mentioned elsewhere, I can a) imagine using this with dialogs to pop up hints about the particular entry widget, and b) would like the popups to respond keyboard (Tab) focus changes as well as mouse movement focus changes. Looking at the config dialog, keyboard focus and mouse focus seems to be independent and parallel systems, so I don't know how easy it would be to respond to both. In general, keyboard focus is indicated by a black dotted line ring, while mouse focus (activation) is indicated by a color change (blue on Windows), though for entry widgets, key focus in indicated by blue background while mouse focus is indicated by a cursor change.

What do you think of changing 'ToolTip' to 'Tooltip'?

+1, I'll change it.

TclError from widget.destroy suggests that we *might* be able to cleanup better.

The try/except clauses which are hard to test are used since the order in which widgets are destroyed can vary, and I don't know how to properly check whether a widget still exists (so that e.g. unbinding events won't throw an exception).

For the on-hover tooltip, events are registered on the "anchor" widget. ISTM these events should be unregistered when the tooltip is destroyed, hence I added that to __del__. Any suggestions for a better cleanup mechanism?

> The TclError exceptions are reproduced by the htest in tooltip.py.
With the code as is? Or if the try-except is removed?
Without the try/except.

I [...] would like the popups to respond keyboard (Tab) focus changes as well as mouse movement focus changes.

Looking at the Tk docs, it seems this will require binding properly to the FocusIn and oFocusOut events. This is indeed a separate mechanism. It should be straightforward when bound to a simple widget such as a button. The Calltip class will need to override this since it already implements an appropriate alternative.

I am going to change calltips.py and CallTips to calltip.py and Calltip in bpo-33907, but will not change tooltip.py there as I would expect a merge conflict. Once the change is made elsewhere, change should be made to tooltip and test_tooltip directly.

I just merged 2nd PR for bpo-33907. Combined effect is that
module calltips and its class CallTips are now calltip and Calltip.
In module calltip_w class CallTip is now CalltipWindow.

> TclError from widget.destroy suggests that we *might* be able to cleanup better.

Any suggestions for a better cleanup mechanism?

(Following up my own question with another question...)

Would it be better to check self.winfo_exists() instead of the try/except clauses?

I [...] would like the popups to respond keyboard (Tab) focus changes as well as mouse movement focus changes.

Should tooltips also be shown automatically when the keyboard focus is on a widget? Using the same delay as for the mouse hover, perhaps? I've never seen this in another app that I can recall.

If so, should it be possible for two tooltips to be shown at once, one for the mouse hover and one for the keyboard focus?

If keyboard activation is desired, perhaps we should have a keyboard shortcut for that? We could use the one currently used for showing calltips in the editor. Is there a standard/convention for this on any/all OS?

I just merged 2nd PR for bpo-33907.

I've merged master into the PR branch for this issue and fixed the merge issues.

if: versus try: The latter is faster if try: succeeds. On the other hand, we could make the if: fail by mocking it. Since there is no except: or else: code to test, it hardly seems worth it. A low priority item.

Generalizing 'show calltip': Great idea, as in "Why didn't I think of that!". Works if cursor on same line as squeeze box. Either same delay or calculate from hover delay. If we have different popup contents, I think 1 at a time would be fine.

> If so, should it be possible for two tooltips to be shown at once, one for the mouse hover and one for the keyboard focus?

If we have different popup contents, I think 1 at a time would be fine.

Consider the following example: A window with two buttons, each with a tooltip. The keyboard focus is on one while the mouse hovers over another.

IMO there should never be more than a single tooltip displayed at once (perhaps not including calltips). We can have tooltips auto-hide when another is displayed (and no-op if itself it attempted to be re-displayed).

Having keyboard focus changes automatically trigger tooltip display (with or without delay) could interact poorly with the above in some cases, e.g. a mouse-hover tooltip suddenly disappearing due to a keyboard delay kicking in and displaying another tooltip.

I'm also not sure keyboard focus should affect tooltips shown by mouse-hover. If we have tooltips triggered by the keyboard, however, then the "anchor" widget losing keyboard focus should certainly hide the tooltip. I lean in favor of simplicity and consistency. IMO any keyboard focus change should likely hide all tooltips.

I suggest the following:

1. Add ability to show tooltips with the keyboard using the same keyboard shortcut as "<<force-open-calltip>>", which defaults to Ctrl+/. Perhaps as a separate "<<show-tooltip>>" virtual event, or just rename "<<force-open-calltip>>" to that and use it for both.
2. No auto-display of tooltips triggered by a widget receiving keyboard focus.
3. Any keyboard focus change should hide all tooltips and calltips.

New changeset 87e59ac by Tal Einat in branch 'master':
bpo-33839: refactor IDLE's tooltips & calltips, add docstrings and tests (GH-7683)
87e59ac

New changeset e65ec49 by Miss Islington (bot) in branch '3.7':
bpo-33839: refactor IDLE's tooltips & calltips, add docstrings and tests (GH-7683)
e65ec49

New changeset 2474cef by Miss Islington (bot) in branch '3.6':
bpo-33839: refactor IDLE's tooltips & calltips, add docstrings and tests (GH-7683)
2474cef