bpo-40511: fix unnecessary flashing of calltips by taleinat · Pull Request #20910 · python/cpython

taleinat · 2020-06-16T09:18:24Z

https://bugs.python.org/issue40511

roseman

tried it out, looks good!

taleinat · 2020-10-25T07:20:27Z

Thanks for trying this out and giving some feedback, @roseman!

terryjreedy

I verified that this patch fixes flashing in at least one case each of parens in string and bytes literals, comments, and expressions.

My first code concern is whether each new line is needed. I suspect some are either not or are not the best fix.

My second is whether the fix introduces regressions. Might tips be suppressed when needed? I tested both nested function calls such as int(float(a)) ('float(' open a float tip, 'a)' closes it and restores the int tip) and forced opens at various places. These continue working. To completely understand the new code and be more confident, I need to look at hyperparser.

I outlined a new non-gui open_calltip testcase and am willing to write it. A slightly different approach would be to add the new test code examples to the hyperparser test and then mock hyperparser for these tests.

bedevere-bot · 2020-10-28T07:57:31Z

When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.

taleinat · 2020-10-28T08:43:07Z

Thanks for the review, @terryjreedy! 🙏

I have made the requested changes; please review again.

I outlined a new non-gui open_calltip testcase and am willing to write it. A slightly different approach would be to add the new test code examples to the hyperparser test and then mock hyperparser for these tests.

Let me know if you'd like me to write such tests or if you'd rather do so yourself.

bedevere-bot · 2020-10-28T08:43:10Z

Thanks for making the requested changes!

@terryjreedy: please review the changes made to this pull request.

terryjreedy

A separate existing bug for another issue: if an inner call has no tip, the outer tip is closed anyway (a correct thing to do) but not restored when the the inner call is completed with ')'. To fix, we might either stack outer tips or flag nested tips.

But I don't want to make additional code changes (other than a docstring and a revised comment) until we have tests, which I will start now.

terryjreedy · 2020-10-30T00:04:39Z

        sur_paren = hp.get_surrounding_brackets('(')
        if not sur_paren:
+            # Not inside parentheses: Don't open a calltip, and close one if
+            # currently open.


Suggested change

if not sur_paren:

# Not inside parentheses: Don't open a calltip, and close one if

# currently open.

if not sur_paren: # Outer call just closed by releasing ).

See response to your response below.

Typing ')' is not the only case where this branch will be reached: it can also happen in various other cases via force_open_calltip_event().

terryjreedy · 2020-10-30T01:02:32Z

+            # in a string or the opener for a tuple: Do nothing.
            return
+
+        # At this point, the current index is after an opening parenthesis, in


I might later think about rewording or condensing the added comments, but tests are a higher priority.

bedevere-bot · 2020-10-30T01:06:10Z

When you're done making the requested changes, leave the comment: I have made the requested changes; please review again.

terryjreedy · 2020-10-30T06:30:58Z

I think the tests I added are sufficient for this issue. open_calltip is covered except for some of the if branches. I plan to follow-up with a separate test PR, aiming for 100% coverage.

terryjreedy · 2020-11-01T02:05:47Z

Tal, if you are too busy now, do you mind if I finish this so I can continue with followups?

Co-authored-by: Terry Jan Reedy <tjreedy@udel.edu>

taleinat · 2020-11-01T09:08:05Z

@terryjreedy, I have made the requested changes; please review again.

I've also refactored the new code in test_calltip a bit, and somewhat condensed the comments you mentioned.

AFAICT that should address your review requests. If I've missed something, apologies in advance, please let me know and I'll handle it quickly.

bedevere-bot · 2020-11-01T09:08:08Z

Thanks for making the requested changes!

@terryjreedy: please review the changes made to this pull request.

…github.com/taleinat/cpython into pr_20910

terryjreedy

I agree with the test change. The test failure was whitespace in the added docstring. My fault I presume. I will merge when the retest passes.

REs with re-specific \ escapes must be r-prefixed to avoid deprecation warning now, exception later. python -m test.test_idle prints them. I added it to the RE I added to mock Text.

miss-islington · 2020-11-02T03:59:55Z

Thanks @taleinat for the PR, and @terryjreedy for merging it 🌮🎉.. I'm working now to backport this PR to: 3.8, 3.9.
🐍🍒⛏🤖

They were occurring with both repeated 'force-calltip' invocations and by typing parentheses in expressions, strings, and comments in the argument code. Co-authored-by: Terry Jan Reedy <tjreedy@udel.edu> (cherry picked from commit da7bb7b) Co-authored-by: Tal Einat <taleinat+github@gmail.com>

bedevere-bot · 2020-11-02T04:00:08Z

GH-23093 is a backport of this pull request to the 3.9 branch.

They were occurring with both repeated 'force-calltip' invocations and by typing parentheses in expressions, strings, and comments in the argument code. Co-authored-by: Terry Jan Reedy <tjreedy@udel.edu> (cherry picked from commit da7bb7b) Co-authored-by: Tal Einat <taleinat+github@gmail.com>

bedevere-bot · 2020-11-02T04:00:16Z

GH-23094 is a backport of this pull request to the 3.8 branch.

They were occurring with both repeated 'force-calltip' invocations and by typing parentheses in expressions, strings, and comments in the argument code. Co-authored-by: Terry Jan Reedy <tjreedy@udel.edu> (cherry picked from commit da7bb7b) Co-authored-by: Tal Einat <taleinat+github@gmail.com>

They were occurring with both repeated 'force-calltip' invocations and by typing parentheses in expressions, strings, and comments in the argument code. Co-authored-by: Terry Jan Reedy <tjreedy@udel.edu>

bpo-40511: fix unnecessary flashing of calltips

af9a54c

taleinat requested a review from terryjreedy as a code owner June 16, 2020 09:18

the-knights-who-say-ni added the CLA signed label Jun 16, 2020

bedevere-bot added the awaiting core review label Jun 16, 2020

roseman approved these changes Oct 24, 2020

View reviewed changes

Update calltip_w.py

228b693

Merge remote-tracking branch 'upstream/master' into pr_20910

687ebfa

terryjreedy requested changes Oct 28, 2020

View reviewed changes

Comment thread Lib/idlelib/calltip.py

Comment thread Lib/idlelib/calltip.py

Comment thread Lib/idlelib/calltip.py

Comment thread Lib/idlelib/calltip.py

bedevere-bot added awaiting changes and removed awaiting core review labels Oct 28, 2020

bedevere-bot added awaiting change review and removed awaiting changes labels Oct 28, 2020

bedevere-bot requested a review from terryjreedy October 28, 2020 08:43

terryjreedy requested changes Oct 30, 2020

View reviewed changes

bedevere-bot added awaiting changes and removed awaiting change review labels Oct 30, 2020

terryjreedy added 3 commits October 30, 2020 02:12

Merge remote-tracking branch 'upstream/master' into pr_20910

a96ae42

Augment mock_tk.Text for hyperparser 'end-#c' indexes.

41573a4

Test that bugs are fixed.

7849ed9

taleinat and others added 3 commits November 1, 2020 10:29

add a doc-string

3dd380e

Co-authored-by: Terry Jan Reedy <tjreedy@udel.edu>

refactor new calltip test code

2cf1351

shorten and properly wrap open_calltip code comments

f3bcb60

bedevere-bot added awaiting change review and removed awaiting changes labels Nov 1, 2020

bedevere-bot requested a review from terryjreedy November 1, 2020 09:08

terryjreedy added 4 commits November 1, 2020 22:04

Merge remote-tracking branch 'upstream/master' into pr_20910

e2749fb

Add 'r' prefix to RE to prevent deprecation warning.

2ac3a6f

Merge branch 'bpo-40511/IDLE-fix-parens-flashing-calltip' of https://…

3ddbbe8

…github.com/taleinat/cpython into pr_20910

Whitespace fix

d12afc9

terryjreedy approved these changes Nov 2, 2020

View reviewed changes

bedevere-bot added awaiting merge and removed awaiting change review labels Nov 2, 2020

terryjreedy added needs backport to 3.8 type-bug An unexpected behavior, bug, or error labels Nov 2, 2020

terryjreedy merged commit da7bb7b into python:master Nov 2, 2020

bedevere-bot removed the awaiting merge label Nov 2, 2020

bedevere-bot removed the needs backport to 3.9 label Nov 2, 2020

bedevere-bot removed the needs backport to 3.8 label Nov 2, 2020

taleinat deleted the bpo-40511/IDLE-fix-parens-flashing-calltip branch November 2, 2020 06:57

Uh oh!

Uh oh!

Conversation

taleinat commented Jun 16, 2020 • edited by bedevere-bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

roseman left a comment

Choose a reason for hiding this comment

Uh oh!

taleinat commented Oct 25, 2020

Uh oh!

terryjreedy left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

bedevere-bot commented Oct 28, 2020

Uh oh!

taleinat commented Oct 28, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

bedevere-bot commented Oct 28, 2020

Uh oh!

terryjreedy left a comment

Choose a reason for hiding this comment

Uh oh!

terryjreedy Oct 30, 2020

Choose a reason for hiding this comment

Uh oh!

taleinat Nov 1, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

terryjreedy Oct 30, 2020

Choose a reason for hiding this comment

Uh oh!

bedevere-bot commented Oct 30, 2020

Uh oh!

terryjreedy commented Oct 30, 2020

Uh oh!

terryjreedy commented Nov 1, 2020

Uh oh!

taleinat commented Nov 1, 2020

Uh oh!

bedevere-bot commented Nov 1, 2020

Uh oh!

terryjreedy left a comment

Choose a reason for hiding this comment

Uh oh!

miss-islington commented Nov 2, 2020

Uh oh!

bedevere-bot commented Nov 2, 2020

Uh oh!

bedevere-bot commented Nov 2, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

taleinat commented Jun 16, 2020 •

edited by bedevere-bot

Loading

taleinat commented Oct 28, 2020 •

edited

Loading

taleinat Nov 1, 2020 •

edited

Loading