classification
Title: IDLE: Squeezed line count ignores wrapping before newline
Type: behavior Stage: resolved
Components: IDLE Versions: Python 3.8, Python 3.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: terry.reedy Nosy List: miss-islington, taleinat, terry.reedy
Priority: normal Keywords: patch

Created on 2018-11-10 07:18 by taleinat, last changed 2018-12-24 16:42 by miss-islington. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 10449 merged taleinat, 2018-11-10 07:34
PR 11305 merged miss-islington, 2018-12-24 16:37
Messages (17)
msg329600 - (view) Author: Tal Einat (taleinat) * (Python committer) 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) * (Python committer) Date: 2018-11-10 07:57
See PR GH-10449 with a fix.
msg331722 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) Date: 2018-12-13 13:05
As I mentioned when opening this issue: 'a'*200 (or 'a'*10000).
msg331759 - (view) Author: Tal Einat (taleinat) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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) * (Python committer) 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
History
Date User Action Args
2018-12-24 16:42:40miss-islingtonsetnosy: + miss-islington
messages: + msg332494
2018-12-24 16:37:42miss-islingtonsetpull_requests: + pull_request10539
2018-12-24 16:37:12taleinatsetmessages: + msg332492
2018-12-24 12:22:01taleinatsetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2018-12-23 22:47:03terry.reedysetmessages: + msg332398
title: IDLE: Squeezed lines count ignores window width -> IDLE: Squeezed line count ignores wrapping before newline
2018-12-23 11:15:34taleinatsetmessages: + msg332386
2018-12-21 05:13:30terry.reedysetmessages: + msg332278
2018-12-15 18:37:55taleinatsetmessages: + msg331904
2018-12-15 03:18:12terry.reedysetmessages: + msg331885
2018-12-14 06:57:57taleinatsetmessages: + msg331784
2018-12-14 04:29:25terry.reedysetmessages: + msg331779
2018-12-13 13:06:28taleinatsetmessages: + msg331759
2018-12-13 13:05:36taleinatsetmessages: + msg331758
2018-12-13 12:52:30terry.reedysetmessages: + msg331757
2018-12-13 11:39:28taleinatsetmessages: + msg331756
2018-12-13 05:43:09terry.reedysetmessages: + msg331742
2018-12-12 23:44:01terry.reedysetmessages: + msg331722
2018-12-11 21:35:07terry.reedysetversions: - Python 3.6
2018-11-10 07:57:50taleinatsetmessages: + msg329602
2018-11-10 07:34:41taleinatsetkeywords: + patch
stage: patch review
pull_requests: + pull_request9724
2018-11-10 07:18:21taleinatcreate