Repository navigation
IDLE - ParenMatch fails to find closing paren of multi-line statements #65955
Description
| BPO | 21756 |
|---|---|
| Nosy | @terryjreedy, @taleinat |
| PRs | |
| Dependencies | |
| Files |
Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.
Show more details
GitHub fields:
assignee = 'https://github.com/terryjreedy'
closed_at = None
created_at = <Date 2014-06-14.09:48:15.547>
labels = ['expert-IDLE', 'type-bug', '3.10']
title = 'IDLE - ParenMatch fails to find closing paren of multi-line statements'
updated_at = <Date 2020-10-04.15:46:19.724>
user = 'https://github.com/taleinat'bugs.python.org fields:
activity = <Date 2020-10-04.15:46:19.724>
actor = 'taleinat'
assignee = 'terry.reedy'
closed = False
closed_date = None
closer = None
components = ['IDLE']
creation = <Date 2014-06-14.09:48:15.547>
creator = 'taleinat'
dependencies = ['21686']
files = ['35626']
hgrepos = []
issue_num = 21756
keywords = ['patch']
message_count = 11.0
messages = ['220542', '220583', '220585', '220592', '220612', '220616', '371076', '371077', '371079', '371080', '377950']
nosy_count = 2.0
nosy_names = ['terry.reedy', 'taleinat']
pr_nums = ['20753']
priority = 'normal'
resolution = None
stage = 'patch review'
status = 'open'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue21756'
versions = ['Python 3.10']Metadata
Metadata
Assignees
Projects
- Status
Activity
taleinat commentedon Jun 14, 2014
Originally reported on issue bpo-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.
terryjreedy commentedon Jun 14, 2014
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().
taleinat commentedon Jun 14, 2014
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.
terryjreedy commentedon Jun 14, 2014
I suspect that the new end_at_eol parameter should take care of calltips.
taleinat commentedon Jun 15, 2014
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().
terryjreedy commentedon Jun 15, 2014
I meant what you said in your last paragraph -- not passing a value for the new parameter would keep current behavior.
terryjreedy commentedon Jun 9, 2020
Updated summary:
Typing a closer highlights closer back to the opener. No issue.
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.
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.
taleinat commentedon Jun 9, 2020
Terry, kudos for getting back to this!
Let me make a PR for this, it will make reviewing and testing easier.
taleinat commentedon Jun 9, 2020
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.
taleinat commentedon Jun 9, 2020
I've created a PR against current master based on the patch, see #64952.
taleinat commentedon Oct 4, 2020
As noted on the PR #64952, 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.