bug#46119: 28.0.50; this-error-recenter

bug#46119: 28.0.50; this-error-recenter

[Tino Calancha]

X-Debbugs-Cc: Juri Linkov <juri@jurta.org>,uyennhi.qm@gmail.com
Severity: wishlist
Tags: patch

As a regular greper with a tiny screen I tend to use this command quite often.

Tentatively, I have named it `this-error-recenter'; if you find a
better name for it, please let me know.

--8<-----------------------------cut here---------------start------------->8---
commit 8cf1b91190013154a82a731a067cdf7d18288379
Author: Tino Calancha <ccalancha@suse.com>
Date:   Tue Jan 26 21:57:50 2021 +0100

    Add command to recenter errors from Occur/Grep buffers
    
    It allows to scroll up/down the current displayed occurrence/error
    without abandon the Occur/Grep buffer.
    
    * lisp/simple.el (this-error-recenter): New command.
    * lisp/progmodes/grep.el (grep-mode-map):
    Delete bidings for n and p.
    
    * lisp/progmodes/compile.el (compilation-minor-mode-map):
    Move here the n and p bindings.
    Bind the new command to l.
    * lisp/replace.el (occur-mode-map):
    Bind the new command to l.
    * etc/NEWS (Changes in Specialized Modes and Packages in Emacs 28.1):
    Announce it.

diff --git a/etc/NEWS b/etc/NEWS
index e038076e96..a51a6b4f63 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -454,9 +454,15 @@ applied when the option 'tab-line-tab-face-functions' is
 so-configured.  That option may also be used to customize tab-line
 faces in other ways.
 
-** New bindings in occur-mode, 'next-error-no-select' bound to 'n' and
+** Occur mode
+
+*** New bindings in occur-mode, 'next-error-no-select' bound to 'n' and
 'previous-error-no-select' bound to 'p'.
 
+*** The new command 'this-error-recenter', bound to 'l', recenters the
+current displayed occurrence from a Occur buffer and the current
+displayed error in a compilation buffer.
+
 ** EIEIO
 
 +++
diff --git a/lisp/progmodes/compile.el b/lisp/progmodes/compile.el
index 94e4f3c6fa..1031f835f8 100644
--- a/lisp/progmodes/compile.el
+++ b/lisp/progmodes/compile.el
@@ -2073,6 +2073,10 @@ compilation-minor-mode-map
     (define-key map "\M-p" 'compilation-previous-error)
     (define-key map "\M-{" 'compilation-previous-file)
     (define-key map "\M-}" 'compilation-next-file)
+    (define-key map "n" 'next-error-no-select)
+    (define-key map "p" 'previous-error-no-select)
+    (define-key map "l" 'this-error-recenter)
+
     (define-key map "g" 'recompile) ; revert
     ;; Set up the menu-bar
     (define-key map [menu-bar compilation]
diff --git a/lisp/progmodes/grep.el b/lisp/progmodes/grep.el
index 1a8435fde3..d6ee8bb423 100644
--- a/lisp/progmodes/grep.el
+++ b/lisp/progmodes/grep.el
@@ -275,8 +275,6 @@ grep-mode-map
     (define-key map "\C-c\C-f" 'next-error-follow-minor-mode)
 
     (define-key map "\r" 'compile-goto-error)  ;; ?
-    (define-key map "n" 'next-error-no-select)
-    (define-key map "p" 'previous-error-no-select)
     (define-key map "{" 'compilation-previous-file)
     (define-key map "}" 'compilation-next-file)
     (define-key map "\t" 'compilation-next-error)
diff --git a/lisp/replace.el b/lisp/replace.el
index db5b340631..bd3a96a26e 100644
--- a/lisp/replace.el
+++ b/lisp/replace.el
@@ -1135,6 +1135,7 @@ occur-mode-map
     (define-key map "\C-o" 'occur-mode-display-occurrence)
     (define-key map "n" 'next-error-no-select)
     (define-key map "p" 'previous-error-no-select)
+    (define-key map "l" 'this-error-recenter)
     (define-key map "\M-n" 'occur-next)
     (define-key map "\M-p" 'occur-prev)
     (define-key map "r" 'occur-rename-buffer)
diff --git a/lisp/simple.el b/lisp/simple.el
index c878fdab92..6da1dec003 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -496,6 +496,16 @@ next-error-message-highlight
         (overlay-put ol 'window (get-buffer-window))
         (setf next-error--message-highlight-overlay ol)))))
 
+(defun this-error-recenter (&optional arg)
+  "Recenter the current displayed error in the `next-error' buffer."
+  (interactive "P")
+  (if (not (or (eq major-mode 'occur-mode)
+               (derived-mode-p 'compilation-mode)))
+      (user-error "This command is for *Occur* or *Grep* buffers")
+    (funcall next-error-function 0 nil)
+    (recenter-top-bottom arg)
+    (pop-to-buffer next-error-last-buffer)))
+
 \f
 ;;;
 

--8<-----------------------------cut here---------------end--------------->8---

In GNU Emacs 28.0.50 (build 9, x86_64-pc-linux-gnu, X toolkit, cairo version 1.16.0, Xaw scroll bars)
 of 2021-01-26 built on localhost.example.com
Repository revision: 8cf1b91190013154a82a731a067cdf7d18288379
Repository branch: this-error-recenter
Windowing system distributor 'The X.Org Foundation', version 11.0.12010000
System Description: openSUSE Tumbleweed

Configured using:
 'configure --with-x-toolkit=lucid'

Configured features:
CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GSETTINGS HARFBUZZ JPEG
LIBSELINUX LIBXML2 MODULES NOTIFY INOTIFY PDUMPER PNG SOUND THREADS TIFF
TOOLKIT_SCROLL_BARS X11 XDBE XIM XPM LUCID ZLIB

bug#46119: 28.0.50; this-error-recenter

[Juri Linkov]

> +(defun this-error-recenter (&optional arg)
> +  "Recenter the current displayed error in the `next-error' buffer."
> +  (interactive "P")
> +  (if (not (or (eq major-mode 'occur-mode)
> +               (derived-mode-p 'compilation-mode)))
> +      (user-error "This command is for *Occur* or *Grep* buffers")
> +    (funcall next-error-function 0 nil)
> +    (recenter-top-bottom arg)
> +    (pop-to-buffer next-error-last-buffer)))

I wonder why restrict this only to occur and compilation?

Like C-M-v/C-M-S-v and M-PgUp/M-PgDown can scroll up/down other window
in any mode, you can use such code to recenter other window everywhere:

  (with-selected-window (other-window-for-scrolling) (recenter-top-bottom))

bug#46119: 28.0.50; this-error-recenter

[Tino Calancha]

Juri Linkov <juri@jurta.org> writes:

>> +(defun this-error-recenter (&optional arg)
>> +  "Recenter the current displayed error in the `next-error' buffer."
>> +  (interactive "P")
>> +  (if (not (or (eq major-mode 'occur-mode)
>> +               (derived-mode-p 'compilation-mode)))
>> +      (user-error "This command is for *Occur* or *Grep* buffers")
>> +    (funcall next-error-function 0 nil)
>> +    (recenter-top-bottom arg)
>> +    (pop-to-buffer next-error-last-buffer)))
>
> I wonder why restrict this only to occur and compilation?
I think because I was blindly looking only to my particular use case: Occur/Grep.

> Like C-M-v/C-M-S-v and M-PgUp/M-PgDown can scroll up/down other window
> in any mode, you can use such code to recenter other window everywhere:
>
>   (with-selected-window (other-window-for-scrolling) (recenter-top-bottom))

Yeah, it makes sense.  The only thing is that it makes harder to find
the right name for this command;  I think by improving the docstring
makes more clear what is this about.
How does it look now?:

(defun this-error-recenter (&optional arg)
  "Recenter the current displayed error in the `next-error' buffer.
If called not from a `next-error' buffer, then it just calls
`recenter-top-bottom' in the other window."
  (interactive "P")
  (if (not (or (eq major-mode 'occur-mode) (derived-mode-p 'compilation-mode)))
      (with-selected-window (other-window-for-scrolling) (recenter-top-bottom arg))
    (funcall next-error-function 0 nil)
    (recenter-top-bottom arg)
    (pop-to-buffer next-error-last-buffer)))

bug#46119: 28.0.50; this-error-recenter

[Juri Linkov]

>> Like C-M-v/C-M-S-v and M-PgUp/M-PgDown can scroll up/down other window
>> in any mode, you can use such code to recenter other window everywhere:
>>
>>   (with-selected-window (other-window-for-scrolling) (recenter-top-bottom))
>
> Yeah, it makes sense.  The only thing is that it makes harder to find
> the right name for this command;  I think by improving the docstring
> makes more clear what is this about.

Indeed, then it has no relation to errors at all.

Like the existing command scroll-other-window-down bound to 'S-C-M-v',
a new command could be named recenter-other-window bound to 'S-C-l'.

> How does it look now?:
>
> (defun this-error-recenter (&optional arg)
>   "Recenter the current displayed error in the `next-error' buffer.
> If called not from a `next-error' buffer, then it just calls
> `recenter-top-bottom' in the other window."
>   (interactive "P")
>   (if (not (or (eq major-mode 'occur-mode) (derived-mode-p 'compilation-mode)))
>       (with-selected-window (other-window-for-scrolling) (recenter-top-bottom arg))
>     (funcall next-error-function 0 nil)
>     (recenter-top-bottom arg)
>     (pop-to-buffer next-error-last-buffer)))

But then no special case is needed for occur/compilation-mode.

bug#46119: 28.0.50; this-error-recenter

[Tino Calancha]

On Sat, 30 Jan 2021, Juri Linkov wrote:

> Like the existing command scroll-other-window-down bound to 'S-C-M-v',
> a new command could be named recenter-other-window bound to 'S-C-l'.
I like that.

>> (defun this-error-recenter (&optional arg)
>>   "Recenter the current displayed error in the `next-error' buffer.
>> If called not from a `next-error' buffer, then it just calls
>> `recenter-top-bottom' in the other window."
>>   (interactive "P")
>>   (if (not (or (eq major-mode 'occur-mode) (derived-mode-p 'compilation-mode)))
>>       (with-selected-window (other-window-for-scrolling) (recenter-top-bottom arg))
>>     (funcall next-error-function 0 nil)
>>     (recenter-top-bottom arg)
>>     (pop-to-buffer next-error-last-buffer)))
>
> But then no special case is needed for occur/compilation-mode.

I expected someone will ask that; I should write my reason in the previous 
email: I like to see the pulse highlight, as with `next-error-no-select', 
that's why I call `next-error-function' here.
[I customize `next-error-highlight' and `next-error-highlight-no-select' 
to 3 s]

bug#46119: 28.0.50; this-error-recenter

[Juri Linkov]

>> Like the existing command scroll-other-window-down bound to 'S-C-M-v',
>> a new command could be named recenter-other-window bound to 'S-C-l'.
> I like that.
>
>> But then no special case is needed for occur/compilation-mode.
>
> I expected someone will ask that; I should write my reason in the previous
> email: I like to see the pulse highlight, as with `next-error-no-select',
> that's why I call `next-error-function' here.
> [I customize `next-error-highlight' and `next-error-highlight-no-select' to
> 3 s]

Woundn't such pulse highlighting be useful for all cases,
not only compilation and occur?

Then you can use:

  (with-selected-window (other-window-for-scrolling)
    (recenter-top-bottom)
    (pulse-momentary-highlight-one-line (point)))

and customize pulse-delay to 3 s, and pulse-iterations to 1.

bug#46119: 28.0.50; this-error-recenter

[Tino Calancha]

Juri Linkov <juri@jurta.org> writes:

>> I like to see the pulse highlight, as with `next-error-no-select',
>> that's why I call `next-error-function' here.
>> [I customize `next-error-highlight' and `next-error-highlight-no-select' to
>> 3 s]
>
> Woundn't such pulse highlighting be useful for all cases,
> not only compilation and occur?
Yes, that helps to drive the eye to the position of the line.

> Then you can use:
>
>   (with-selected-window (other-window-for-scrolling)
>     (recenter-top-bottom)
>     (pulse-momentary-highlight-one-line (point)))
>
> and customize pulse-delay to 3 s, and pulse-iterations to 1.

There is a problem: It is not guaranteed that
Occur/Grep will scroll the right target window.  Users might have
divided the frame in 3 or more windows.

For Occur/Grep, to ensure we scroll the right window, we would need to
let bind `other-window-scroll-buffer' to the right targets (available at
*Occur* and *Grep* as text properties); that part complicates the code.


The original implementation, i.e.:
+    (funcall next-error-function 0 nil)
+    (recenter-top-bottom arg)
+    (pop-to-buffer next-error-last-buffer)))

automatically takes care of finding the target.

I'd prefer adding them as two separated entities:
- recenter-other-window:
  as you have suggested, including the pulse.

- recenter-this-error (or this-error-recenter):
  For occur/grep with the original proposal.

Another small benefit is that for occur/grep, the pulse highlight
automatically behaves the same as in `next-error-no-select':
it matches only the occurrence/error.

Do you agree with adding two separated commands?

bug#46119: 28.0.50; this-error-recenter

[Juri Linkov]

>>   (with-selected-window (other-window-for-scrolling)
>>     (recenter-top-bottom)
>>     (pulse-momentary-highlight-one-line (point)))
>>
>> and customize pulse-delay to 3 s, and pulse-iterations to 1.
>
> There is a problem: It is not guaranteed that
> Occur/Grep will scroll the right target window.  Users might have
> divided the frame in 3 or more windows.
>
> For Occur/Grep, to ensure we scroll the right window, we would need to
> let bind `other-window-scroll-buffer' to the right targets (available at
> *Occur* and *Grep* as text properties); that part complicates the code.

This is exactly what we need for bug#45688 where other-window-scroll-buffer
could help to find the previous-window where the previous error
or grep hit was displayed, and display the next error in the same window.

> The original implementation, i.e.:
> +    (funcall next-error-function 0 nil)
> +    (recenter-top-bottom arg)
> +    (pop-to-buffer next-error-last-buffer)))
>
> automatically takes care of finding the target.
>
> I'd prefer adding them as two separated entities:
> - recenter-other-window:
>   as you have suggested, including the pulse.
>
> - recenter-this-error (or this-error-recenter):
>   For occur/grep with the original proposal.
>
> Another small benefit is that for occur/grep, the pulse highlight
> automatically behaves the same as in `next-error-no-select':
> it matches only the occurrence/error.
>
> Do you agree with adding two separated commands?

Maybe two commands is fine.  Then the users can decide what command
better suites user's needs.

bug#46119: 28.0.50; this-error-recenter

[Juri Linkov]

>> Do you agree with adding two separated commands?
>
> Maybe two commands is fine.  Then the users can decide what command
> better suites user's needs.

Actually everything is much simpler.  The hint is in the default value
of 'xref-after-jump-hook' that contains 'recenter' by default.
So when you customize 'xref-after-jump-hook' to the value:

  (recenter-top-bottom xref-pulse-momentarily)

you can use 'C-o' to recenter another window from xref buffer,
exactly like 'C-l' does for the selected window.

The same way you can add:

  (add-hook 'next-error-hook 'recenter-top-bottom)

then use 'C-o' in any next-error capable buffer (compilation, occur, ...)
to recenter its corresponding window exactly like you want.

bug#46119: 28.0.50; this-error-recenter

[Tino Calancha]

On Mon, 1 Feb 2021, Juri Linkov wrote:

> Actually everything is much simpler.  The hint is in the default value
> of 'xref-after-jump-hook' that contains 'recenter' by default.
> So when you customize 'xref-after-jump-hook' to the value:
>
>  (recenter-top-bottom xref-pulse-momentarily)
>
> you can use 'C-o' to recenter another window from xref buffer,
> exactly like 'C-l' does for the selected window.
>
> The same way you can add:
>
>  (add-hook 'next-error-hook 'recenter-top-bottom)
>
> then use 'C-o' in any next-error capable buffer (compilation, occur, ...)
> to recenter its corresponding window exactly like you want.

Thank you, that's nice. I still see superior adding a new command 
for 2 resons.

- I like to provide the binding by default to the users; same as we do 
with `n' or `p' to navigate the matches without selecting the buffer.  It 
is quite convenient.

- The hook above doesn't accept a prefix.  With the original proposal I 
can do:
2 l ; scroll the current displayed match at the second line.
This become handy when you want to inspect the context of the match.

bug#46119: 28.0.50; this-error-recenter

[Juri Linkov]

>>  (add-hook 'next-error-hook 'recenter-top-bottom)
>>
>> then use 'C-o' in any next-error capable buffer (compilation, occur, ...)
>> to recenter its corresponding window exactly like you want.
>
> Thank you, that's nice. I still see superior adding a new command for
> 2 resons.
>
> - I like to provide the binding by default to the users; same as we do with
>   `n' or `p' to navigate the matches without selecting the buffer.  It is
>  quite convenient.
>
> - The hook above doesn't accept a prefix.  With the original proposal I can
>  do:
> 2 l ; scroll the current displayed match at the second line.
> This become handy when you want to inspect the context of the match.

Is it possible to add a prefix to `C-o'?  What I'm worried about is
two commands `C-o' and `l' doing the same thing.  Or maybe these
commands are used for different purposes?

bug#46119: 28.0.50; this-error-recenter

[Tino Calancha]

Juri Linkov <juri@jurta.org> writes:


> Is it possible to add a prefix to `C-o'?  What I'm worried about is
> two commands `C-o' and `l' doing the same thing.  Or maybe these
> commands are used for different purposes?

I see your point.  After adding the new command, `C-o' becomes
an idempotent subset of `l'.  Conceptually they are still different things.

bug#46119: 28.0.50; this-error-recenter

[Tino Calancha]

Juri Linkov <juri@jurta.org> writes:

>> Do you agree with adding two separated commands?
>
> Maybe two commands is fine.  Then the users can decide what command
> better suites user's needs.

I am posting now the approach using two new commands:

--8<-----------------------------cut here---------------start------------->8---
commit 16ea65701456ce7071f56d337f518d49a03e723f
Author: Tino Calancha <ccalancha@suse.com>
Date:   Mon Feb 1 20:07:22 2021 +0100

    Add command to recenter errors from Occur/Grep buffers
    
    To scroll up/down the current displayed occurrence/error
    without abandon the Occur/Grep buffer.
    
    Add also a command 'recenter-other-window' to recenter
    the other window from any kind of buffer.
    
    * lisp/window.el (recenter-other-window): New command.
    Bind recenter-other-window to M-C-l.
    
    * lisp/simple.el (this-error-recenter): New command.
    * lisp/progmodes/grep.el (grep-mode-map):
    Delete bidings for n and p.
    
    * lisp/progmodes/compile.el (compilation-minor-mode-map):
    Move here the n and p bindings.
    Bind `this-error-recenter' to l.
    * lisp/replace.el (occur-mode-map):
    Same.
    
    * etc/NEWS (Changes in Specialized Modes and Packages in Emacs 28.1):
    Announce the changes.

diff --git a/etc/NEWS b/etc/NEWS
index e038076e96..dbe53b3f29 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -85,6 +85,8 @@ useful on systems such as FreeBSD which ships only with "etc/termcap".
 \f
 * Changes in Emacs 28.1
 
+** The new command 'recenter-other-window' is bound to 'M-C-l'.
+
 ** The new NonGNU ELPA archive is enabled by default alongside GNU ELPA
 
 ** Minibuffer scrolling is now conservative by default.
@@ -454,9 +456,15 @@ applied when the option 'tab-line-tab-face-functions' is
 so-configured.  That option may also be used to customize tab-line
 faces in other ways.
 
-** New bindings in occur-mode, 'next-error-no-select' bound to 'n' and
+** Occur mode
+
+*** New bindings in occur-mode, 'next-error-no-select' bound to 'n' and
 'previous-error-no-select' bound to 'p'.
 
+*** The new command 'this-error-recenter', bound to 'l', recenters the
+current displayed occurrence from a Occur buffer and the current
+displayed error in a compilation buffer.
+
 ** EIEIO
 
 +++
diff --git a/lisp/progmodes/compile.el b/lisp/progmodes/compile.el
index 94e4f3c6fa..1031f835f8 100644
--- a/lisp/progmodes/compile.el
+++ b/lisp/progmodes/compile.el
@@ -2073,6 +2073,10 @@ compilation-minor-mode-map
     (define-key map "\M-p" 'compilation-previous-error)
     (define-key map "\M-{" 'compilation-previous-file)
     (define-key map "\M-}" 'compilation-next-file)
+    (define-key map "n" 'next-error-no-select)
+    (define-key map "p" 'previous-error-no-select)
+    (define-key map "l" 'this-error-recenter)
+
     (define-key map "g" 'recompile) ; revert
     ;; Set up the menu-bar
     (define-key map [menu-bar compilation]
diff --git a/lisp/progmodes/grep.el b/lisp/progmodes/grep.el
index 1a8435fde3..d6ee8bb423 100644
--- a/lisp/progmodes/grep.el
+++ b/lisp/progmodes/grep.el
@@ -275,8 +275,6 @@ grep-mode-map
     (define-key map "\C-c\C-f" 'next-error-follow-minor-mode)
 
     (define-key map "\r" 'compile-goto-error)  ;; ?
-    (define-key map "n" 'next-error-no-select)
-    (define-key map "p" 'previous-error-no-select)
     (define-key map "{" 'compilation-previous-file)
     (define-key map "}" 'compilation-next-file)
     (define-key map "\t" 'compilation-next-error)
diff --git a/lisp/replace.el b/lisp/replace.el
index db5b340631..bd3a96a26e 100644
--- a/lisp/replace.el
+++ b/lisp/replace.el
@@ -1135,6 +1135,7 @@ occur-mode-map
     (define-key map "\C-o" 'occur-mode-display-occurrence)
     (define-key map "n" 'next-error-no-select)
     (define-key map "p" 'previous-error-no-select)
+    (define-key map "l" 'this-error-recenter)
     (define-key map "\M-n" 'occur-next)
     (define-key map "\M-p" 'occur-prev)
     (define-key map "r" 'occur-rename-buffer)
diff --git a/lisp/simple.el b/lisp/simple.el
index c878fdab92..31a2ac3c88 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -496,6 +496,16 @@ next-error-message-highlight
         (overlay-put ol 'window (get-buffer-window))
         (setf next-error--message-highlight-overlay ol)))))
 
+(defun this-error-recenter (&optional arg)
+  "Recenter the current displayed error in the `next-error' buffer."
+  (interactive "P")
+  (if (not (or (eq major-mode 'occur-mode) (derived-mode-p 'compilation-mode)))
+      (user-error "This command is for *Occur* or *Grep* buffers")
+    (funcall next-error-function 0 nil)
+    (recenter-top-bottom arg)
+    (pop-to-buffer next-error-last-buffer)))
+
+
 \f
 ;;;
 
diff --git a/lisp/window.el b/lisp/window.el
index 0a37d16273..c75ed90ef5 100644
--- a/lisp/window.el
+++ b/lisp/window.el
@@ -9771,6 +9771,19 @@ recenter-top-bottom
 
 (define-key global-map [?\C-l] 'recenter-top-bottom)
 
+(defun recenter-other-window (&optional arg)
+  "Call `recenter-top-bottom' in the other window.
+
+A prefix argument is handled like `recenter':
+ With numeric prefix ARG, move current line to window-line ARG.
+ With plain `C-u', move current line to window center."
+  (interactive "P")
+  (with-selected-window (other-window-for-scrolling)
+    (recenter-top-bottom arg)
+    (pulse-momentary-highlight-one-line (point))))
+
+(define-key global-map [?\M-\C-l] 'recenter-other-window)
+
 (defun move-to-window-line-top-bottom (&optional arg)
   "Position point relative to window.
 

--8<-----------------------------cut here---------------end--------------->8---

bug#46119: 28.0.50; this-error-recenter

[Juri Linkov]

> I am posting now the approach using two new commands:
>
>     * lisp/window.el (recenter-other-window): New command.
>     Bind recenter-other-window to M-C-l.

M-C-l can't be used because M-C-l is already bound to 'reposition-window'.
Only S-M-C-l is free.

>     * lisp/simple.el (this-error-recenter): New command.

Wouldn't a better name be recenter-current-error
since existing name is compilation-current-error?

> +(defun this-error-recenter (&optional arg)
> +  "Recenter the current displayed error in the `next-error' buffer."
> +  (interactive "P")
> +  (if (not (or (eq major-mode 'occur-mode) (derived-mode-p 'compilation-mode)))
> +      (user-error "This command is for *Occur* or *Grep* buffers")

This is bad restriction that needs to be removed.

> +    (funcall next-error-function 0 nil)
> +    (recenter-top-bottom arg)
> +    (pop-to-buffer next-error-last-buffer)))

`(pop-to-buffer next-error-last-buffer)' has problems
and should be replaced with `save-selected-window'.

Please see the commit 072b4c679dfd5528e74849cad18246730a991933
and bug#32607 for explanations.

bug#46119: 28.0.50; this-error-recenter

[Tino Calancha]

Juri Linkov <juri@jurta.org> writes:

>>     * lisp/window.el (recenter-other-window): New command.
>>     Bind recenter-other-window to M-C-l.
>
> M-C-l can't be used because M-C-l is already bound to 'reposition-window'.
> Only S-M-C-l is free.
Right, my bad.  Ok, I will use S-M-C-l then.

>>     * lisp/simple.el (this-error-recenter): New command.
>
> Wouldn't a better name be recenter-current-error
> since existing name is compilation-current-error?
Agreed.

>> +(defun this-error-recenter (&optional arg)
>> +  "Recenter the current displayed error in the `next-error' buffer."
>> +  (interactive "P")
>> +  (if (not (or (eq major-mode 'occur-mode) (derived-mode-p 'compilation-mode)))
>> +      (user-error "This command is for *Occur* or *Grep* buffers")
>
> This is bad restriction that needs to be removed.
OK

>> +    (funcall next-error-function 0 nil)
>> +    (recenter-top-bottom arg)
>> +    (pop-to-buffer next-error-last-buffer)))
>
> `(pop-to-buffer next-error-last-buffer)' has problems
> and should be replaced with `save-selected-window'.
>
> Please see the commit 072b4c679dfd5528e74849cad18246730a991933
> and bug#32607 for explanations.

I must set the buffer with the locus current, otherwise `recenter' rises
an error:

(defun recenter-current-error (&optional arg)
  "Recenter the current displayed error in the `next-error' buffer."
  (interactive "P")
  (save-selected-window
    (let ((next-error-highlight next-error-highlight-no-select)
          (display-buffer-overriding-action
           '(nil (inhibit-same-window . t))))
      (next-error 0)
      (set-buffer (window-buffer))
      (recenter-top-bottom arg))))

bug#46119: 28.0.50; this-error-recenter

[Juri Linkov]

> I must set the buffer with the locus current, otherwise `recenter' rises
> an error:
>
> (defun recenter-current-error (&optional arg)
>   "Recenter the current displayed error in the `next-error' buffer."
>   (interactive "P")
>   (save-selected-window
>     (let ((next-error-highlight next-error-highlight-no-select)
>           (display-buffer-overriding-action
>            '(nil (inhibit-same-window . t))))
>       (next-error 0)
>       (set-buffer (window-buffer))
>       (recenter-top-bottom arg))))

Would it be possible to avoid code duplication between
next-error-no-select and recenter-current-error?

bug#46119: 28.0.50; this-error-recenter

[Tino Calancha]

Juri Linkov <juri@jurta.org> writes:

> Would it be possible to avoid code duplication between
> next-error-no-select and recenter-current-error?

How about adding this macro?

(defmacro with-selected-locus (n &rest body)
  "Visit the Nth error and execute BODY, then select the previously selected window."
  (declare (indent 1) (debug (form body)))
  `(save-selected-window
     (let ((next-error-highlight next-error-highlight-no-select)
           (display-buffer-overriding-action
            '(nil (inhibit-same-window . t))))
       (next-error ,n)
       ,@body)))

(defun next-error-no-select (&optional n)
  "Move point to the next error in the `next-error' buffer and highlight match.
Prefix arg N says how many error messages to move forwards (or
backwards, if negative).
Finds and highlights the source line like \\[next-error], but does not
select the source buffer."
  (interactive "p")
  (with-selected-locus n))

(defun recenter-current-error (&optional arg)
  "Recenter the current displayed error in the `next-error' buffer."
  (interactive "P")
  (with-selected-locus 0
    (set-buffer (window-buffer))
    (recenter-top-bottom arg)))

bug#46119: 28.0.50; this-error-recenter

[Juri Linkov]

>> Would it be possible to avoid code duplication between
>> next-error-no-select and recenter-current-error?
>
> How about adding this macro?
>
> (defmacro with-selected-locus (n &rest body)
>
> (defun next-error-no-select (&optional n)
>...
>   (with-selected-locus n))

I don't know, a macro call with empty body doesn't look nice.  I guess
then better to have code duplication than such macro.

bug#46119: 28.0.50; this-error-recenter

[Tino Calancha]

Juri Linkov <juri@jurta.org> writes:

>>> Would it be possible to avoid code duplication between
>>> next-error-no-select and recenter-current-error?
>>
>> How about adding this macro?
>>
>> (defmacro with-selected-locus (n &rest body)
>>
>> (defun next-error-no-select (&optional n)
>>...
>>   (with-selected-locus n))
>
> I don't know, a macro call with empty body doesn't look nice.  I guess
> then better to have code duplication than such macro.

Thank you. Then, I have chosen the previous one without the macro.

Pushed into master as commit
"Add command to recenter errors from Occur/Grep buffers"
(9380a7ed906e667df4fc5b9d9c8e487fafa7c654)

bug#46119: 28.0.50; this-error-recenter

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

  > As a regular greper with a tiny screen I tend to use this command quite often.

  > Tentatively, I have named it `this-error-recenter'; if you find a
  > better name for it, please let me know.

Would you please say what the command does?  I see this

      > Add command to recenter errors from Occur/Grep buffers
    
      > It allows to scroll up/down the current displayed occurrence/error
      > without abandon the Occur/Grep buffer.

but I am not sure what that means, concretely.

-- 
Dr Richard Stallman
Chief GNUisance of the GNU Project (https://gnu.org)
Founder, Free Software Foundation (https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)

bug#46119: 28.0.50; this-error-recenter

[Tino Calancha]

Richard Stallman <rms@gnu.org> writes:

>   > As a regular greper with a tiny screen I tend to use this command quite often.
>
>   > Tentatively, I have named it `this-error-recenter'; if you find a
>   > better name for it, please let me know.
>
> Would you please say what the command does?  I see this
>
>       > Add command to recenter errors from Occur/Grep buffers
>     
>       > It allows to scroll up/down the current displayed occurrence/error
>       > without abandon the Occur/Grep buffer.
>
> but I am not sure what that means, concretely.

;; Grep example (Call Emacs from the root Emacs source code using my patch)
emacs -Q lisp
M-x rgrep RET (defun RET *.el RET RET
C-x o
n ; display errors w/o selecting its window
l ; scroll up/down the current displayed error

;; Occur example
emacs -Q lisp/subr.el
M-s o (defun RET
C-x o
n ; display occurrences w/o selecting its window
l ; scroll up/down the current displayed occurrence