bug#27008: 26.0.50; auto-hscroll-mode and scroll-left

bug#27008: 26.0.50; auto-hscroll-mode and scroll-left

[Stephen Berman]

[-- Attachment #1: Type: text/plain, Size: 1508 bytes --]

When auto-hscroll-mode is set to `current-line' and scroll-left is
invoked with arguments ARG > 0 and SET-MINIMUM non-nil, then when the
current line is automatically horizontally scrolled, all other lines in
the buffer are scrolled back to logical BOL, i.e. SET-MINIMUM is ignored
(except on the current line).  To reproduce:

0. emacs -Q
1. Set auto-hscroll-mode to `current-line'.
2. Type `C-x C-f /path/to/hscroll-bug RET' (the attached file).
3. Type `M-x toggle-truncate-lines' and `M-: (scroll-left 32 t)'.
4. Type `C-p' repeatedly.
=> When point is on the third line, and for all subsequent vertical
   motion, all lines but the current one are displayed starting at BOL
   instead of column SET-MINIMUM.

(I found this bug because I sometimes use scroll-left with non-nil
SET-MINIMUM in the Gnus *Summary* buffer to see more of the article
Subject lines; the attached file hscroll-bug, used in the above recipe,
is taken from such a buffer.)

In GNU Emacs 26.0.50 (build 14, x86_64-pc-linux-gnu, GTK+ Version 3.22.8)
 of 2017-05-21 built on rosalinde
Repository revision: 08212929ba7052883bd506be320dfaaae5b68970
Windowing system distributor 'The X.Org Foundation', version 11.0.11901000

Configured using:
 'configure 'CFLAGS=-Og -g3''

Configured features:
XPM JPEG TIFF GIF PNG RSVG IMAGEMAGICK SOUND GPM DBUS GSETTINGS NOTIFY
ACL GNUTLS LIBXML2 FREETYPE M17N_FLT LIBOTF XFT ZLIB TOOLKIT_SCROLL_BARS
GTK3 X11

Important settings:
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix


[-- Attachment #2: hscroll-bug --]
[-- Type: application/octet-stream, Size: 1881 bytes --]

   (  56) Eli Zaretskii        │ master c189986: Add an optional arguments to string-trim
   (  74) Eli Zaretskii        │ master d35da21: Improve documentation of 'split-string'
   (  12) Michael Albinus      │ ▶ master updated (d35da21 -> 1b0ec9f)
   (  30) Michael Albinus      │ master 6de77cf 1/2: Fix a problem with OpenSSH 7 in Tramp
   (  56) Michael Albinus      │ master 1b0ec9f 2/2: Minor tweaks in tramp-tests.el
   (  85) Eli Zaretskii        │ master c1c6f16: Fix turning off whitespace-mode
   (  30) Eli Zaretskii        │ emacs-25 e80f6a2: Describe problems with Microsoft Intellipoint
   ( 117) Stefan Monnier       │ master b372e56: * lisp/emacs-lisp/package.el: Quote `package-desc' in docstrings
   (  24) Eli Zaretskii        │ master c7391db: ; * doc/emacs/files.texi (Auto Save Files): Fix a cross-reference.
   (  65) Paul Eggert          │ master 7ff8c5c: Minor .gitignore fixes
   (  35) Paul Eggert          │ master c1c8b67: Check that signed right shift is arithmetic
   (  16) Noam Postavsky       │ ▶ master updated (c1c8b67 -> acd58c9)
   ( 284) Noam Postavsky       │ master 267be4b 1/2: Refactor lisp eval result printing
   ( 379) Noam Postavsky       │ master acd58c9 2/2: Limit integers printed as characters (Bug#16828)
   (  27) Paul Eggert          │ master a5acb37: Port --enable-gcc-warnings to clang 3.9.1
   (  34) Paul Eggert          │ master 7d00410: Add handlerlist assertion to module code
   ( 234) Eli Zaretskii        │ master 1cbbece: Fix automatic hscrolling of only the current line
   ( 138) Eli Zaretskii        │ master 021430f: New commands: find-library-other-window, find-library-other-frame
   (  33) Eli Zaretskii        │ master 6c7bf03: Avoid crashes in GC due to unescaped characters warning
   ( 656) Philipp Stephani     │ master 31fded0: Reimplement module functions

bug#27008: 26.0.50; auto-hscroll-mode and scroll-left

[Eli Zaretskii]

> From: Stephen Berman <stephen.berman@gmx.net>
> Date: Sun, 21 May 2017 16:10:04 +0200
> 
> When auto-hscroll-mode is set to `current-line' and scroll-left is
> invoked with arguments ARG > 0 and SET-MINIMUM non-nil, then when the
> current line is automatically horizontally scrolled, all other lines in
> the buffer are scrolled back to logical BOL, i.e. SET-MINIMUM is ignored
> (except on the current line).  To reproduce:
> 
> 0. emacs -Q
> 1. Set auto-hscroll-mode to `current-line'.
> 2. Type `C-x C-f /path/to/hscroll-bug RET' (the attached file).
> 3. Type `M-x toggle-truncate-lines' and `M-: (scroll-left 32 t)'.
> 4. Type `C-p' repeatedly.
> => When point is on the third line, and for all subsequent vertical
>    motion, all lines but the current one are displayed starting at BOL
>    instead of column SET-MINIMUM.

I don't understand what you expected instead.  current-line hscrolling
is designed to be disabled when manual scrolling is used, so using
scroll-left is incompatible with automatic hscrolling and should have
disabled it.  If anything, I could understand a complaint that the
current line is still hscrolled in this recipe, but otherwise I think
your expectations are a tad too much; the effect you describe is more
or less what I intended to happen.

Technically, the minimum hscroll is implemented by the same code which
calculates the window's hscroll value upon redisplay, and in
current-line hscrolling that value affects only the current line, the
rest of the window is displayed as if the hscroll is zero.

bug#27008: 26.0.50; auto-hscroll-mode and scroll-left

[Stephen Berman]

On Sun, 21 May 2017 18:23:10 +0300 Eli Zaretskii <eliz@gnu.org> wrote:

>> From: Stephen Berman <stephen.berman@gmx.net>
>> Date: Sun, 21 May 2017 16:10:04 +0200
>> 
>> When auto-hscroll-mode is set to `current-line' and scroll-left is
>> invoked with arguments ARG > 0 and SET-MINIMUM non-nil, then when the
>> current line is automatically horizontally scrolled, all other lines in
>> the buffer are scrolled back to logical BOL, i.e. SET-MINIMUM is ignored
>> (except on the current line).  To reproduce:
>> 
>> 0. emacs -Q
>> 1. Set auto-hscroll-mode to `current-line'.
>> 2. Type `C-x C-f /path/to/hscroll-bug RET' (the attached file).
>> 3. Type `M-x toggle-truncate-lines' and `M-: (scroll-left 32 t)'.
>> 4. Type `C-p' repeatedly.
>> => When point is on the third line, and for all subsequent vertical
>>    motion, all lines but the current one are displayed starting at BOL
>>    instead of column SET-MINIMUM.
>
> I don't understand what you expected instead.  

I expected behavior similar to when auto-hscroll-mode is set to t and I
evaluate (scroll-left 32 t): in that case, automatic scrolling still
happens on long enough lines, but the minimum hscroll is respected.  So
with auto-hscroll-mode set to `current-line' I expected to get scrolling
of the current line and the rest of the lines would be displayed as if
the window's left edge was at column 32.

>                                                current-line hscrolling
> is designed to be disabled when manual scrolling is used, so using
> scroll-left is incompatible with automatic hscrolling and should have
> disabled it.  

Why should current-line hscrolling be different from window hscrolling
in this respect?  In other words, since automatic window hscrolling
still works when scroll-left is executed, why shouldn't current-line
hscrolling also work then?

>               If anything, I could understand a complaint that the
> current line is still hscrolled in this recipe, but otherwise I think
> your expectations are a tad too much; the effect you describe is more
> or less what I intended to happen.

There is an oddity that I doubt you intended: when point is at the left
window edge of the current line (e.g. via C-a), column-number-mode shows
it at column 0, though this is not displayed, and indeed typing C-n does
not advance point until it reaches column 33 (SET-MINIMUM + 1);
nevertheless, all the other lines are displayed from their respective
BOL, i.e. column 0 (as you point out below).  When scrolling vertically
through the buffer with C-p and C-n or the arrow keys, this looks very
strange.

> Technically, the minimum hscroll is implemented by the same code which
> calculates the window's hscroll value upon redisplay, and in
> current-line hscrolling that value affects only the current line, the
> rest of the window is displayed as if the hscroll is zero.

Is this necessary?  Why can't the other lines be displayed with hscroll
set to w->min_hscroll, as they are with auto-hscroll-mode set to t?

Steve Berman

bug#27008: 26.0.50; auto-hscroll-mode and scroll-left

[Eli Zaretskii]

> From: Stephen Berman <stephen.berman@gmx.net>
> Cc: 27008@debbugs.gnu.org
> Date: Sun, 21 May 2017 22:12:27 +0200
> 
> Why can't the other lines be displayed with hscroll set to
> w->min_hscroll, as they are with auto-hscroll-mode set to t?

Thanks, I've tried to implement this idea in the attached.  I won't
have enough time to test it, though, so please run with this applied
for a few days and see if there are any adverse effects.  If not, I
will push this.

diff --git a/src/xdisp.c b/src/xdisp.c
index ddb26b8..898eb6b 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -2890,8 +2890,19 @@ init_iterator (struct it *it, struct window *w,
     }
   else
     {
+      /* When hscrolling only the current line, don't apply the
+	 hscroll here, it will be applied by display_line when it gets
+	 to laying out the line showing point.  However, if the
+	 window's min_hscroll is positive, the user specified a lower
+	 bound for automatic hscrolling, so they expect the
+	 non-current lines to obey that hscroll amount.  */
       if (hscrolling_current_line_p (w))
-	it->first_visible_x = 0;
+	{
+	  if (w->min_hscroll > 0)
+	    it->first_visible_x = w->min_hscroll;
+	  else
+	    it->first_visible_x = 0;
+	}
       else
 	it->first_visible_x =
 	  window_hscroll_limited (w, it->f) * FRAME_COLUMN_WIDTH (it->f);

bug#27008: 26.0.50; auto-hscroll-mode and scroll-left

[Stephen Berman]

On Tue, 30 May 2017 17:56:07 +0300 Eli Zaretskii <eliz@gnu.org> wrote:

>> From: Stephen Berman <stephen.berman@gmx.net>
>> Cc: 27008@debbugs.gnu.org
>> Date: Sun, 21 May 2017 22:12:27 +0200
>> 
>> Why can't the other lines be displayed with hscroll set to
>> w->min_hscroll, as they are with auto-hscroll-mode set to t?
>
> Thanks, I've tried to implement this idea in the attached.  I won't
> have enough time to test it, though, so please run with this applied
> for a few days and see if there are any adverse effects.  If not, I
> will push this.

Thanks very much for pursuing this.  I updated my master branch, applied
your patch and rebuilt, but when I executed the recipe of my OP, I saw
the same effect with the patch as without it: only the current line
respects w->min_hscroll, the other are displayed from BOL.  I'd be happy
to try and help diagnose this further, if you can advise me what to do.

Steve Berman

bug#27008: 26.0.50; auto-hscroll-mode and scroll-left

[Eli Zaretskii]

> From: Stephen Berman <stephen.berman@gmx.net>
> Cc: 27008@debbugs.gnu.org
> Date: Tue, 30 May 2017 18:57:01 +0200
> 
> Thanks very much for pursuing this.  I updated my master branch, applied
> your patch and rebuilt, but when I executed the recipe of my OP, I saw
> the same effect with the patch as without it: only the current line
> respects w->min_hscroll, the other are displayed from BOL.  I'd be happy
> to try and help diagnose this further, if you can advise me what to do.

I don't have any advice, because your recipe worked for me after the
changes.  I'm confused now.

bug#27008: 26.0.50; auto-hscroll-mode and scroll-left

[Eli Zaretskii]

> Date: Tue, 30 May 2017 20:18:27 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: 27008@debbugs.gnu.org
> 
> > From: Stephen Berman <stephen.berman@gmx.net>
> > Cc: 27008@debbugs.gnu.org
> > Date: Tue, 30 May 2017 18:57:01 +0200
> > 
> > Thanks very much for pursuing this.  I updated my master branch, applied
> > your patch and rebuilt, but when I executed the recipe of my OP, I saw
> > the same effect with the patch as without it: only the current line
> > respects w->min_hscroll, the other are displayed from BOL.  I'd be happy
> > to try and help diagnose this further, if you can advise me what to do.
> 
> I don't have any advice, because your recipe worked for me after the
> changes.  I'm confused now.

Ah, I see: I sent the wrong patch.  Here's the right one:

diff --git a/src/xdisp.c b/src/xdisp.c
index ddb26b8..3ccd035 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -2890,8 +2890,19 @@ init_iterator (struct it *it, struct window *w,
     }
   else
     {
+      /* When hscrolling only the current line, don't apply the
+	 hscroll here, it will be applied by display_line when it gets
+	 to laying out the line showing point.  However, if the
+	 window's min_hscroll is positive, the user specified a lower
+	 bound for automatic hscrolling, so they expect the
+	 non-current lines to obey that hscroll amount.  */
       if (hscrolling_current_line_p (w))
-	it->first_visible_x = 0;
+	{
+	  if (w->min_hscroll > 0)
+	    it->first_visible_x = w->min_hscroll * FRAME_COLUMN_WIDTH (it->f);
+	  else
+	    it->first_visible_x = 0;
+	}
       else
 	it->first_visible_x =
 	  window_hscroll_limited (w, it->f) * FRAME_COLUMN_WIDTH (it->f);

bug#27008: 26.0.50; auto-hscroll-mode and scroll-left

[Stephen Berman]

On Tue, 30 May 2017 20:31:38 +0300 Eli Zaretskii <eliz@gnu.org> wrote:

>> Date: Tue, 30 May 2017 20:18:27 +0300
>> From: Eli Zaretskii <eliz@gnu.org>
>> Cc: 27008@debbugs.gnu.org
>> 
>> > From: Stephen Berman <stephen.berman@gmx.net>
>> > Cc: 27008@debbugs.gnu.org
>> > Date: Tue, 30 May 2017 18:57:01 +0200
>> > 
>> > Thanks very much for pursuing this.  I updated my master branch, applied
>> > your patch and rebuilt, but when I executed the recipe of my OP, I saw
>> > the same effect with the patch as without it: only the current line
>> > respects w->min_hscroll, the other are displayed from BOL.  I'd be happy
>> > to try and help diagnose this further, if you can advise me what to do.
>> 
>> I don't have any advice, because your recipe worked for me after the
>> changes.  I'm confused now.
>
> Ah, I see: I sent the wrong patch.  Here's the right one:

Thanks.  Yes, with this patch the non-current lines are displayed from
w->min_hscroll.  However, there's a new problem: now when points moves
to a line, that line automatically scrolls further by w->min_hscroll.
So when I do `(scroll-left 32 t)' and then move point to another line,
that line scrolls further left so that its column 64 is on the left edge
of the window (the other lines remain displayed starting at column 32).
When point moves to the next line, that one scrolls further to column 64
and the previous one goes back to being displayed from column 32.

Steve Berman

bug#27008: 26.0.50; auto-hscroll-mode and scroll-left

[Eli Zaretskii]

> From: Stephen Berman <stephen.berman@gmx.net>
> Cc: 27008@debbugs.gnu.org
> Date: Tue, 30 May 2017 21:45:47 +0200
> 
> Thanks.  Yes, with this patch the non-current lines are displayed from
> w->min_hscroll.  However, there's a new problem: now when points moves
> to a line, that line automatically scrolls further by w->min_hscroll.
> So when I do `(scroll-left 32 t)' and then move point to another line,
> that line scrolls further left so that its column 64 is on the left edge
> of the window (the other lines remain displayed starting at column 32).
> When point moves to the next line, that one scrolls further to column 64
> and the previous one goes back to being displayed from column 32.

Does the below (to be applied on top of current master, i.e. first
revert the previous patch) fix this?

diff --git a/src/xdisp.c b/src/xdisp.c
index c03689b..c96ffce 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -2890,8 +2890,19 @@ init_iterator (struct it *it, struct window *w,
     }
   else
     {
+      /* When hscrolling only the current line, don't apply the
+	 hscroll here, it will be applied by display_line when it gets
+	 to laying out the line showing point.  However, if the
+	 window's min_hscroll is positive, the user specified a lower
+	 bound for automatic hscrolling, so they expect the
+	 non-current lines to obey that hscroll amount.  */
       if (hscrolling_current_line_p (w))
-	it->first_visible_x = 0;
+	{
+	  if (w->min_hscroll > 0)
+	    it->first_visible_x = w->min_hscroll * FRAME_COLUMN_WIDTH (it->f);
+	  else
+	    it->first_visible_x = 0;
+	}
       else
 	it->first_visible_x =
 	  window_hscroll_limited (w, it->f) * FRAME_COLUMN_WIDTH (it->f);
@@ -13099,7 +13110,9 @@ hscroll_window_tree (Lisp_Object window)
 		     that doesn't need to be hscrolled.  If we omit
 		     this condition, the line from which we move will
 		     remain hscrolled.  */
-		  || (hscl && w->hscroll && !cursor_row->truncated_on_left_p)))
+		  || (hscl
+		      && w->hscroll != w->min_hscroll
+		      && !cursor_row->truncated_on_left_p)))
 	    {
 	      struct it it;
 	      ptrdiff_t hscroll;
@@ -20710,7 +20723,9 @@ display_line (struct it *it, int cursor_vpos)
   /* If we are going to display the cursor's line, account for the
      hscroll of that line.  */
   if (hscroll_this_line)
-    x_incr = window_hscroll_limited (it->w, it->f) * FRAME_COLUMN_WIDTH (it->f);
+    x_incr =
+      (window_hscroll_limited (it->w, it->f) - it->w->min_hscroll)
+      * FRAME_COLUMN_WIDTH (it->f);
 
   /* Move over display elements that are not visible because we are
      hscrolled.  This may stop at an x-position < first_visible_x

bug#27008: 26.0.50; auto-hscroll-mode and scroll-left

[Stephen Berman]

On Wed, 31 May 2017 10:14:41 +0300 Eli Zaretskii <eliz@gnu.org> wrote:

>> From: Stephen Berman <stephen.berman@gmx.net>
>> Cc: 27008@debbugs.gnu.org
>> Date: Tue, 30 May 2017 21:45:47 +0200
>> 
>> Thanks.  Yes, with this patch the non-current lines are displayed from
>> w->min_hscroll.  However, there's a new problem: now when points moves
>> to a line, that line automatically scrolls further by w->min_hscroll.
>> So when I do `(scroll-left 32 t)' and then move point to another line,
>> that line scrolls further left so that its column 64 is on the left edge
>> of the window (the other lines remain displayed starting at column 32).
>> When point moves to the next line, that one scrolls further to column 64
>> and the previous one goes back to being displayed from column 32.
>
> Does the below (to be applied on top of current master, i.e. first
> revert the previous patch) fix this?

Yes, the display and the horizontal scrolling is now exactly as I hoped
it would be; many thanks!  

There is one minor issue: with auto-hscroll-mode set to `current-line'
and (scroll-left 32 t), when I scroll vertically by holding down <down>
or C-n or <up> or C-p, the motion is less smooth than usual.  With the
same auto-hscroll-mode setting but no scroll-left, vertical scrolling is
smoother, but still not as smooth as with auto-hscroll-mode set to t or
nil (with either of these settings, vertical scrolling seems slightly
less smooth with (scroll-left 32 t) than with no scroll-left, though the
difference is less noticeable than with auto-hscroll-mode set to
current-line).  But I don't think this is a serious annoyance, so unless
you see an easy fix for it, I think you should commit the patch as is to
master.

Steve Berman

bug#27008: 26.0.50; auto-hscroll-mode and scroll-left

[Eli Zaretskii]

> From: Stephen Berman <stephen.berman@gmx.net>
> Cc: 27008@debbugs.gnu.org
> Date: Wed, 31 May 2017 16:18:26 +0200
> 
> There is one minor issue: with auto-hscroll-mode set to `current-line'
> and (scroll-left 32 t), when I scroll vertically by holding down <down>
> or C-n or <up> or C-p, the motion is less smooth than usual.  With the
> same auto-hscroll-mode setting but no scroll-left, vertical scrolling is
> smoother, but still not as smooth as with auto-hscroll-mode set to t or
> nil (with either of these settings, vertical scrolling seems slightly
> less smooth with (scroll-left 32 t) than with no scroll-left, though the
> difference is less noticeable than with auto-hscroll-mode set to
> current-line).

The display engine is working harder when this mode is on, and some
optimizations are disabled.  I didn't see a way of implementing this
that would avoid more expensive redisplay.  Sorry.

> But I don't think this is a serious annoyance, so unless you see an
> easy fix for it, I think you should commit the patch as is to
> master.

OK, thanks, will do.

bug#27008: 26.0.50; auto-hscroll-mode and scroll-left

[Eli Zaretskii]

> Date: Wed, 31 May 2017 17:39:08 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: 27008@debbugs.gnu.org
> 
> > But I don't think this is a serious annoyance, so unless you see an
> > easy fix for it, I think you should commit the patch as is to
> > master.
> 
> OK, thanks, will do.

Done.