msg329600 - (view) |
Author: Tal Einat (taleinat) *  |
Date: 2018-11-10 07:18 |
Squeezing a single long line with a newline, e.g. 'a'*200, results in "Squeezed text (1 lines)".
Also, changing the width of the window (even to very small widths) doesn't affect the number of lines reported when squeezing (when squeezing *after* the width change).
|
msg329602 - (view) |
Author: Tal Einat (taleinat) *  |
Date: 2018-11-10 07:57 |
See PR GH-10449 with a fix.
|
msg331722 - (view) |
Author: Terry J. Reedy (terry.reedy) *  |
Date: 2018-12-12 23:44 |
Changing the width of the window changes the number of visible lines because of line wrapping. But it does not change the number of logical lines. My understanding is that squeezer currently reports the latter, and that is not necessarily a bug. Not that I am considering replacing line wrapping with a horizontal scrollbar.
|
msg331742 - (view) |
Author: Terry J. Reedy (terry.reedy) *  |
Date: 2018-12-13 05:43 |
Note that I *am* considering ...
It seems that a reasonable rule might be to squeeze if n lines or the equivalent of n full lines (75 chars each) in total characters. In other words, if lines >= N or chars >= to 75*N: squeeze(). Do we have a rule not to squeeze fewer but longer lines?
|
msg331756 - (view) |
Author: Tal Einat (taleinat) *  |
Date: 2018-12-13 11:39 |
Squeezer currently does take wrapping into account, i.e. it does take IDLE's "soft" line wrapping into account when counting lines. Therefore the auto-squeeze criterion of "at least X lines" does lead to auto-squeezing for any mixture of very long lines and/or many short lines.
However, there is currently a bug in the line counting mechanism which is causing it to not count long, soft-wrapped lines properly in some cases. This issue is about that bug, and the PR fixes it. Merging this fix should result in the originally intended behavior, which is similar to what Terry described in his last comment here.
|
msg331757 - (view) |
Author: Terry J. Reedy (terry.reedy) *  |
Date: 2018-12-13 12:52 |
In the example I gave on the PR, 200 70 char lines, the squeezed box says 200 lines with or without line wrapping (before the patch).
What is a simple case that you think is buggy?
|
msg331758 - (view) |
Author: Tal Einat (taleinat) *  |
Date: 2018-12-13 13:05 |
As I mentioned when opening this issue: 'a'*200 (or 'a'*10000).
|
msg331759 - (view) |
Author: Tal Einat (taleinat) *  |
Date: 2018-12-13 13:06 |
Note the lack of newline at the end; that triggers the problematic path in the current calculation.
|
msg331779 - (view) |
Author: Terry J. Reedy (terry.reedy) *  |
Date: 2018-12-14 04:29 |
There are 2 questions. 1. Should we review squeezer, in light of further complemplation and experience, and possibly patch it in a couple of weeks or so? Yes.
2. Is there such a severe bug that we should possibly rush a fix and ask Ned to cherry-pick something after the well announced deadline? Since I see behavior different from your report, I don't currently see a reason to ask.
I have tested with Windows 3.7.1 64 bit installed, Windows 3.7.2c1+ 32 bit repository debug build from yesterday, Mac 3.7.2rc1 64 bit installed.
'a'*200 and 'a'*200 + '\n' both display over 3 lines and are *not* autosqueezed. Both squeeze as 3 lines. Unsqueeze, narrow window, and resqueeze, and the result is *4* lines, not 3. Line single lines with 20000 and 20002 characters, a'*20000 and 400 * (48*'a'+'\n), are autosqueezed, as more than 250 lines, depending on the window width. Note interactive echo uses repr(), so that the '\n' converted to 1 newline is converted back to '\n'. Lines here also depends on width.
print(200 * (70*'a'+'\n')) is squeezed as 200 lines regardless of wrapping or not. So the effect of wrapping on line count depends on the number of lines.
print(40 * (100*'a'+'\n')) (4000 chars) is not squeezed, even though it would be by the 50 lines * 75 chars/line (3750 chars) rule I suggested.
Further experiments shows that autosqueezing only applies to single writes to sys.stdout (and maybe to sys.stderr). print(40 * (100*'a'+'\n'), 40 * (100*'a'+'\n')) prints 40 lines twice with separate write calls and is not autosqueezed. Manual squeeze squeezes contiguous stdout or stderr blocks even if the former is the result of more than one print arg or calls. Stdout blocks separated by an input() response are autosqueezed separately. This all seems fine. The doc could use a few more words.
|
msg331784 - (view) |
Author: Tal Einat (taleinat) *  |
Date: 2018-12-14 06:57 |
My mistake on the reproduction; try: print('a'*10000+'\n'). It does not trigger auto-squeezing, though this is an obvious case where it should be triggered. (Manual squeezing also results in showing just 1 as the line count.)
|
msg331885 - (view) |
Author: Terry J. Reedy (terry.reedy) *  |
Date: 2018-12-15 03:18 |
My suggest simple rule would sqeeze it. With n = 50 and k = 75,
if len(s) > n*k or s.count('\n') > n: squeeze(s)
would squeeze at 3750. With k = 50, as 2500.
I am not sure yet what to do about wrapping. For development, a count of actual lines might be more helpful than a variable number of wrapped lines.
|
msg331904 - (view) |
Author: Tal Einat (taleinat) *  |
Date: 2018-12-15 18:37 |
Terry, I'm not sure I follow your thinking, but it feels like over-thinking. This is a simple bug with a simple fix.
Barring the current bug (which the attached PR fixes), the current logic counts lines, taking soft wrapping into account. This was chosen to make things as simple as possible for the users, in terms of what's displayed on squeezed buttons and in terms of configuring auto-squeezing. The fix is simple and with it Squeezer works quickly and consistently.
If you'd like to change the behavior, let's discuss that elsewhere.
|
msg332278 - (view) |
Author: Terry J. Reedy (terry.reedy) *  |
Date: 2018-12-21 05:13 |
Tal, trying to understand your confused description of what behavior you want to fix required me to experiment and think. There are at least 2 separate issues: triggering of auto-squeeze and lines reported (regardless of what triggers squeezing). The following pair of experiments exhibits inconsistency in both respects.
>>> print('a'*3920) # Fills 49 80-char lines, correctly not squeezed.
...
>>> print('a'*3921) # Wrapped to 50 lines, correctly auto squeezed.
[Squeezed text (50 lines).] # Correct number when reporting wrapped lines.
>>> print('a'*3921+'\n') # Ditto, but not auto-squeezed.
...
# Squeeze manually
[Squeezed text (1 line).] # Different line count -- of output lines.
>>> print('a'*3920+'\na') # Not initially squeezed, '2 lines'.
From msg331784 it appears that you are more concerned here with auto squeeze triggering than with line count. Now that I think I know what you are trying to fix, I can review the code change.
I agree to consider the ambiguity between output lines and display lines, and the effect on line count, later.
Part of my thinking with the simple auto-squeeze formula, besides just simplifying the code, it this. Raymond claimed that squeezing slows down printing. If measurably true, one way to avoid a slow down would be to use a simple heuristic formula to estimate the number of wrapped lines instead of exactly counting. This would be a separate issue, if needed.
|
msg332386 - (view) |
Author: Tal Einat (taleinat) *  |
Date: 2018-12-23 11:15 |
> Part of my thinking with the simple auto-squeeze formula, besides just simplifying the code, it this. Raymond claimed that squeezing slows down printing. If measurably true, one way to avoid a slow down would be to use a simple heuristic formula to estimate the number of wrapped lines instead of exactly counting. This would be a separate issue, if needed.
After Raymond opened issue #35196, I checked and found several places where the auto-squeeze code was being inefficient. There's a PR ready with those optimizations attached to that issue (GH-10454).
|
msg332398 - (view) |
Author: Terry J. Reedy (terry.reedy) *  |
Date: 2018-12-23 22:47 |
The specific bug being fixed here is that wrapped lines before newline are ignored because after
if s[pos] == '\n':
linecount += 1
current_column = 0
this block
if current_column > 0:
lines, column = divmod(current_column - 1, linewidth)
linecount += lines
current_column = column + 1
has no effect. The fix, in the PR, is to put the linecount increment in the \n block before resetting current_column. (Since the column>0 block also has no net effect after \t, I removed it)
|
msg332492 - (view) |
Author: Tal Einat (taleinat) *  |
Date: 2018-12-24 16:37 |
New changeset 44a79cc5b3d1fb0c03c99077aa26def85ec26c67 by Tal Einat in branch 'master':
bpo-35208: Fix IDLE Squeezer line counting (GH-10449)
https://github.com/python/cpython/commit/44a79cc5b3d1fb0c03c99077aa26def85ec26c67
|
msg332494 - (view) |
Author: miss-islington (miss-islington) |
Date: 2018-12-24 16:42 |
New changeset 0e0cc553ab4c234e583b410accc7069eb97e392a by Miss Islington (bot) in branch '3.7':
bpo-35208: Fix IDLE Squeezer line counting (GH-10449)
https://github.com/python/cpython/commit/0e0cc553ab4c234e583b410accc7069eb97e392a
|
|
Date |
User |
Action |
Args |
2022-04-11 14:59:08 | admin | set | github: 79389 |
2018-12-24 16:42:40 | miss-islington | set | nosy:
+ miss-islington messages:
+ msg332494
|
2018-12-24 16:37:42 | miss-islington | set | pull_requests:
+ pull_request10539 |
2018-12-24 16:37:12 | taleinat | set | messages:
+ msg332492 |
2018-12-24 12:22:01 | taleinat | set | status: open -> closed resolution: fixed stage: patch review -> resolved |
2018-12-23 22:47:03 | terry.reedy | set | messages:
+ msg332398 title: IDLE: Squeezed lines count ignores window width -> IDLE: Squeezed line count ignores wrapping before newline |
2018-12-23 11:15:34 | taleinat | set | messages:
+ msg332386 |
2018-12-21 05:13:30 | terry.reedy | set | messages:
+ msg332278 |
2018-12-15 18:37:55 | taleinat | set | messages:
+ msg331904 |
2018-12-15 03:18:12 | terry.reedy | set | messages:
+ msg331885 |
2018-12-14 06:57:57 | taleinat | set | messages:
+ msg331784 |
2018-12-14 04:29:25 | terry.reedy | set | messages:
+ msg331779 |
2018-12-13 13:06:28 | taleinat | set | messages:
+ msg331759 |
2018-12-13 13:05:36 | taleinat | set | messages:
+ msg331758 |
2018-12-13 12:52:30 | terry.reedy | set | messages:
+ msg331757 |
2018-12-13 11:39:28 | taleinat | set | messages:
+ msg331756 |
2018-12-13 05:43:09 | terry.reedy | set | messages:
+ msg331742 |
2018-12-12 23:44:01 | terry.reedy | set | messages:
+ msg331722 |
2018-12-11 21:35:07 | terry.reedy | set | versions:
- Python 3.6 |
2018-11-10 07:57:50 | taleinat | set | messages:
+ msg329602 |
2018-11-10 07:34:41 | taleinat | set | keywords:
+ patch stage: patch review pull_requests:
+ pull_request9724 |
2018-11-10 07:18:21 | taleinat | create | |