This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: IDLE - ParenMatch fails to find closing paren of multi-line statements
Type: behavior Stage: patch review
Components: IDLE Versions: Python 3.10
process
Status: open Resolution:
Dependencies: 21686 Superseder:
Assigned To: terry.reedy Nosy List: taleinat, terry.reedy
Priority: normal Keywords: patch

Created on 2014-06-14 09:48 by taleinat, last changed 2022-04-11 14:58 by admin.

Files
File name Uploaded Description Edit
taleinat.20140614.IDLE_parenmatch_multiline_statement.patch taleinat, 2014-06-14 09:48 patch fixing the issue review
Pull Requests
URL Status Linked Edit
PR 20753 closed taleinat, 2020-06-09 07:58
Messages (11)
msg220542 - (view) Author: Tal Einat (taleinat) * (Python committer) Date: 2014-06-14 09:48
Originally reported on issue #21694.

Regarding, for example, the following code:
(3  +
 4 -   1)

Placing the cursor after the opening parenthesis and invoking the "Show surrounding parens" event causes just the first line to be highlighted. Obviously, the second line should be highlighted as well.

I've found a good (clean & fast) solution for shell windows, but can't find any good way for editor windows other than always parsing the entire code. Doing this every time HyperParser is used could significantly degrade IDLE's performance. Therefore I've decided to have this done only by ParenMatch.flash_paren_event(). Note that ParenMatch.paren_closed_event() doesn't need this, since all of the code which is relevant for inspection lies before the closing parenthesis.

See attached patch fixing this issue. Review and additional testing are required.
msg220583 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2014-06-14 21:41
The current behavior is documented, if not exactly 'chosen' as the best possible. "return the indices of the opening bracket and the closing bracket (or the end of line, whichever comes first". 

As you note, the automatic matching when pausing afte typing a closer only needs to search back, and that should not be affected. When I type ^0, I am willing to wait the extra 1/20? of a second to get a proper highlight.

To see if anything might depend on the truncated search, I grepped

Searching 'get_surrounding_brackets' in C:\Programs\Python34\Lib\idlelib\*.py ...
CallTips.py: 66:         sur_paren = hp.get_surrounding_brackets('(')
HyperParser.py: 105:     def get_surrounding_brackets(self, openers='([{', mustclose=False):
ParenMatch.py: 93:         indices = HyperParser(self.editwin, "insert").get_surrounding_brackets()
ParenMatch.py: 109:         indices = hp.get_surrounding_brackets(_openers[closer], True)

So we have to make sure that calltips are not disabled or burdened. (The 'testing' you said is needed.)

I don't understand the call because the goal is to find the expression preceding '('. There usually is no matching ')' after '(' just typed.  I just know that unless unless evalfuncs is True, calltips are not fetched when the function expression contains a function call. 'f(' gives a calltip for f, but 'f()(' does not call f to give a calltip for f().
msg220585 - (view) Author: Tal Einat (taleinat) * (Python committer) Date: 2014-06-14 21:52
Note that the patch changes the behavior only for ParenMatch.flash_paren_event(). Other uses, such as in CallTips, are not affected.

The only possibly unwanted effect that I can think of is in an editor window, on a line missing a closing parenthesis, triggering flash_paren_event() could highlight the entire rest of the code. I'll have to check this tomorrow.

WRT CallTips.open_calltip() calling HyperParser.get_surrounding_parens('('), that is to find the real last opening parenthesis. It is needed since open_calltip() can be triggered manually, not only after '(' is typed.
msg220592 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2014-06-14 22:54
I suspect that the new end_at_eol parameter should take care of calltips.
msg220612 - (view) Author: Tal Einat (taleinat) * (Python committer) Date: 2014-06-15 05:57
Terry, I'm not sure what you mean but your last comment.

HyperParser.get_surrounding_brackets() will return a previous opening bracket, even if no closing bracket is found for it. CallTips depends on that behavior to find the previous opening parenthesis even if it is not closed.

I can surely say that CallTips profits from the existing behavior of HyperParser, because it doesn't care whether the parenthesis is closed, and this allows HyperParser to do less parsing work.

This patch preserves all of the above and does not affect CallTips at all, since for CallTips it leaves end_at_eol at its default value of True. Likewise for all other uses of HyperParser, including those in ParenMatch, except ParenMatch.flash_paren_event().
msg220616 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2014-06-15 07:32
I meant what you said in your last paragraph -- not passing a value for the new parameter would keep current behavior.
msg371076 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2020-06-09 06:21
Updated summary:
1. Typing a closer highlights closer back to the opener.  No issue.

2. Hitting ^0 on the closer line before the closer highlights from opener to closer.  Hitting ^0 on after the opener on the opener line or any following line before the closer line highlights from opener to the end of that line.  This behavior is documented but seems problematical. ^0 is most useful when the match spans multiple lines.

3. The 4 occurrences of 'get_surrounding_brackets in master.
hyperparser.py, 116:
    def get_surrounding_brackets(self, openers='([{', mustclose=False)

'tip': calltip.py, 61, in open_calltip
        # Called in all 3 tip function.
        sur_paren = hp.get_surrounding_brackets('(')

'flash' (^0): parenmatch.py, 79, in flash_paren_event
        indices = (HyperParser(self.editwin, "insert")
                   .get_surrounding_brackets())

'close' (closer): parenmatch.py, 92, paren_closed_event
        indices = hp.get_surrounding_brackets(_openers[closer], True)

'tip' and 'flash' both use default mustclose=False.  The truncate behavior for tips is that one can move the cursur between opener and closer or eol without dismissing box.  I think we agree that 'tip' should remain truncated while 'flash' need not be.

You solution is to add a new parameter to __init__ only used by 'flash'.  I would rather replace 'mustclose' with mode = 'tip', 'flash', or 'close'.  I believe that Stopatindex could be revised within the call.

I have not reviewed most of the rest of the patch.
msg371077 - (view) Author: Tal Einat (taleinat) * (Python committer) Date: 2020-06-09 07:22
Terry, kudos for getting back to this!

Let me make a PR for this, it will make reviewing and testing easier.
msg371079 - (view) Author: Tal Einat (taleinat) * (Python committer) Date: 2020-06-09 07:42
> You solution is to add a new parameter to __init__ only used by 'flash'.  I would rather replace 'mustclose' with mode = 'tip', 'flash', or 'close'.  I believe that Stopatindex could be revised within the call.

The problem with this suggestion is that 'mustclose' is a parameter for HyperParser.get_surrounding_brackets(). When that is called, it is too late to affect whether the parsing goes beyond the end of the current line. We could add a 'mode' parameter to the HyperParser constructor, though.

Still, I don't like that approach: HyperParser is currently well encapsulated, but giving it such "modes" would tightly couple it with its current specific uses in IDLE. The approach used in the current patch, with the 'end_at_eol' parameter, preserves HyperParser's encapsulation.
msg371080 - (view) Author: Tal Einat (taleinat) * (Python committer) Date: 2020-06-09 08:00
I've created a PR against current master based on the patch, see GH-20753.
msg377950 - (view) Author: Tal Einat (taleinat) * (Python committer) Date: 2020-10-04 15:46
As noted on the PR GH-20753, an incremental change to the current logic doesn't seem like a promising approach.

It seems to me that this will require refactoring the code parsing logic to allow it to incrementally parse lines backwards and forwards in order to find the beginning and end of a code block.
History
Date User Action Args
2022-04-11 14:58:04adminsetgithub: 65955
2020-10-04 15:46:19taleinatsetmessages: + msg377950
2020-06-09 08:00:01taleinatsetmessages: + msg371080
2020-06-09 07:58:48taleinatsetpull_requests: + pull_request19955
2020-06-09 07:42:34taleinatsetmessages: + msg371079
2020-06-09 07:22:34taleinatsetmessages: + msg371077
2020-06-09 06:21:11terry.reedysetversions: + Python 3.10, - Python 3.6, Python 3.7
2020-06-09 06:21:04terry.reedysetmessages: + msg371076
2017-06-30 00:56:48terry.reedysetassignee: terry.reedy
stage: test needed -> patch review
versions: + Python 3.6, Python 3.7, - Python 2.7, Python 3.4, Python 3.5
2014-06-15 07:32:09terry.reedysetmessages: + msg220616
2014-06-15 05:57:10taleinatsetmessages: + msg220612
2014-06-14 23:18:28terry.reedysetdependencies: + IDLE - Test hyperparser
stage: test needed
2014-06-14 22:54:04terry.reedysetmessages: + msg220592
2014-06-14 21:52:55taleinatsetmessages: + msg220585
2014-06-14 21:41:57terry.reedysetmessages: + msg220583
2014-06-14 09:48:56taleinatsettype: behavior
2014-06-14 09:48:15taleinatcreate