bpo-21756: fix IDLE's "show surrounding parens" for multi-line statements

[taleinat]

https://bugs.python.org/issue21756

[csabella]

Just a nit, but I prefer

Suggested change

	def __init__(self, editwin, index, end_at_eol=True):
	def __init__(self, editwin, index, *, end_at_eol=True):

for something like this.

[taleinat]

Great suggestion!

I'm still slowly chipping away at the deeply-ingrained "IDLE supports very old Python versions" rule in my mind...

[terryjreedy]

One 3.7.8 is out, we can consider using new-in-3.8 features such as '/' in signatures and ':='.

[csabella]

This else is a bit unwieldy. Since hyperparser has tests, maybe it can be restructured with a high confidence of correctness?

        else:
            r = text.tag_prevrange("console", index)
            if r:
                startatindex = r[1]
            else:
                startatindex = "1.0"
            stopatindex = "%d.end" % lno
            if end_at_eol:
                stopatindex = "%d.end" % lno
            else:
                r = text.tag_nextrange("console", index)
                if r:
                    stopatindex = text.index(r[0] + "-1c")
                else:
                    stopatindex = "end-1c"

[csabella]

Nice! This works well. test_hyperparser.py has pretty good coverage, so maybe add tests for this change?

Also #18539 changes this code a little. It doesn't fix this issue, but we'll need to rebase whichever one is merged second.

[csabella]

Do you think wrap or something like it would work instead of end_at_eol? I had to stop and think about end_at_eol to comprehend its meaning.

[taleinat]

Yes, the name "end_at_eol" isn't great. I'm not sure "wrap" is clear enough though. Perhaps "stop_at_line_end" or "multiline"?

[terryjreedy]

I prefer one word or at most two (which don't necessarily need '_') rather than a phrase. The docstring can explain the word. In any case, the audience is only IDLE maintainers and there may never be more than one or two calls that change the current default. 'endline=True' is a possibility.

[taleinat]

So "multiline"?

[csabella]

endline, multiline, oneline, singleline, continuation?

[csabella]

Since you're changing this line, an f-string would make this more readable.

[terryjreedy]

Definitely.

[taleinat]

Also #18539 changes this code a little. It doesn't fix this issue, but we'll need to rebase whichever one is merged second.

I'd be happy to handle that merging/rebasing, @csabella.

Also, let me know if you'd like my review for GH-18539.

[taleinat]

Thanks for taking a look at this @csabella! I've made some changes based on your suggestions. It would be great if you could take another look :)

[taleinat]

I also added a test and then discovered that this breaks rather badly in some cases... The underlying PyParse parser strongly assumes that it's only given code up to the end of the current statement.

[terryjreedy]

An possible alternative is to leave Hyperparser.__init__ alone, add the new parameter, possibly endfile=False, to get_surrounding_brackets(), and do a separate forward scan for the closer when endfile is True and afterindex does not mark the closer.

[csabella]

Also, let me know if you'd like my review for GH-18539.

Always! :-)

[taleinat]

So, as it stands, this PR doesn't work properly, and breaks certain use-cases :( It's not a minor problem with the PR, it a problem with the approach. So I'm closing this PR.

Properly fixing bpo-21756 will simply require a more significant refactoring. Specifically, it will likely need a version of the code parser which can handle lines incrementally, maybe both backwards (to find the beginning of a code block) and forwards (to find the end).