Skip to content
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

Open
taleinat opened this issue Jun 14, 2014 · 11 comments
Open

IDLE - ParenMatch fails to find closing paren of multi-line statements #65955

taleinat opened this issue Jun 14, 2014 · 11 comments
Assignees
Labels
3.10 only security fixes topic-IDLE type-bug An unexpected behavior, bug, or error

Comments

@taleinat
Copy link
Contributor

BPO 21756
Nosy @terryjreedy, @taleinat
PRs
  • bpo-21756: fix IDLE's "show surrounding parens" for multi-line statements #20753
  • Dependencies
  • bpo-21686: IDLE - Test hyperparser
  • Files
  • taleinat.20140614.IDLE_parenmatch_multiline_statement.patch: patch fixing the issue
  • 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']

    @taleinat
    Copy link
    Contributor Author

    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.

    @taleinat taleinat added topic-IDLE type-bug An unexpected behavior, bug, or error labels Jun 14, 2014
    @terryjreedy
    Copy link
    Member

    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
    Copy link
    Contributor Author

    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
    Copy link
    Member

    I suspect that the new end_at_eol parameter should take care of calltips.

    @taleinat
    Copy link
    Contributor Author

    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
    Copy link
    Member

    I meant what you said in your last paragraph -- not passing a value for the new parameter would keep current behavior.

    @terryjreedy terryjreedy added the 3.7 (EOL) end of life label Jun 30, 2017
    @terryjreedy terryjreedy self-assigned this Jun 30, 2017
    @terryjreedy
    Copy link
    Member

    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.

    @terryjreedy terryjreedy added 3.10 only security fixes and removed 3.7 (EOL) end of life labels Jun 9, 2020
    @taleinat
    Copy link
    Contributor Author

    taleinat commented 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
    Copy link
    Contributor Author

    taleinat commented Jun 9, 2020

    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.

    @taleinat
    Copy link
    Contributor Author

    taleinat commented Jun 9, 2020

    I've created a PR against current master based on the patch, see #64952.

    @taleinat
    Copy link
    Contributor Author

    taleinat commented 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.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.10 only security fixes topic-IDLE type-bug An unexpected behavior, bug, or error
    Projects
    Status: In Progress
    Development

    No branches or pull requests

    2 participants