bug#28864: 25.3.50; next-error-no-select does select

[Juri Linkov]

>> I believe this patch is a change for the better:
>
> Could you resend it as unified diff? Or push to
> a branch. diff-context->unified doesn't seem to work on the patch you sent.

I thought maybe context diff is easier to read in this case,
but here is unified diff below.

>> 1. makes next-error-function buffer-local
>
> Interesting.
>
>> 2. sets both buffer-local and global values
>
> Why? If next-error-find-buffer-function can find the appropriate buffer, it
> can use the local value in it.

We need the global value as well to get the last next-error buffer
when there is no buffer-local value yet in the current buffer.

>> 3. adds customizable next-error-find-buffer-function
>>
>> 4. moves window-on-frame-visibility code to separate function
>>     that can be used to customize for backward-compatibility
>
> Do we get a built-in alternative to this value? If we just provide
> a customization point, that nice, but not a significant improvement.

You mean to remove next-error-buffer-on-selected-frame?
Or maybe to do the other way: add more predefined functions like
a function to use window-local navigation as an alternative.

>> 5. adds next-error-select-buffer to manually switch to another
>>     next-error capable buffer
>
> Not having tried the patch yet... Will the user have to do that after
> semi-accidentally opening a changelog-mode buffer, in order not to switch
> to its error list?

When the user visits a ChangeLog buffer from *grep*, then
next-error continues the *grep* navigation.  When the user
visit a ChangeLog file by any other command, then next-error
navigates entries in that ChangeLog buffer.

But in case when the user changes mind and decides to switch
from *grep* navigation to ChangeLog navigation, there is
this new command.

>> 6. message to show which next-error buffer is currently used
>
> Every time we call `next-error'?

That's right.

diff --git a/lisp/simple.el b/lisp/simple.el
index 12d65e5..55de17d 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -143,6 +143,7 @@ next-error-last-buffer
 A buffer becomes most recent when its compilation, grep, or
 similar mode is started, or when it is used with \\[next-error]
 or \\[compile-goto-error].")
+(make-variable-buffer-local 'next-error-function)
 
 (defvar next-error-function nil
   "Function to use to find the next error in the current buffer.
@@ -191,6 +192,30 @@ next-error-buffer-p
           (and extra-test-inclusive
                (funcall extra-test-inclusive))))))
 
+(defcustom next-error-find-buffer-function nil
+  "Function called to find a `next-error' capable buffer."
+  :type '(choice (const :tag "Buffer on selected frame" 
next-error-buffer-on-selected-frame)
+                 (const :tag "No default" nil)
+                 (function :tag "Other function"))
+  :group 'next-error
+  :version "27.1")
+
+(defun next-error-buffer-on-selected-frame (&optional avoid-current
+                                                      extra-test-inclusive
+                                                      extra-test-exclusive)
+  "Return a single visible next-error buffer on the selected frame."
+  (let ((window-buffers
+         (delete-dups
+          (delq nil (mapcar (lambda (w)
+                              (if (next-error-buffer-p
+                                  (window-buffer w)
+                                   avoid-current
+                                   extra-test-inclusive extra-test-exclusive)
+                                  (window-buffer w)))
+                            (window-list))))))
+    (if (eq (length window-buffers) 1)
+        (car window-buffers))))
+
 (defun next-error-find-buffer (&optional avoid-current
                                         extra-test-inclusive
                                         extra-test-exclusive)
@@ -207,18 +232,11 @@ next-error-find-buffer
 that would normally be considered usable.  If it returns nil,
 that buffer is rejected."
   (or
-   ;; 1. If one window on the selected frame displays such buffer, return it.
-   (let ((window-buffers
-          (delete-dups
-           (delq nil (mapcar (lambda (w)
-                               (if (next-error-buffer-p
-                                   (window-buffer w)
-                                    avoid-current
-                                    extra-test-inclusive extra-test-exclusive)
-                                   (window-buffer w)))
-                             (window-list))))))
-     (if (eq (length window-buffers) 1)
-         (car window-buffers)))
+   ;; 1. If a customizable function returns a buffer, use it.
+   (when next-error-find-buffer-function
+     (funcall next-error-find-buffer-function avoid-current
+                                              extra-test-inclusive
+                                              extra-test-exclusive))
    ;; 2. If next-error-last-buffer is an acceptable buffer, use that.
    (if (and next-error-last-buffer
             (next-error-buffer-p next-error-last-buffer avoid-current
@@ -279,23 +297,43 @@ next-error
 `compilation-error-regexp-alist'."
   (interactive "P")
   (if (consp arg) (setq reset t arg nil))
-  (when (setq next-error-last-buffer (next-error-find-buffer))
+  (let ((next-error-buffer (next-error-find-buffer)))
+    (when next-error-buffer
+      ;; we know here that next-error-function is a valid symbol we can funcall
+      (with-current-buffer next-error-last-buffer
+        (funcall next-error-function (prefix-numeric-value arg) reset)
+        ;; next-error-function might change the value of
+        ;; next-error-last-buffer, so set it later
+        (setq next-error-last-buffer next-error-buffer)
+        (setq-default next-error-last-buffer next-error-last-buffer)
+        (when next-error-recenter
+          (recenter next-error-recenter))
+        (message "Next error buffer from %s" next-error-last-buffer)
+        (run-hooks 'next-error-hook)))))
+
+(defun next-error-internal ()
+  "Visit the source code corresponding to the `next-error' message at point."
+  (let ((next-error-buffer (current-buffer)))
     ;; we know here that next-error-function is a valid symbol we can funcall
-    (with-current-buffer next-error-last-buffer
-      (funcall next-error-function (prefix-numeric-value arg) reset)
+    (with-current-buffer next-error-buffer
+      (funcall next-error-function 0 nil)
+      ;; next-error-function might change the value of
+      ;; next-error-last-buffer, so set it later
+      (setq next-error-last-buffer next-error-buffer)
+      (setq-default next-error-last-buffer next-error-last-buffer)
       (when next-error-recenter
         (recenter next-error-recenter))
+      (message "Next error buffer from %s" next-error-last-buffer)
       (run-hooks 'next-error-hook))))
 
-(defun next-error-internal ()
-  "Visit the source code corresponding to the `next-error' message at point."
-  (setq next-error-last-buffer (current-buffer))
-  ;; we know here that next-error-function is a valid symbol we can funcall
-  (with-current-buffer next-error-last-buffer
-    (funcall next-error-function 0 nil)
-    (when next-error-recenter
-      (recenter next-error-recenter))
-    (run-hooks 'next-error-hook)))
+(defun next-error-select-buffer (next-error-buffer)
+  "Select a `next-error' capable buffer and set it as the last used."
+  (interactive
+   (list (get-buffer
+          (read-buffer "Select next-error buffer: " nil nil
+                       (lambda (b) (next-error-buffer-p (cdr b)))))))
+  (setq next-error-last-buffer next-error-buffer)
+  (setq-default next-error-last-buffer next-error-last-buffer))
 
 (defalias 'goto-next-locus 'next-error)
 (defalias 'next-match 'next-error)