New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
IDLE - ParenMatch fails to find closing paren of multi-line statements #65955
Comments
Originally reported on issue bpo-21694. Regarding, for example, the following code: 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. |
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 ... 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(). |
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. |
I suspect that the new end_at_eol parameter should take care of calltips. |
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(). |
I meant what you said in your last paragraph -- not passing a value for the new parameter would keep current behavior. |
Updated summary:
'tip': calltip.py, 61, in open_calltip 'flash' (^0): parenmatch.py, 79, in flash_paren_event 'close' (closer): parenmatch.py, 92, paren_closed_event '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. |
Terry, kudos for getting back to this! Let me make a PR for this, it will make reviewing and testing easier. |
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. |
I've created a PR against current master based on the patch, see #64952. |
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. |
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:
bugs.python.org fields:
The text was updated successfully, but these errors were encountered: