bpo-35208: Fix IDLE Squeezer line counting#10449
Conversation
taleinat
added
type-bug
taleinat
force-pushed
the
bpo-35208/squeezer_line_counting_fix
branch
from
73e0fea to
675b42f
Compare
taleinat
force-pushed
the
bpo-35208/squeezer_line_counting_fix
branch
from
675b42f to
01fa746
Compare
|
@terryjreedy, this is a bugfix and I really think it should go into 3.6 before it's too late. Would you like to review this or can I just merge it? |
|
It is semi too late in that the rcs are already out. We would have to commit to 3.8 and 3.7.2+ and request cherry-pick to the two release branches. However, the plural issue is not critical, and as I said on issue, I don't necessarily consider reporting logical lines to be a bug. If I have misunderstood the issue, please clarify. |
|
Example: |
|
At the moment, I cannot make any changes, or even make an inline comment. Same as for #11214. |
All tests continue to pass.
Tests continue to pass.
terryjreedy
left a comment
terryjreedy
left a comment
There was a problem hiding this comment.
I added news items and made a couple of changes. If you are happy with them, go ahead and merge.
Lib/idlelib/squeezer.py
Outdated
| # (1,0), even though a new line hadn't yet been started. The same | ||
| # is true if length is any exact multiple of linewidth. Therefore, | ||
| # subtract 1 before dividing a non-empty line. | ||
| if current_column > linewidth: # Don't bother adding 0. |
There was a problem hiding this comment.
This change does avoid the current_column == 0 edge-case, but the way it is now written (including the comment) makes it seem like a questionable optimization rather than handling an important edge-case (which happens with empty lines).
I suggest at least expanding the comment, e.g.:
# Avoid the `current_column == 0` edge-case, and while we're at it,
# don't bother adding 0.
There was a problem hiding this comment.
The comment is already longer than I would like, but I did not readily see how to nicely condense it. A bit more will not hurt.
|
@terryjreedy, thanks for the review and improvements! I have a few comments, let me know what you think. |
|
Thanks @taleinat for the PR 🌮🎉.. I'm working now to backport this PR to: 3.7. |
|
GH-11305 is a backport of this pull request to the 3.7 branch. |
(cherry picked from commit 44a79cc) Co-authored-by: Tal Einat <taleinat+github@gmail.com>
5 participants
This also fixes the label when squeezing a single line to "Squeezed text (1 line)", rather than "Squeezed text (1 lines)".
https://bugs.python.org/issue35208