bug#31325: 27.0.50; PROPOSAL: introduce a new function to recenter without redisplaying the frame

bug#31325: 27.0.50; PROPOSAL: introduce a new function to recenter without redisplaying the frame

[John Shahid]

This BUG is a continuation of this help-gnu-emacs mailing list
thread [1]. As mentioned in the thread, I ran into a terminal flickering
issue in flycheck which turns out to be caused by `recenter'
redisplaying the frame when `recenter-redisplay' value is either `t' or
`tty'. I agree with the arguments in the thread to maintain the backward
compatible behavior. Instead, I'm proposing one of the following
alternatives:

1. introduce a new lisp function for recentering and discourage the use
of `recenter' from lisp (i.e. declaring it `interactive-only'), or
2. add a new `recenter-and-redisplay' and bound it to C-l

Also, Stefan's suggestion/possible solution:

3. adding an argument to `recenter' to control the redisplay behavior

[1]: http://lists.gnu.org/archive/html/help-gnu-emacs/2018-04/msg00326.html

In GNU Emacs 27.0.50 (build 1, x86_64-pc-linux-gnu)
 of 2018-04-16 built on 043fb36e1435
Repository revision: 6ec3ec7fbec2c04d5e9570b0470217cd97e5e7e6
System Description: Ubuntu 16.04.4 LTS

Configured using:
 'configure --without-x'

Configured features:
SOUND NOTIFY GNUTLS LIBXML2 ZLIB THREADS

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

bug#31325: 27.0.50; PROPOSAL: introduce a new function to recenter without redisplaying the frame

[Stefan Monnier]

> This BUG is a continuation of this help-gnu-emacs mailing list
> thread [1]. As mentioned in the thread, I ran into a terminal flickering
> issue in flycheck which turns out to be caused by `recenter'
> redisplaying the frame when `recenter-redisplay' value is either `t' or
> `tty'. I agree with the arguments in the thread to maintain the backward
> compatible behavior. Instead, I'm proposing one of the following
> alternatives:

Indeed, most/all uses from Elisp shouldn't redisplay.

> 1. introduce a new lisp function for recentering and discourage the use
> of `recenter' from lisp (i.e. declaring it `interactive-only'), or
> 2. add a new `recenter-and-redisplay' and bound it to C-l
> Also, Stefan's suggestion/possible solution:
> 3. adding an argument to `recenter' to control the redisplay behavior

Either of those is OK.  The benefit of the 3rd is that it would likely
"magically fix" 99% of the existing uses (I'm thinking of adding an
optional argument which we could call `and-redisplay` which when non-nil
tells recenter to do a redisplay, so all existing Elisp calls would be
implicitly modified not to cause a redisplay).


        Stefan

bug#31325: 27.0.50; PROPOSAL: introduce a new function to recenter without redisplaying the frame

[John Shahid]

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


Stefan Monnier <monnier@iro.umontreal.ca> writes:

>> This BUG is a continuation of this help-gnu-emacs mailing list
>> thread [1]. As mentioned in the thread, I ran into a terminal flickering
>> issue in flycheck which turns out to be caused by `recenter'
>> redisplaying the frame when `recenter-redisplay' value is either `t' or
>> `tty'. I agree with the arguments in the thread to maintain the backward
>> compatible behavior. Instead, I'm proposing one of the following
>> alternatives:
>
> Indeed, most/all uses from Elisp shouldn't redisplay.
>
>> 1. introduce a new lisp function for recentering and discourage the use
>> of `recenter' from lisp (i.e. declaring it `interactive-only'), or
>> 2. add a new `recenter-and-redisplay' and bound it to C-l
>> Also, Stefan's suggestion/possible solution:
>> 3. adding an argument to `recenter' to control the redisplay behavior
>
> Either of those is OK.  The benefit of the 3rd is that it would likely
> "magically fix" 99% of the existing uses (I'm thinking of adding an
> optional argument which we could call `and-redisplay` which when non-nil
> tells recenter to do a redisplay, so all existing Elisp calls would be
> implicitly modified not to cause a redisplay).

I ended up going with option 3. I also used `redisplay` instead of
`and-redisplay`. I felt the `and` is redundant, not sure how strongly
you feel about the name.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Add-a-new-argument-to-recenter-to-allow-finer-contro.patch --]
[-- Type: text/x-diff, Size: 3683 bytes --]

From a8c544c40f1d01544ced20fad3e81a231f5715c7 Mon Sep 17 00:00:00 2001
From: John Shahid <jvshahid@gmail.com>
Date: Thu, 28 Jun 2018 09:13:45 -0400
Subject: [PATCH] Add a new argument to `recenter' to allow finer control of
 redisplay

* window.c (recenter): add a new REDISPLAY argument to allow the
  caller to control the redisplay behavior. `recenter' will only
  redisplay the frame if this new arg and `recenter-redisplay' are
  both non-nil.
* window.el (recenter-top-bottom): pass an extra non-nil argument to
  `recenter' to force a redisplay
---
 lisp/window.el | 10 +++++-----
 src/window.c   | 22 ++++++++++++----------
 2 files changed, 17 insertions(+), 15 deletions(-)

diff --git a/lisp/window.el b/lisp/window.el
index fdd510401d..6d9d8bdcd2 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -8778,15 +8778,15 @@ recenter-top-bottom
 	   (min (max 0 scroll-margin)
 		(truncate (/ (window-body-height) 4.0)))))
       (cond ((eq recenter-last-op 'middle)
-	     (recenter))
+	     (recenter nil t))
 	    ((eq recenter-last-op 'top)
-	     (recenter this-scroll-margin))
+	     (recenter this-scroll-margin t))
 	    ((eq recenter-last-op 'bottom)
-	     (recenter (- -1 this-scroll-margin)))
+	     (recenter (- -1 this-scroll-margin) t))
 	    ((integerp recenter-last-op)
-	     (recenter recenter-last-op))
+	     (recenter recenter-last-op t))
 	    ((floatp recenter-last-op)
-	     (recenter (round (* recenter-last-op (window-height))))))))))
+	     (recenter (round (* recenter-last-op (window-height))) t)))))))
 
 (define-key global-map [?\C-l] 'recenter-top-bottom)
 
diff --git a/src/window.c b/src/window.c
index 81fd7f2b47..fcdb0d444e 100644
--- a/src/window.c
+++ b/src/window.c
@@ -5898,22 +5898,23 @@ displayed_window_lines (struct window *w)
 }
 
 
-DEFUN ("recenter", Frecenter, Srecenter, 0, 1, "P",
+DEFUN ("recenter", Frecenter, Srecenter, 0, 2, "P",
        doc: /* Center point in selected window and maybe redisplay frame.
 With a numeric prefix argument ARG, recenter putting point on screen line ARG
 relative to the selected window.  If ARG is negative, it counts up from the
 bottom of the window.  (ARG should be less than the height of the window.)
 
-If ARG is omitted or nil, then recenter with point on the middle line of
-the selected window; if the variable `recenter-redisplay' is non-nil,
-also erase the entire frame and redraw it (when `auto-resize-tool-bars'
-is set to `grow-only', this resets the tool-bar's height to the minimum
-height needed); if `recenter-redisplay' has the special value `tty',
-then only tty frames are redrawn.
+If ARG is omitted or nil, then recenter with point on the middle line
+of the selected window; if REDISPLAY & `recenter-redisplay' are
+non-nil, also erase the entire frame and redraw it (when
+`auto-resize-tool-bars' is set to `grow-only', this resets the
+tool-bar's height to the minimum height needed); if
+`recenter-redisplay' has the special value `tty', then only tty frames
+are redrawn.
 
 Just C-u as prefix means put point in the center of the window
 and redisplay normally--don't erase and redraw the frame.  */)
-  (register Lisp_Object arg)
+  (register Lisp_Object arg, register Lisp_Object redisplay)
 {
   struct window *w = XWINDOW (selected_window);
   struct buffer *buf = XBUFFER (w->contents);
@@ -5933,8 +5934,9 @@ and redisplay normally--don't erase and redraw the frame.  */)
 
   if (NILP (arg))
     {
-      if (!NILP (Vrecenter_redisplay)
-	  && (!EQ (Vrecenter_redisplay, Qtty)
+      if (!NILP (redisplay)
+	  && !NILP (Vrecenter_redisplay)
+	  && (!EQ (redisplay, Qtty)
 	      || !NILP (Ftty_type (selected_frame))))
 	{
 	  ptrdiff_t i;
-- 
2.18.0

bug#31325: 27.0.50; PROPOSAL: introduce a new function to recenter without redisplaying the frame

[Eli Zaretskii]

> From: John Shahid <jvshahid@gmail.com>
> Date: Thu, 28 Jun 2018 13:27:34 +0000
> Cc: 31325@debbugs.gnu.org
> 
> >> 1. introduce a new lisp function for recentering and discourage the use
> >> of `recenter' from lisp (i.e. declaring it `interactive-only'), or
> >> 2. add a new `recenter-and-redisplay' and bound it to C-l
> >> Also, Stefan's suggestion/possible solution:
> >> 3. adding an argument to `recenter' to control the redisplay behavior
> >
> > Either of those is OK.  The benefit of the 3rd is that it would likely
> > "magically fix" 99% of the existing uses (I'm thinking of adding an
> > optional argument which we could call `and-redisplay` which when non-nil
> > tells recenter to do a redisplay, so all existing Elisp calls would be
> > implicitly modified not to cause a redisplay).
> 
> I ended up going with option 3. I also used `redisplay` instead of
> `and-redisplay`. I felt the `and` is redundant, not sure how strongly
> you feel about the name.

Thanks, but please also include a NEWS entry announcing the change,
and a suitable change for the ELisp manual.

And I have a few comments:

> -  (register Lisp_Object arg)
> +  (register Lisp_Object arg, register Lisp_Object redisplay)

I believe nowadays we remove the 'register' qualifiers whenever we
change code that uses them.

>  {
>    struct window *w = XWINDOW (selected_window);
>    struct buffer *buf = XBUFFER (w->contents);
> @@ -5933,8 +5934,9 @@ and redisplay normally--don't erase and redraw the frame.  */)
>  
>    if (NILP (arg))
>      {
> -      if (!NILP (Vrecenter_redisplay)
> -	  && (!EQ (Vrecenter_redisplay, Qtty)
> +      if (!NILP (redisplay)
> +	  && !NILP (Vrecenter_redisplay)
> +	  && (!EQ (redisplay, Qtty)
                   ^^^^^^^^^
This should have been Vrecenter_redisplay, right?

bug#31325: 27.0.50; PROPOSAL: introduce a new function to recenter without redisplaying the frame

[John Shahid]

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


Eli Zaretskii <eliz@gnu.org> writes:

>> From: John Shahid <jvshahid@gmail.com>
>> Date: Thu, 28 Jun 2018 13:27:34 +0000
>> Cc: 31325@debbugs.gnu.org
>> 
>> >> 1. introduce a new lisp function for recentering and discourage the use
>> >> of `recenter' from lisp (i.e. declaring it `interactive-only'), or
>> >> 2. add a new `recenter-and-redisplay' and bound it to C-l
>> >> Also, Stefan's suggestion/possible solution:
>> >> 3. adding an argument to `recenter' to control the redisplay behavior
>> >
>> > Either of those is OK.  The benefit of the 3rd is that it would likely
>> > "magically fix" 99% of the existing uses (I'm thinking of adding an
>> > optional argument which we could call `and-redisplay` which when non-nil
>> > tells recenter to do a redisplay, so all existing Elisp calls would be
>> > implicitly modified not to cause a redisplay).
>> 
>> I ended up going with option 3. I also used `redisplay` instead of
>> `and-redisplay`. I felt the `and` is redundant, not sure how strongly
>> you feel about the name.
>
> Thanks, but please also include a NEWS entry announcing the change,
> and a suitable change for the ELisp manual.
>
> And I have a few comments:
>
>> -  (register Lisp_Object arg)
>> +  (register Lisp_Object arg, register Lisp_Object redisplay)
>
> I believe nowadays we remove the 'register' qualifiers whenever we
> change code that uses them.
>
>>  {
>>    struct window *w = XWINDOW (selected_window);
>>    struct buffer *buf = XBUFFER (w->contents);
>> @@ -5933,8 +5934,9 @@ and redisplay normally--don't erase and redraw the frame.  */)
>>  
>>    if (NILP (arg))
>>      {
>> -      if (!NILP (Vrecenter_redisplay)
>> -	  && (!EQ (Vrecenter_redisplay, Qtty)
>> +      if (!NILP (redisplay)
>> +	  && !NILP (Vrecenter_redisplay)
>> +	  && (!EQ (redisplay, Qtty)
>                    ^^^^^^^^^
> This should have been Vrecenter_redisplay, right?

fixed all the issues mentioned above in the attached patch and
documented the change in the NEWS file and the manual.

cheers,


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Add-a-new-argument-to-recenter-to-allow-finer-contro.patch --]
[-- Type: text/x-diff, Size: 5917 bytes --]

From 1b7380df866e3ca7c1a940a92c36f43ce8e5043f Mon Sep 17 00:00:00 2001
From: John Shahid <jvshahid@gmail.com>
Date: Thu, 28 Jun 2018 09:13:45 -0400
Subject: [PATCH] Add a new argument to `recenter' to allow finer control of
 redisplay

* window.c (recenter): add a new REDISPLAY argument to allow the
  caller to control the redisplay behavior. `recenter' will only
  redisplay the frame if this new arg and `recenter-redisplay' are
  both non-nil.
* window.el (recenter-top-bottom): pass an extra non-nil argument to
  `recenter' to force a redisplay
---
 doc/lispref/windows.texi | 12 +++++++-----
 etc/NEWS                 |  5 +++++
 lisp/window.el           | 10 +++++-----
 src/window.c             | 20 +++++++++++---------
 4 files changed, 28 insertions(+), 19 deletions(-)

diff --git a/doc/lispref/windows.texi b/doc/lispref/windows.texi
index 5497759595..9740bbebf2 100644
--- a/doc/lispref/windows.texi
+++ b/doc/lispref/windows.texi
@@ -4138,7 +4138,7 @@ Textual Scrolling
 only if point is already on that position do they signal an error.
 @end defopt
 
-@deffn Command recenter &optional count
+@deffn Command recenter &optional count redisplay
 @cindex centering point
 This function scrolls the text in the selected window so that point is
 displayed at a specified vertical position within the window.  It does
@@ -4152,8 +4152,9 @@ Textual Scrolling
 
 If @var{count} is @code{nil} (or a non-@code{nil} list),
 @code{recenter} puts the line containing point in the middle of the
-window.  If @var{count} is @code{nil}, this function may redraw the
-frame, according to the value of @code{recenter-redisplay}.
+window.  If @var{count} is @code{nil} and @var{redisplay} is
+non-@code{nil}, this function may redraw the frame, according to the
+value of @code{recenter-redisplay}.
 
 When @code{recenter} is called interactively, @var{count} is the raw
 prefix argument.  Thus, typing @kbd{C-u} as the prefix sets the
@@ -4181,8 +4182,9 @@ Textual Scrolling
 
 @defopt recenter-redisplay
 If this variable is non-@code{nil}, calling @code{recenter} with a
-@code{nil} argument redraws the frame.  The default value is
-@code{tty}, which means only redraw the frame if it is a tty frame.
+@code{nil} @var{count} argument and non-@code{nil} @var{redisplay}
+argument redraws the frame.  The default value is @code{tty}, which
+means only redraw the frame if it is a tty frame.
 @end defopt
 
 @deffn Command recenter-top-bottom &optional count
diff --git a/etc/NEWS b/etc/NEWS
index 12757f61d2..8adb962120 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -658,6 +658,11 @@ manual for more details.
 \f
 * Lisp Changes in Emacs 27.1
 
++++
+** The function 'recenter' accepts an additional optional argument.
+If the optional second argument is nil, recenter will not redisplay
+the frame regardless of the value of 'recenter-redisplay'.
+
 +++
 ** New functions 'major-mode-suspend' and 'major-mode-restore'.
 Use them when switching temporarily to another major mode, e.g. for
diff --git a/lisp/window.el b/lisp/window.el
index fdd510401d..6d9d8bdcd2 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -8778,15 +8778,15 @@ recenter-top-bottom
 	   (min (max 0 scroll-margin)
 		(truncate (/ (window-body-height) 4.0)))))
       (cond ((eq recenter-last-op 'middle)
-	     (recenter))
+	     (recenter nil t))
 	    ((eq recenter-last-op 'top)
-	     (recenter this-scroll-margin))
+	     (recenter this-scroll-margin t))
 	    ((eq recenter-last-op 'bottom)
-	     (recenter (- -1 this-scroll-margin)))
+	     (recenter (- -1 this-scroll-margin) t))
 	    ((integerp recenter-last-op)
-	     (recenter recenter-last-op))
+	     (recenter recenter-last-op t))
 	    ((floatp recenter-last-op)
-	     (recenter (round (* recenter-last-op (window-height))))))))))
+	     (recenter (round (* recenter-last-op (window-height))) t)))))))
 
 (define-key global-map [?\C-l] 'recenter-top-bottom)
 
diff --git a/src/window.c b/src/window.c
index 81fd7f2b47..a966e99ca8 100644
--- a/src/window.c
+++ b/src/window.c
@@ -5898,22 +5898,23 @@ displayed_window_lines (struct window *w)
 }
 
 
-DEFUN ("recenter", Frecenter, Srecenter, 0, 1, "P",
+DEFUN ("recenter", Frecenter, Srecenter, 0, 2, "P",
        doc: /* Center point in selected window and maybe redisplay frame.
 With a numeric prefix argument ARG, recenter putting point on screen line ARG
 relative to the selected window.  If ARG is negative, it counts up from the
 bottom of the window.  (ARG should be less than the height of the window.)
 
-If ARG is omitted or nil, then recenter with point on the middle line of
-the selected window; if the variable `recenter-redisplay' is non-nil,
-also erase the entire frame and redraw it (when `auto-resize-tool-bars'
-is set to `grow-only', this resets the tool-bar's height to the minimum
-height needed); if `recenter-redisplay' has the special value `tty',
-then only tty frames are redrawn.
+If ARG is omitted or nil, then recenter with point on the middle line
+of the selected window; if REDISPLAY & `recenter-redisplay' are
+non-nil, also erase the entire frame and redraw it (when
+`auto-resize-tool-bars' is set to `grow-only', this resets the
+tool-bar's height to the minimum height needed); if
+`recenter-redisplay' has the special value `tty', then only tty frames
+are redrawn.
 
 Just C-u as prefix means put point in the center of the window
 and redisplay normally--don't erase and redraw the frame.  */)
-  (register Lisp_Object arg)
+  (Lisp_Object arg, Lisp_Object redisplay)
 {
   struct window *w = XWINDOW (selected_window);
   struct buffer *buf = XBUFFER (w->contents);
@@ -5933,7 +5934,8 @@ and redisplay normally--don't erase and redraw the frame.  */)
 
   if (NILP (arg))
     {
-      if (!NILP (Vrecenter_redisplay)
+      if (!NILP (redisplay)
+	  && !NILP (Vrecenter_redisplay)
 	  && (!EQ (Vrecenter_redisplay, Qtty)
 	      || !NILP (Ftty_type (selected_frame))))
 	{
-- 
2.18.0

bug#31325: 27.0.50; PROPOSAL: introduce a new function to recenter without redisplaying the frame

[martin rudalics]

 > * window.c (recenter): add a new REDISPLAY argument to allow the

Please capitalize the word after the colon ...

 >   caller to control the redisplay behavior. `recenter' will only

.. and use two spaces when beginning a new sentence.

Thank you, martin

bug#31325: 27.0.50; PROPOSAL: introduce a new function to recenter without redisplaying the frame

[John Shahid]

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


martin rudalics <rudalics@gmx.at> writes:

>> * window.c (recenter): add a new REDISPLAY argument to allow the
>
> Please capitalize the word after the colon ...
>
>>   caller to control the redisplay behavior. `recenter' will only
>
> .. and use two spaces when beginning a new sentence.
>
> Thank you, martin

Fixed in the attached patch.

Cheers,


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Add-a-new-argument-to-recenter-to-allow-finer-contro.patch --]
[-- Type: text/x-diff, Size: 5918 bytes --]

From c945faf3774ea8d331ced94b80422dfaebd83018 Mon Sep 17 00:00:00 2001
From: John Shahid <jvshahid@gmail.com>
Date: Thu, 28 Jun 2018 09:13:45 -0400
Subject: [PATCH] Add a new argument to `recenter' to allow finer control of
 redisplay

* window.c (recenter): Add a new REDISPLAY argument to allow the
  caller to control the redisplay behavior.  `recenter' will only
  redisplay the frame if this new arg and `recenter-redisplay' are
  both non-nil.
* window.el (recenter-top-bottom): pass an extra non-nil argument to
  `recenter' to force a redisplay
---
 doc/lispref/windows.texi | 12 +++++++-----
 etc/NEWS                 |  5 +++++
 lisp/window.el           | 10 +++++-----
 src/window.c             | 20 +++++++++++---------
 4 files changed, 28 insertions(+), 19 deletions(-)

diff --git a/doc/lispref/windows.texi b/doc/lispref/windows.texi
index 5497759595..9740bbebf2 100644
--- a/doc/lispref/windows.texi
+++ b/doc/lispref/windows.texi
@@ -4138,7 +4138,7 @@ Textual Scrolling
 only if point is already on that position do they signal an error.
 @end defopt
 
-@deffn Command recenter &optional count
+@deffn Command recenter &optional count redisplay
 @cindex centering point
 This function scrolls the text in the selected window so that point is
 displayed at a specified vertical position within the window.  It does
@@ -4152,8 +4152,9 @@ Textual Scrolling
 
 If @var{count} is @code{nil} (or a non-@code{nil} list),
 @code{recenter} puts the line containing point in the middle of the
-window.  If @var{count} is @code{nil}, this function may redraw the
-frame, according to the value of @code{recenter-redisplay}.
+window.  If @var{count} is @code{nil} and @var{redisplay} is
+non-@code{nil}, this function may redraw the frame, according to the
+value of @code{recenter-redisplay}.
 
 When @code{recenter} is called interactively, @var{count} is the raw
 prefix argument.  Thus, typing @kbd{C-u} as the prefix sets the
@@ -4181,8 +4182,9 @@ Textual Scrolling
 
 @defopt recenter-redisplay
 If this variable is non-@code{nil}, calling @code{recenter} with a
-@code{nil} argument redraws the frame.  The default value is
-@code{tty}, which means only redraw the frame if it is a tty frame.
+@code{nil} @var{count} argument and non-@code{nil} @var{redisplay}
+argument redraws the frame.  The default value is @code{tty}, which
+means only redraw the frame if it is a tty frame.
 @end defopt
 
 @deffn Command recenter-top-bottom &optional count
diff --git a/etc/NEWS b/etc/NEWS
index 12757f61d2..8adb962120 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -658,6 +658,11 @@ manual for more details.
 \f
 * Lisp Changes in Emacs 27.1
 
++++
+** The function 'recenter' accepts an additional optional argument.
+If the optional second argument is nil, recenter will not redisplay
+the frame regardless of the value of 'recenter-redisplay'.
+
 +++
 ** New functions 'major-mode-suspend' and 'major-mode-restore'.
 Use them when switching temporarily to another major mode, e.g. for
diff --git a/lisp/window.el b/lisp/window.el
index fdd510401d..6d9d8bdcd2 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -8778,15 +8778,15 @@ recenter-top-bottom
 	   (min (max 0 scroll-margin)
 		(truncate (/ (window-body-height) 4.0)))))
       (cond ((eq recenter-last-op 'middle)
-	     (recenter))
+	     (recenter nil t))
 	    ((eq recenter-last-op 'top)
-	     (recenter this-scroll-margin))
+	     (recenter this-scroll-margin t))
 	    ((eq recenter-last-op 'bottom)
-	     (recenter (- -1 this-scroll-margin)))
+	     (recenter (- -1 this-scroll-margin) t))
 	    ((integerp recenter-last-op)
-	     (recenter recenter-last-op))
+	     (recenter recenter-last-op t))
 	    ((floatp recenter-last-op)
-	     (recenter (round (* recenter-last-op (window-height))))))))))
+	     (recenter (round (* recenter-last-op (window-height))) t)))))))
 
 (define-key global-map [?\C-l] 'recenter-top-bottom)
 
diff --git a/src/window.c b/src/window.c
index 81fd7f2b47..a966e99ca8 100644
--- a/src/window.c
+++ b/src/window.c
@@ -5898,22 +5898,23 @@ displayed_window_lines (struct window *w)
 }
 
 
-DEFUN ("recenter", Frecenter, Srecenter, 0, 1, "P",
+DEFUN ("recenter", Frecenter, Srecenter, 0, 2, "P",
        doc: /* Center point in selected window and maybe redisplay frame.
 With a numeric prefix argument ARG, recenter putting point on screen line ARG
 relative to the selected window.  If ARG is negative, it counts up from the
 bottom of the window.  (ARG should be less than the height of the window.)
 
-If ARG is omitted or nil, then recenter with point on the middle line of
-the selected window; if the variable `recenter-redisplay' is non-nil,
-also erase the entire frame and redraw it (when `auto-resize-tool-bars'
-is set to `grow-only', this resets the tool-bar's height to the minimum
-height needed); if `recenter-redisplay' has the special value `tty',
-then only tty frames are redrawn.
+If ARG is omitted or nil, then recenter with point on the middle line
+of the selected window; if REDISPLAY & `recenter-redisplay' are
+non-nil, also erase the entire frame and redraw it (when
+`auto-resize-tool-bars' is set to `grow-only', this resets the
+tool-bar's height to the minimum height needed); if
+`recenter-redisplay' has the special value `tty', then only tty frames
+are redrawn.
 
 Just C-u as prefix means put point in the center of the window
 and redisplay normally--don't erase and redraw the frame.  */)
-  (register Lisp_Object arg)
+  (Lisp_Object arg, Lisp_Object redisplay)
 {
   struct window *w = XWINDOW (selected_window);
   struct buffer *buf = XBUFFER (w->contents);
@@ -5933,7 +5934,8 @@ and redisplay normally--don't erase and redraw the frame.  */)
 
   if (NILP (arg))
     {
-      if (!NILP (Vrecenter_redisplay)
+      if (!NILP (redisplay)
+	  && !NILP (Vrecenter_redisplay)
 	  && (!EQ (Vrecenter_redisplay, Qtty)
 	      || !NILP (Ftty_type (selected_frame))))
 	{
-- 
2.18.0

bug#31325: 27.0.50; PROPOSAL: introduce a new function to recenter without redisplaying the frame

[Eli Zaretskii]

> From: John Shahid <jvshahid@gmail.com>
> Cc: Eli Zaretskii <eliz@gnu.org>, 31325@debbugs.gnu.org, monnier@iro.umontreal.ca
> Date: Sun, 01 Jul 2018 13:17:17 +0000
> 
> Fixed in the attached patch.

Thanks, pushed to the master branch.

Please in the future mention the bug number in the log message, and
also grep the sources for callers of the functions you change.  (In
this case, there was one caller in w32console.c, and it wouldn't
compile after the change, because the number of arguments changed.)

bug#31325: 27.0.50; PROPOSAL: introduce a new function to recenter without redisplaying the frame

[John Shahid]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: John Shahid <jvshahid@gmail.com>
>> Cc: Eli Zaretskii <eliz@gnu.org>, 31325@debbugs.gnu.org, monnier@iro.umontreal.ca
>> Date: Sun, 01 Jul 2018 13:17:17 +0000
>>
>> Fixed in the attached patch.
>
> Thanks, pushed to the master branch.
>
> Please in the future mention the bug number in the log message, and
> also grep the sources for callers of the functions you change.  (In
> this case, there was one caller in w32console.c, and it wouldn't
> compile after the change, because the number of arguments changed.)

Thanks Eli.  Sorry about that, I will try to be more careful in the
future.

It just occurred to me that this change will break backward
comparability for those who bind some key to the `recenter' command,
since now the second argument will always be nil and thus won't
redisplay.  Should we also change the interactive form of recenter to
use "p" for the second argument?  That way the second arg is never nil
when used interactively.

bug#31325: 27.0.50; PROPOSAL: introduce a new function to recenter without redisplaying the frame

[Eli Zaretskii]

> From: John Shahid <jvshahid@gmail.com>
> Cc: rudalics@gmx.at, 31325-done@debbugs.gnu.org, monnier@iro.umontreal.ca
> Date: Sun, 01 Jul 2018 16:54:53 +0000
> 
> It just occurred to me that this change will break backward
> comparability for those who bind some key to the `recenter' command,
> since now the second argument will always be nil and thus won't
> redisplay.  Should we also change the interactive form of recenter to
> use "p" for the second argument?  That way the second arg is never nil
> when used interactively.

I think you are right.

Alternatively, we could simply make 'recenter' behave specially in
interactive invocations.  But in any case, incompatible changes in
behavior are bad, and should be avoided.

bug#31325: 27.0.50; PROPOSAL: introduce a new function to recenter without redisplaying the frame

[Eli Zaretskii]

> Date: Sun, 01 Jul 2018 20:03:21 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> Cc: 31325@debbugs.gnu.org, monnier@iro.umontreal.ca
> 
> Alternatively, we could simply make 'recenter' behave specially in
> interactive invocations.  But in any case, incompatible changes in
> behavior are bad, and should be avoided.

Or maybe invert the meaning of the new argument?

bug#31325: 27.0.50; PROPOSAL: introduce a new function to recenter without redisplaying the frame

[John Shahid]

Eli Zaretskii <eliz@gnu.org> writes:

>> From: John Shahid <jvshahid@gmail.com>
>> Cc: rudalics@gmx.at, 31325-done@debbugs.gnu.org, monnier@iro.umontreal.ca
>> Date: Sun, 01 Jul 2018 16:54:53 +0000
>>
>> It just occurred to me that this change will break backward
>> comparability for those who bind some key to the `recenter' command,
>> since now the second argument will always be nil and thus won't
>> redisplay.  Should we also change the interactive form of recenter to
>> use "p" for the second argument?  That way the second arg is never nil
>> when used interactively.
>
> I think you are right.
>
> Alternatively, we could simply make 'recenter' behave specially in
> interactive invocations.  But in any case, incompatible changes in
> behavior are bad, and should be avoided.

I am happy to make the change. Should we just revert this patch and
instead of adding a new argument change the behavior of`recenter' to
redraw if called interactively ?

How can we determine from C whether the function is called interactively
? Is it ok to use `called-interactively-p' ?

bug#31325: 27.0.50; PROPOSAL: introduce a new function to recenter without redisplaying the frame

[John Shahid]

Eli Zaretskii <eliz@gnu.org> writes:

>> Date: Sun, 01 Jul 2018 20:03:21 +0300
>> From: Eli Zaretskii <eliz@gnu.org>
>> Cc: 31325@debbugs.gnu.org, monnier@iro.umontreal.ca
>>
>> Alternatively, we could simply make 'recenter' behave specially in
>> interactive invocations.  But in any case, incompatible changes in
>> behavior are bad, and should be avoided.
>
> Or maybe invert the meaning of the new argument?

That isn't very ideal. The whole point from this change is not to force
lisp callers to change anything. If lisp callers have to change how they
call `recenter' they might as well change the call to be

   (let ((recenter-redisplay nil)) (recenter))

Few days ago I noticed the same flickering behavior in
`xref-find-definitions' because it uses `recenter' in the
`xref-after-jump-hook'.

bug#31325: 27.0.50; PROPOSAL: introduce a new function to recenter without redisplaying the frame

[Eli Zaretskii]

> From: John Shahid <jvshahid@gmail.com>
> Cc: rudalics@gmx.at, 31325@debbugs.gnu.org, monnier@iro.umontreal.ca
> Date: Sun, 01 Jul 2018 17:18:08 +0000
> 
> >> It just occurred to me that this change will break backward
> >> comparability for those who bind some key to the `recenter' command,
> >> since now the second argument will always be nil and thus won't
> >> redisplay.  Should we also change the interactive form of recenter to
> >> use "p" for the second argument?  That way the second arg is never nil
> >> when used interactively.
> >
> > I think you are right.
> >
> > Alternatively, we could simply make 'recenter' behave specially in
> > interactive invocations.  But in any case, incompatible changes in
> > behavior are bad, and should be avoided.
> 
> I am happy to make the change. Should we just revert this patch and
> instead of adding a new argument change the behavior of`recenter' to
> redraw if called interactively ?
> 
> How can we determine from C whether the function is called interactively
> ? Is it ok to use `called-interactively-p' ?

That'd be gross, I think.  Going through the interactive spec, as you
suggested originally, should be much simpler.

bug#31325: 27.0.50; PROPOSAL: introduce a new function to recenter without redisplaying the frame

[John Shahid]

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


Eli Zaretskii <eliz@gnu.org> writes:

>> From: John Shahid <jvshahid@gmail.com>
>> Cc: rudalics@gmx.at, 31325@debbugs.gnu.org, monnier@iro.umontreal.ca
>> Date: Sun, 01 Jul 2018 17:18:08 +0000
>>
>> >> It just occurred to me that this change will break backward
>> >> comparability for those who bind some key to the `recenter' command,
>> >> since now the second argument will always be nil and thus won't
>> >> redisplay.  Should we also change the interactive form of recenter to
>> >> use "p" for the second argument?  That way the second arg is never nil
>> >> when used interactively.
>> >
>> > I think you are right.
>> >
>> > Alternatively, we could simply make 'recenter' behave specially in
>> > interactive invocations.  But in any case, incompatible changes in
>> > behavior are bad, and should be avoided.
>>
>> I am happy to make the change. Should we just revert this patch and
>> instead of adding a new argument change the behavior of`recenter' to
>> redraw if called interactively ?
>>
>> How can we determine from C whether the function is called interactively
>> ? Is it ok to use `called-interactively-p' ?
>
> That'd be gross, I think.  Going through the interactive spec, as you
> suggested originally, should be much simpler.

Updated the interactive spec in the attached patch and made the
necessary changes to the manual and documentation.  I also missed a
place where `recenter-top-bottom' was calling `recenter' without passing
a non-nil value for REDISPLAY that I also fixed in the attached patch.

Let me know what you think.

Thanks,


[-- Attachment #2: 0001-Keep-recenter-behavior-backward-compatible-when-call.patch --]
[-- Type: text/x-diff, Size: 2892 bytes --]

From 3ac3d2d5f60593776563e27d4b406b2776de9b96 Mon Sep 17 00:00:00 2001
From: John Shahid <jvshahid@gmail.com>
Date: Sun, 1 Jul 2018 14:48:24 -0400
Subject: [PATCH] Keep `recenter' behavior backward compatible when called
 interactively

* window.c (recenter): Change the interactive spec to always pass a
  non-nil value to the REDISPLAY argument when called interactively.
* window.el (recenter-top-bottom): Make sure recenter's second
  argument is non-nil everywhere.
* windows.texi (Textual Scrolling): Edit documentation of `recenter'.
---
 doc/lispref/windows.texi | 3 ++-
 lisp/window.el           | 2 +-
 src/window.c             | 4 ++--
 3 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/doc/lispref/windows.texi b/doc/lispref/windows.texi
index 9740bbebf2..8903922da5 100644
--- a/doc/lispref/windows.texi
+++ b/doc/lispref/windows.texi
@@ -4154,7 +4154,8 @@ Textual Scrolling
 @code{recenter} puts the line containing point in the middle of the
 window.  If @var{count} is @code{nil} and @var{redisplay} is
 non-@code{nil}, this function may redraw the frame, according to the
-value of @code{recenter-redisplay}.
+value of @code{recenter-redisplay}. Interactive calls pass non-‘nil’
+for @var{redisplay}.
 
 When @code{recenter} is called interactively, @var{count} is the raw
 prefix argument.  Thus, typing @kbd{C-u} as the prefix sets the
diff --git a/lisp/window.el b/lisp/window.el
index 6d9d8bdcd2..d56bed63da 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -8767,7 +8767,7 @@ recenter-top-bottom
  With plain `C-u', move current line to window center."
   (interactive "P")
   (cond
-   (arg (recenter arg))			; Always respect ARG.
+   (arg (recenter arg t))                 ; Always respect ARG.
    (t
     (setq recenter-last-op
 	  (if (eq this-command last-command)
diff --git a/src/window.c b/src/window.c
index a966e99ca8..ffa2236cf8 100644
--- a/src/window.c
+++ b/src/window.c
@@ -5898,7 +5898,7 @@ displayed_window_lines (struct window *w)
 }
 
 
-DEFUN ("recenter", Frecenter, Srecenter, 0, 2, "P",
+DEFUN ("recenter", Frecenter, Srecenter, 0, 2, "P\np",
        doc: /* Center point in selected window and maybe redisplay frame.
 With a numeric prefix argument ARG, recenter putting point on screen line ARG
 relative to the selected window.  If ARG is negative, it counts up from the
@@ -5910,7 +5910,7 @@ non-nil, also erase the entire frame and redraw it (when
 `auto-resize-tool-bars' is set to `grow-only', this resets the
 tool-bar's height to the minimum height needed); if
 `recenter-redisplay' has the special value `tty', then only tty frames
-are redrawn.
+are redrawn.  Interactive calls pass non-nil for REDISPLAY.
 
 Just C-u as prefix means put point in the center of the window
 and redisplay normally--don't erase and redraw the frame.  */)
-- 
2.18.0

bug#31325: 27.0.50; PROPOSAL: introduce a new function to recenter without redisplaying the frame

[Drew Adams]

Sorry, but I've not been following this bug thread.
I took only a (very) quick look at it, at
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=31325.

It looks like you are changing the default behavior of
`recenter', which is a backward-incompatible change.

Am I right about that, so that all Lisp calls of that
function would now need to be changed, to add a non-nil
2nd arg in order to keep the same behavior?

The reported bug was only about a problem for Emacs in
a terminal, right?  Can't it just be fixed to DTRT in
that case, without changing so much in the definition
and behavior of the function?

Apologies if I misunderstood something.  I really only
glanced at the thread.  But if what thought I saw is
right, is this really the right fix?

The behavior of `recenter' has remained pretty much
unchanged since, well, _forever_.  And when I came up
with `recenter-top-bottom' the idea was explicitly
_not_ to change the behavior of `recenter'.

And in the old days people used Emacs mainly (and
originally, only) in the terminal.  Why is the terminal
behavior suddenly considered to be bugged?  Or given
that it is bugged - is changing the default behavior
the best fix?

Please ignore, if my understanding is wrong or my
comments on this are not helpful.

bug#31325: 27.0.50; PROPOSAL: introduce a new function to recenter without redisplaying the frame

[John Shahid]

Drew Adams <drew.adams@oracle.com> writes:

> Sorry, but I've not been following this bug thread.
> I took only a (very) quick look at it, at
> https://debbugs.gnu.org/cgi/bugreport.cgi?bug=31325.
>
> It looks like you are changing the default behavior of
> `recenter', which is a backward-incompatible change.

That's right but only for non-interactive uses of `recenter'.  All
interactive uses should remain backward compatible.

> Am I right about that, so that all Lisp calls of that
> function would now need to be changed, to add a non-nil
> 2nd arg in order to keep the same behavior?

That's right but only if the caller need to redisplay the frame.  Are
you aware of any lisp calls that make use of that feature
(i.e. redisplay the frame as well as recenter) ? I'm only aware of two
uses in Flycheck and `xref-find-definitions'.  IMHO, in both cases the
redisplay side-effect is undesirable.

> The reported bug was only about a problem for Emacs in
> a terminal, right?  Can't it just be fixed to DTRT in
> that case, without changing so much in the definition
> and behavior of the function?

I am not sure what does the right thing mean in this case.  AFAIK, the
patch was trying to do the right thing by keeping the interactive use
backward compatible but prevent calling `recenter' from lisp from
redisplaying the frame.

> And in the old days people used Emacs mainly (and
> originally, only) in the terminal.  Why is the terminal
> behavior suddenly considered to be bugged?  Or given
> that it is bugged - is changing the default behavior
> the best fix?

I only use Emacs from the terminal, in fact I compile emacs `without-x`
since I never use the GUI.

> Please ignore, if my understanding is wrong or my
> comments on this are not helpful.

I think it is very valuable to have more feedback on the patch.  That
said, I am still having trouble understanding what are your concerns.

Cheers,

bug#31325: 27.0.50; PROPOSAL: introduce a new function to recenter without redisplaying the frame

[Drew Adams]

> > It looks like you are changing the default behavior of
> > `recenter', which is a backward-incompatible change.
> 
> That's right but only for non-interactive uses of `recenter'.  All
> interactive uses should remain backward compatible.

Sure.  That's why I said "Lisp calls".

> > Am I right about that, so that all Lisp calls of that
> > function would now need to be changed, to add a non-nil
> > 2nd arg in order to keep the same behavior?
> 
> That's right but only if the caller need to redisplay the frame.  Are
> you aware of any lisp calls that make use of that feature
> (i.e. redisplay the frame as well as recenter) ? I'm only aware of two
> uses in Flycheck and `xref-find-definitions'.  IMHO, in both cases the
> redisplay side-effect is undesirable.

I think you mean that in both cases it is undesirable _in
a terminal_, no?

Am I aware of any such calls?  That's irrelevant - to me.

And I really had no idea whether I know of such calls.  The
point is that this function has been around since Day One,
with the same behavior, and it's likely that there is code
out there that is broken by this change.  It's certainly
not enough to search the Lisp code distributed with Emacs,
to get an idea whether something is used.

However, as it turns out, I do have such calls in some of
my code.  I expected that this was not true (bad memory),
but grepping just now shows that it is.  I know it might
look like I was complaining about this change affecting my
code, but that's not the case.  I really had no idea (I'd
forgotten) that I in fact use `recenter'.

In Bookmark+ function `bmkp-handle-region-default' I use
`(recenter 1)'.  And yes, that code should redisplay.

I haven't bothered to try it in terminal mode, but if that's
a problem then doesn't it indicate a problem (bug) in Emacs
terminal mode, not in `recenter'?  Invoking a function from
Lisp is not the same as invoking it from Lisp with Emacs in
terminal mode.

And I have other uses of it, all of which should redisplay.

Icicles has multiple calls like this, where the default
value of option `icicle-recenter' is -4:

(unless (pos-visible-in-window-p) (recenter icicle-recenter))

And in Isearch+ I have this in my version of `isearch-cancel':

(when isearchp-win-pt-line (recenter isearchp-win-pt-line))

where that variable is the line number of point before
searching, relative to `window-start'.

And in `palette.el' I do this:
(when win
  (select-window win) (goto-char (point-min)) (recenter))

And in `synonyms.el' I have `(recenter 2)'.

Could I just use `set-window-point' or some such?  Maybe;
dunno.  I don't feel like debugging all such stuff, to see.
Sure, I could adapt your code change, to add the 2nd arg
(also testing the Emacs version...).  And I will, if you
go ahead with your change.

But this is not at all about my code, and it is not really
even about whether there is other code out there that uses
`recenter' expecting it to redisplay, although I expect that
there is such other code.  That's just what `recenter' does,
and it always has.  So sure, there is bound to be code that
counts on that behavior.  It's a very old function, with a
pretty simple behavior (from a use point of view, at least).

But the existence of code that would break is not really
the question.  The question is why this change should be
made to the Lisp behavior, just to accommodate a
terminal-Emacs display annoyance.  What does the one have
to do with the other?

It's really about what the _bug_ is and what it's solution
should be.  So far (but again, I haven't really read the
thread), it does not sound to me like `recenter' has a
bug.  It sounds like Emacs behaves badly in a terminal,
in this case (or in some terminals, some of the time,
perhaps).

There is nothing older than the combination of `recenter'
(same as it's always been) and Emacs in a terminal, is
there?  Why is this suddenly a bug now?

> > The reported bug was only about a problem for Emacs in
> > a terminal, right?  Can't it just be fixed to DTRT in
> > that case, without changing so much in the definition
> > and behavior of the function?
> 
> I am not sure what does the right thing mean in this case.  AFAIK, the
> patch was trying to do the right thing by keeping the interactive use
> backward compatible but prevent calling `recenter' from lisp from
> redisplaying the frame.

Use of `recenter' in a terminal is not the same as use of
`recenter' from Lisp.  Lisp is used with GUI Emacs also.

TRT surely has something to do with terminal Emacs, not
with whether or not the use is interactive, no?

Please do fix the behavior of `recenter' for Emacs in a
terminal, if it's broken.  But why "fix" it for GUI too,
if it's not broken there?  Maybe just test
`display-graphic-p'?

> > And in the old days people used Emacs mainly (and
> > originally, only) in the terminal.  Why is the terminal
> > behavior suddenly considered to be bugged?  Or given
> > that it is bugged - is changing the default behavior
> > the best fix?
> 
> I only use Emacs from the terminal, in fact I compile emacs `without-x`
> since I never use the GUI.

OK.  And did you just start seeing this bugged behavior?
Or did you just start using Emacs?  Haven't people been
using it in a terminal for decades, without this having
been reported or addressed?

Maybe it's a new problem, due to some other change.  Or
maybe it's always been there and people have just ignored
it.  In any case, IF it is to be fixed then it seems like
it should be fixed only for terminal Emacs.

> > Please ignore, if my understanding is wrong or my
> > comments on this are not helpful.
> 
> I think it is very valuable to have more feedback on the patch.  That
> said, I am still having trouble understanding what are your concerns.

Thanks; HTH.  Is it any clearer now?

bug#31325: 27.0.50; PROPOSAL: introduce a new function to recenter without redisplaying the frame

[John Shahid]

Drew Adams <drew.adams@oracle.com> writes:

> In Bookmark+ function `bmkp-handle-region-default' I use
> `(recenter 1)'.  And yes, that code should redisplay.

Are you confusing redisplaying the window that is displaying the buffer
with clearing and redisplaying the entire tty frame ? This patch
addresses the latter.  To be clear, `recenter' will redisplay the window
displaying the buffer and this patch doesn't change that behavior.

If you meant the former (i.e. clearing and redisplaying the entire
frame), can you elaborate on why `bmkp-handle-region-default' would need
to do that.

Clearing and redisplaying the entire frame causes unnecessary flickering
that makes the eating experience very poor.  To my knowledge this
behavior is only needed for interactive users in order to clear the
terminal and redisplay Emacs' content.  For example, this could happen
if the kernel decides to print messages or if a background process
prints to the same terminal that Emacs is using.  I fail to see how a
lisp call would be interested in redisplaying the entire frame.

> Icicles has multiple calls like this, where the default
> value of option `icicle-recenter' is -4:
>
> (unless (pos-visible-in-window-p) (recenter icicle-recenter))
>
> And in Isearch+ I have this in my version of `isearch-cancel':
>
> (when isearchp-win-pt-line (recenter isearchp-win-pt-line))
>
> where that variable is the line number of point before
> searching, relative to `window-start'.
>
> And in `palette.el' I do this:
> (when win
>   (select-window win) (goto-char (point-min)) (recenter))
>
> And in `synonyms.el' I have `(recenter 2)'.

Same as above.  I don't see how these calls to `recenter' should
redisplay the entire frame as opposed to redrawing the single window
that displays the buffer.

> Could I just use `set-window-point' or some such?  Maybe;
> dunno.  I don't feel like debugging all such stuff, to see.
> Sure, I could adapt your code change, to add the 2nd arg
> (also testing the Emacs version...).  And I will, if you
> go ahead with your change.

I don't think you have to use a different lisp function or add the extra
argument, unless of course I misunderstood what these libraries are
trying to do.

> But this is not at all about my code, and it is not really
> even about whether there is other code out there that uses
> `recenter' expecting it to redisplay, although I expect that
> there is such other code.  That's just what `recenter' does,
> and it always has.  So sure, there is bound to be code that
> counts on that behavior.  It's a very old function, with a
> pretty simple behavior (from a use point of view, at least).
>
> But the existence of code that would break is not really
> the question.  The question is why this change should be
> made to the Lisp behavior, just to accommodate a
> terminal-Emacs display annoyance.  What does the one have
> to do with the other?
>
> It's really about what the _bug_ is and what it's solution
> should be.  So far (but again, I haven't really read the
> thread), it does not sound to me like `recenter' has a
> bug.  It sounds like Emacs behaves badly in a terminal,
> in this case (or in some terminals, some of the time,
> perhaps).
>
> There is nothing older than the combination of `recenter'
> (same as it's always been) and Emacs in a terminal, is
> there?  Why is this suddenly a bug now?

I don't think anything changed other than having the desire and time to
fix this behavior.  I have been noticing this behavior for a while but
didn't bother debugging it to figure out the root cause.

> Use of `recenter' in a terminal is not the same as use of
> `recenter' from Lisp.  Lisp is used with GUI Emacs also.

As I mentioned previously, the behavior of `recenter' in the GUI hasn't
changed, assuming you use the default value of `recenter-redisplay'.

> TRT surely has something to do with terminal Emacs, not
> with whether or not the use is interactive, no?
>
> Please do fix the behavior of `recenter' for Emacs in a
> terminal, if it's broken.  But why "fix" it for GUI too,
> if it's not broken there?  Maybe just test
> `display-graphic-p'?

I don't think we need to do anything special for the GUI.  As I
mentioned above `recenter' doesn't redisplay the entire frame by default
when a GUI is being used.  This is because redisplaying the frame is
unnecessary for GUI, since they don't suffer from the same problem as
terminals where some random process could clobber the display.

>> > And in the old days people used Emacs mainly (and
>> > originally, only) in the terminal.  Why is the terminal
>> > behavior suddenly considered to be bugged?  Or given
>> > that it is bugged - is changing the default behavior
>> > the best fix?
>>
>> I only use Emacs from the terminal, in fact I compile emacs `without-x`
>> since I never use the GUI.
>
> OK.  And did you just start seeing this bugged behavior?
> Or did you just start using Emacs?  Haven't people been
> using it in a terminal for decades, without this having
> been reported or addressed?

I have been using Emacs for almost 10 years now.  I don't think there is
anything new about this except for my desire to fix it now.

> Maybe it's a new problem, due to some other change.  Or
> maybe it's always been there and people have just ignored
> it.  In any case, IF it is to be fixed then it seems like
> it should be fixed only for terminal Emacs.
>
>> > Please ignore, if my understanding is wrong or my
>> > comments on this are not helpful.
>>
>> I think it is very valuable to have more feedback on the patch.  That
>> said, I am still having trouble understanding what are your concerns.
>
> Thanks; HTH.  Is it any clearer now?

I don't think so.  The way you argue against merging the patch makes me
believe you misunderstood the intended change.  I suggest you read the
entire thread as well as the original help-gnu-emacs [1] thread for more
context.

[1]: http://lists.gnu.org/archive/html/help-gnu-emacs/2018-04/msg00326.html

bug#31325: 27.0.50; PROPOSAL: introduce a new function to recenter without redisplaying the frame

[Eli Zaretskii]

> From: John Shahid <jvshahid@gmail.com>
> Cc: Eli Zaretskii <eliz@gnu.org>, 31325@debbugs.gnu.org, monnier@iro.umontreal.ca
> Date: Mon, 02 Jul 2018 02:27:22 +0000
> 
> Are you confusing redisplaying the window that is displaying the buffer
> with clearing and redisplaying the entire tty frame ? This patch
> addresses the latter.  To be clear, `recenter' will redisplay the window
> displaying the buffer and this patch doesn't change that behavior.

Indeed, I think there's some confusion here.  Recentering a window
will always trigger redisplay of that window if the new point is out
of view, or if enters the display margins.  But it doesn't necessarily
clear and redraw the entire frame.

For the record: historically, 'recenter' always cleared the entire
frame and drew it anew.  Then, almost exactly 9 years ago, we changed
the default behavior to clear and redraw frame only on TTYs, and
introduced a variable, recenter-redisplay, to disable redrawing even
of TTY frames.

The current change goes a step further in the same direction: it
modifies the behavior of non-interactive calls of 'recenter', such
that even on a TTY it doesn't by default redraw the entire frame,
unless the caller explicitly requests that.  If not specifically
requested, 'recenter' called non-interactively will just recenter (and
trigger redisplay of the window which it recentered, if necessary).
The rationale for this change is that the vast majority of
non-interactive callers of 'recenter' have no reason to redraw the
entire frame, and recenter-redisplay is a knob meant to control only
the interactive behavior.

bug#31325: 27.0.50; PROPOSAL: introduce a new function to recenter without redisplaying the frame

[Eli Zaretskii]

> From: John Shahid <jvshahid@gmail.com>
> Cc: rudalics@gmx.at, 31325@debbugs.gnu.org, monnier@iro.umontreal.ca
> Date: Sun, 01 Jul 2018 18:58:09 +0000
> 
> >From 3ac3d2d5f60593776563e27d4b406b2776de9b96 Mon Sep 17 00:00:00 2001
> From: John Shahid <jvshahid@gmail.com>
> Date: Sun, 1 Jul 2018 14:48:24 -0400
> Subject: [PATCH] Keep `recenter' behavior backward compatible when called
>  interactively
> 
> * window.c (recenter): Change the interactive spec to always pass a
>   non-nil value to the REDISPLAY argument when called interactively.
> * window.el (recenter-top-bottom): Make sure recenter's second
>   argument is non-nil everywhere.
> * windows.texi (Textual Scrolling): Edit documentation of `recenter'.

Bug number is missing.  Also, we prefer to quite in documentation (as
opposed to doc strings) 'like this', not `like this'.

> --- a/doc/lispref/windows.texi
> +++ b/doc/lispref/windows.texi
> @@ -4154,7 +4154,8 @@ Textual Scrolling
>  @code{recenter} puts the line containing point in the middle of the
>  window.  If @var{count} is @code{nil} and @var{redisplay} is
>  non-@code{nil}, this function may redraw the frame, according to the
> -value of @code{recenter-redisplay}.
> +value of @code{recenter-redisplay}. Interactive calls pass non-‘nil’
> +for @var{redisplay}.              ^^

Two spaces between sentences.

> -DEFUN ("recenter", Frecenter, Srecenter, 0, 2, "P",
> +DEFUN ("recenter", Frecenter, Srecenter, 0, 2, "P\np",
>         doc: /* Center point in selected window and maybe redisplay frame.
>  With a numeric prefix argument ARG, recenter putting point on screen line ARG
>  relative to the selected window.  If ARG is negative, it counts up from the
> @@ -5910,7 +5910,7 @@ non-nil, also erase the entire frame and redraw it (when
>  `auto-resize-tool-bars' is set to `grow-only', this resets the
>  tool-bar's height to the minimum height needed); if
>  `recenter-redisplay' has the special value `tty', then only tty frames
> -are redrawn.
> +are redrawn.  Interactive calls pass non-nil for REDISPLAY.

I think this text is slightly more clear:

  Interactively, REDISPLAY is always non-nil.

Thanks.

bug#31325: 27.0.50; PROPOSAL: introduce a new function to recenter without redisplaying the frame

[John Shahid]

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


Eli Zaretskii <eliz@gnu.org> writes:

>> From: John Shahid <jvshahid@gmail.com>
>> Cc: rudalics@gmx.at, 31325@debbugs.gnu.org, monnier@iro.umontreal.ca
>> Date: Sun, 01 Jul 2018 18:58:09 +0000
>> 
>> >From 3ac3d2d5f60593776563e27d4b406b2776de9b96 Mon Sep 17 00:00:00 2001
>> From: John Shahid <jvshahid@gmail.com>
>> Date: Sun, 1 Jul 2018 14:48:24 -0400
>> Subject: [PATCH] Keep `recenter' behavior backward compatible when called
>>  interactively
>> 
>> * window.c (recenter): Change the interactive spec to always pass a
>>   non-nil value to the REDISPLAY argument when called interactively.
>> * window.el (recenter-top-bottom): Make sure recenter's second
>>   argument is non-nil everywhere.
>> * windows.texi (Textual Scrolling): Edit documentation of `recenter'.
>
> Bug number is missing.  Also, we prefer to quite in documentation (as
> opposed to doc strings) 'like this', not `like this'.

That is fixed in the attached patch.

>
>> --- a/doc/lispref/windows.texi
>> +++ b/doc/lispref/windows.texi
>> @@ -4154,7 +4154,8 @@ Textual Scrolling
>>  @code{recenter} puts the line containing point in the middle of the
>>  window.  If @var{count} is @code{nil} and @var{redisplay} is
>>  non-@code{nil}, this function may redraw the frame, according to the
>> -value of @code{recenter-redisplay}.
>> +value of @code{recenter-redisplay}. Interactive calls pass non-‘nil’
>> +for @var{redisplay}.              ^^
>
> Two spaces between sentences.

Sorry about that, I'm still getting used to using 2 spaces after
period. How do you spot these ? Do you have some font-lock keyword to
highlight them. I found checkdoc not to be very useful for checking the
manual or docstrings in .c files.

>
>> -DEFUN ("recenter", Frecenter, Srecenter, 0, 2, "P",
>> +DEFUN ("recenter", Frecenter, Srecenter, 0, 2, "P\np",
>>         doc: /* Center point in selected window and maybe redisplay frame.
>>  With a numeric prefix argument ARG, recenter putting point on screen line ARG
>>  relative to the selected window.  If ARG is negative, it counts up from the
>> @@ -5910,7 +5910,7 @@ non-nil, also erase the entire frame and redraw it (when
>>  `auto-resize-tool-bars' is set to `grow-only', this resets the
>>  tool-bar's height to the minimum height needed); if
>>  `recenter-redisplay' has the special value `tty', then only tty frames
>> -are redrawn.
>> +are redrawn.  Interactive calls pass non-nil for REDISPLAY.
>
> I think this text is slightly more clear:
>
>   Interactively, REDISPLAY is always non-nil.

I changed the text in the attached patch.

Thanks,


[-- Attachment #2: 0001-Keep-interactive-uses-of-recenter-backward-compatibl.patch --]
[-- Type: text/x-diff, Size: 2888 bytes --]

From ed35d20c422adc80bfbd4090b64d9a43d099b6b9 Mon Sep 17 00:00:00 2001
From: John Shahid <jvshahid@gmail.com>
Date: Sun, 1 Jul 2018 14:48:24 -0400
Subject: [PATCH] Keep interactive uses of 'recenter' backward compatible
 (Bug#31325)

* window.c (recenter): Change the interactive spec to always pass a
  non-nil value to the REDISPLAY argument when called interactively.
* window.el (recenter-top-bottom): Make sure recenter's second
  argument is non-nil everywhere.
* windows.texi (Textual Scrolling): Edit documentation of 'recenter'.
---
 doc/lispref/windows.texi | 3 ++-
 lisp/window.el           | 2 +-
 src/window.c             | 4 ++--
 3 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/doc/lispref/windows.texi b/doc/lispref/windows.texi
index 9740bbebf2..6f1cf788a0 100644
--- a/doc/lispref/windows.texi
+++ b/doc/lispref/windows.texi
@@ -4154,7 +4154,8 @@ Textual Scrolling
 @code{recenter} puts the line containing point in the middle of the
 window.  If @var{count} is @code{nil} and @var{redisplay} is
 non-@code{nil}, this function may redraw the frame, according to the
-value of @code{recenter-redisplay}.
+value of @code{recenter-redisplay}.  Interactive calls pass non-‘nil’
+for @var{redisplay}.
 
 When @code{recenter} is called interactively, @var{count} is the raw
 prefix argument.  Thus, typing @kbd{C-u} as the prefix sets the
diff --git a/lisp/window.el b/lisp/window.el
index 6d9d8bdcd2..d56bed63da 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -8767,7 +8767,7 @@ recenter-top-bottom
  With plain `C-u', move current line to window center."
   (interactive "P")
   (cond
-   (arg (recenter arg))			; Always respect ARG.
+   (arg (recenter arg t))                 ; Always respect ARG.
    (t
     (setq recenter-last-op
 	  (if (eq this-command last-command)
diff --git a/src/window.c b/src/window.c
index 20f6862e3b..047b741475 100644
--- a/src/window.c
+++ b/src/window.c
@@ -5896,7 +5896,7 @@ displayed_window_lines (struct window *w)
 }
 
 
-DEFUN ("recenter", Frecenter, Srecenter, 0, 2, "P",
+DEFUN ("recenter", Frecenter, Srecenter, 0, 2, "P\np",
        doc: /* Center point in selected window and maybe redisplay frame.
 With a numeric prefix argument ARG, recenter putting point on screen line ARG
 relative to the selected window.  If ARG is negative, it counts up from the
@@ -5908,7 +5908,7 @@ non-nil, also erase the entire frame and redraw it (when
 `auto-resize-tool-bars' is set to `grow-only', this resets the
 tool-bar's height to the minimum height needed); if
 `recenter-redisplay' has the special value `tty', then only tty frames
-are redrawn.
+are redrawn.  Interactively, REDISPLAY is always non-nil.
 
 Just C-u as prefix means put point in the center of the window
 and redisplay normally--don't erase and redraw the frame.  */)
-- 
2.18.0

bug#31325: 27.0.50; PROPOSAL: introduce a new function to recenter without redisplaying the frame

[Robert Pluim]

John Shahid <jvshahid@gmail.com> writes:

> Eli Zaretskii <eliz@gnu.org> writes:
>>
>>> --- a/doc/lispref/windows.texi
>>> +++ b/doc/lispref/windows.texi
>>> @@ -4154,7 +4154,8 @@ Textual Scrolling
>>>  @code{recenter} puts the line containing point in the middle of the
>>>  window.  If @var{count} is @code{nil} and @var{redisplay} is
>>>  non-@code{nil}, this function may redraw the frame, according to the
>>> -value of @code{recenter-redisplay}.
>>> +value of @code{recenter-redisplay}. Interactive calls pass non-‘nil’
>>> +for @var{redisplay}.              ^^
>>
>> Two spaces between sentences.
>
> Sorry about that, I'm still getting used to using 2 spaces after
> period. How do you spot these ? Do you have some font-lock keyword to
> highlight them. I found checkdoc not to be very useful for checking the
> manual or docstrings in .c files.

You get used to it. One way to check is to put point at the beginning
of the text you changed and seeing if 'M-e' (forward-sentence) does
the expected thing.

Robert

bug#31325: 27.0.50; PROPOSAL: introduce a new function to recenter without redisplaying the frame

[John Shahid]

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


John Shahid <jvshahid@gmail.com> writes:

> Eli Zaretskii <eliz@gnu.org> writes:
>
>>> From: John Shahid <jvshahid@gmail.com>
>>> Cc: rudalics@gmx.at, 31325@debbugs.gnu.org, monnier@iro.umontreal.ca
>>> Date: Sun, 01 Jul 2018 18:58:09 +0000
>>>
>>> >From 3ac3d2d5f60593776563e27d4b406b2776de9b96 Mon Sep 17 00:00:00 2001
>>> From: John Shahid <jvshahid@gmail.com>
>>> Date: Sun, 1 Jul 2018 14:48:24 -0400
>>> Subject: [PATCH] Keep `recenter' behavior backward compatible when called
>>>  interactively
>>>
>>> * window.c (recenter): Change the interactive spec to always pass a
>>>   non-nil value to the REDISPLAY argument when called interactively.
>>> * window.el (recenter-top-bottom): Make sure recenter's second
>>>   argument is non-nil everywhere.
>>> * windows.texi (Textual Scrolling): Edit documentation of `recenter'.
>>
>> Bug number is missing.  Also, we prefer to quite in documentation (as
>> opposed to doc strings) 'like this', not `like this'.
>
> That is fixed in the attached patch.

I rebased and attached a new patch since there were some changes to the
manual on master.


[-- Attachment #2: 0001-Keep-interactive-uses-of-recenter-backward-compatibl.patch --]
[-- Type: text/x-diff, Size: 2896 bytes --]

From 88c07293e82e2bcd9dbfc09a87e4d45f3e56676c Mon Sep 17 00:00:00 2001
From: John Shahid <jvshahid@gmail.com>
Date: Sun, 1 Jul 2018 14:48:24 -0400
Subject: [PATCH] Keep interactive uses of 'recenter' backward compatible
 (Bug#31325)

* window.c (recenter): Change the interactive spec to always pass a
  non-nil value to the REDISPLAY argument when called interactively.
* window.el (recenter-top-bottom): Make sure recenter's second
  argument is non-nil everywhere.
* windows.texi (Textual Scrolling): Edit documentation of 'recenter'.
---
 doc/lispref/windows.texi | 3 ++-
 lisp/window.el           | 2 +-
 src/window.c             | 4 ++--
 3 files changed, 5 insertions(+), 4 deletions(-)

diff --git a/doc/lispref/windows.texi b/doc/lispref/windows.texi
index ae6837b444..3eaa15a603 100644
--- a/doc/lispref/windows.texi
+++ b/doc/lispref/windows.texi
@@ -4156,7 +4156,8 @@ Textual Scrolling
 non-@code{nil}, this function may redraw the frame, according to the
 value of @code{recenter-redisplay}.  Thus, omitting the second
 argument can be used to countermand the effect of
-@code{recenter-redisplay} being non-@code{nil}.
+@code{recenter-redisplay} being non-@code{nil}.  Interactive calls
+pass non-‘nil’ for @var{redisplay}.
 
 When @code{recenter} is called interactively, @var{count} is the raw
 prefix argument.  Thus, typing @kbd{C-u} as the prefix sets the
diff --git a/lisp/window.el b/lisp/window.el
index 6d9d8bdcd2..d56bed63da 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -8767,7 +8767,7 @@ recenter-top-bottom
  With plain `C-u', move current line to window center."
   (interactive "P")
   (cond
-   (arg (recenter arg))			; Always respect ARG.
+   (arg (recenter arg t))                 ; Always respect ARG.
    (t
     (setq recenter-last-op
 	  (if (eq this-command last-command)
diff --git a/src/window.c b/src/window.c
index 20f6862e3b..047b741475 100644
--- a/src/window.c
+++ b/src/window.c
@@ -5896,7 +5896,7 @@ displayed_window_lines (struct window *w)
 }
 
 
-DEFUN ("recenter", Frecenter, Srecenter, 0, 2, "P",
+DEFUN ("recenter", Frecenter, Srecenter, 0, 2, "P\np",
        doc: /* Center point in selected window and maybe redisplay frame.
 With a numeric prefix argument ARG, recenter putting point on screen line ARG
 relative to the selected window.  If ARG is negative, it counts up from the
@@ -5908,7 +5908,7 @@ non-nil, also erase the entire frame and redraw it (when
 `auto-resize-tool-bars' is set to `grow-only', this resets the
 tool-bar's height to the minimum height needed); if
 `recenter-redisplay' has the special value `tty', then only tty frames
-are redrawn.
+are redrawn.  Interactively, REDISPLAY is always non-nil.
 
 Just C-u as prefix means put point in the center of the window
 and redisplay normally--don't erase and redraw the frame.  */)
-- 
2.18.0

bug#31325: 27.0.50; PROPOSAL: introduce a new function to recenter without redisplaying the frame

[Eli Zaretskii]

> From: John Shahid <jvshahid@gmail.com>
> Cc: rudalics@gmx.at, 31325@debbugs.gnu.org, monnier@iro.umontreal.ca
> Date: Tue, 03 Jul 2018 16:38:32 +0000
> 
> >> Bug number is missing.  Also, we prefer to quite in documentation (as
> >> opposed to doc strings) 'like this', not `like this'.
> >
> > That is fixed in the attached patch.
> 
> I rebased and attached a new patch since there were some changes to the
> manual on master.

Thanks, pushed.