next-error use cases

next-error use cases

[Juri Linkov]

Several bug reports such as bug#20489 and bug#30674 revolve around
the uncertainty caused by unclear requirements for the next-error framework.
It would be helpful to identify possible scenarios to get a clearer picture of
what it's supposed to do.

The cornerstone of the whole feature is the function next-error-find-buffer
that returns a next-error capable buffer that defines next-error-function
to navigate to the next location.

Most fallbacks in next-error-find-buffer are fine in case the first condition
fails, but the main question is about the first condition itself: What is the
best default behavior for it?  What other useful options it should provide?
What context should be used to select the right next-error-function buffer.

There are several alternatives of what buffer with next-error-function
should be selected after running next-error:

1. last next-error-function used anywhere globally
Pros: context-free, thus easier to understand
Cons: not WYSIWYG, i.e. in some cases produces unexpected results:
      for example, after switching to another next-error-function buffer
      will still continue the previous navigation from the hidden buffer
      with last used next-error-function

2. next-error-function found in the current buffer
Cons: unexpected results when navigation visits another buffer with next-error-function,
      e.g. when a ChangeLog file is visited by next-error from the *grep* buffer,
      the next next-error should continue visiting grep hits, not ChangeLog entries.
      (But when a next-error navigation never visited a next-error-function buffer, then
      running `next-error' in a next-error-function buffer should use it unconditionally,
      e.g. switching to a *compilation* buffer and running next-error should use it.)

3. next-error-function that visited the current buffer,
   i.e. buffer-local value of next-error-last-buffer remembers the last used
   next-error-function buffer in each visited buffer, and uses it for the next
   next-error.
Cons: unexpected results when the same buffer is visited in different windows
      from different next-error-function buffers

4. next-error-function that visited the buffer in the selected window
Pros: window-based logic gives more expected results

5. buffer with next-error-function visible in one of adjacent windows on the same frame
Pros: WYSIWYG
Cons: ambiguous when there are more than one window with next-error-function on the frame

What complicates matter more is that the value of next-error-function might
come from the same buffer where navigation occurs, i.e. unlike these cases
where next-error-function is defined in a special separate buffer:

compilation-next-error-function
xref--next-error-function
occur-next-error
change-log-next-error
diff-next-error

next-error-function can be defined in the same buffer, for example for:

flymake-goto-next-error
flyspell-goto-next-error

where the list of locations comes from the same buffer where next-error
navigates.

So the question is how one buffer can provide many next-error functions
by such different major/minor modes (flymake, flyspell) enabled in it
at the same time?

Re: next-error use cases

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Date: Sun, 08 Apr 2018 00:47:42 +0300
> 
> So the question is how one buffer can provide many next-error functions
> by such different major/minor modes (flymake, flyspell) enabled in it
> at the same time?

Define some kind of priority between them?  And if two or more
functions have the same priority, ask the user?

Re: next-error use cases

[Juri Linkov]

>> So the question is how one buffer can provide many next-error functions
>> by such different major/minor modes (flymake, flyspell) enabled in it
>> at the same time?
>
> Define some kind of priority between them?  And if two or more
> functions have the same priority, ask the user?

We could try to define priorities by ‘add-function’.

Re: next-error use cases

[Clément Pit-Claudel]

On 2018-04-08 15:56, Juri Linkov wrote:
>>> So the question is how one buffer can provide many next-error functions
>>> by such different major/minor modes (flymake, flyspell) enabled in it
>>> at the same time?
>>
>> Define some kind of priority between them?  And if two or more
>> functions have the same priority, ask the user?
> 
> We could try to define priorities by ‘add-function’.

Or we could query all backends and go to the closest location.  It would match with what the user sees on the screen: if I have a flyspell underline followed by a FlyCheck error, it would visit one then the other.

Re: next-error use cases

[Dmitry Gutov]

On 4/8/18 11:26 PM, Clément Pit-Claudel wrote:

> Or we could query all backends and go to the closest location.  It would match with what the user sees on the screen: if I have a flyspell underline followed by a FlyCheck error, it would visit one then the other.

I'm afraid that won't match the user's intention: if I just run Grep, I 
probably want to jump between its results, and not between Flyspell markers.

Re: next-error use cases

[Dmitry Gutov]

On 4/8/18 4:48 PM, Eli Zaretskii wrote:
>> From: Juri Linkov <juri@linkov.net>
>> Date: Sun, 08 Apr 2018 00:47:42 +0300
>>
>> So the question is how one buffer can provide many next-error functions
>> by such different major/minor modes (flymake, flyspell) enabled in it
>> at the same time?
> 
> Define some kind of priority between them?
I really doubt we can define priorities such that the user will always 
want to see the errors of some "kind" before the others.

Re: next-error use cases

[Eli Zaretskii]

> From: Clément Pit-Claudel <cpitclaudel@gmail.com>
> Date: Sun, 8 Apr 2018 16:26:59 -0400
> 
> > We could try to define priorities by ‘add-function’.
> 
> Or we could query all backends and go to the closest location.  It would match with what the user sees on the screen: if I have a flyspell underline followed by a FlyCheck error, it would visit one then the other.

I don't think "closest location" is a reliable indicator in this
case.  next-error exists to go to the next/previous locus, which could
be far away.

Re: next-error use cases

[Eli Zaretskii]

> From: Dmitry Gutov <dgutov@yandex.ru>
> Date: Sun, 8 Apr 2018 23:36:37 +0300
> Cc: emacs-devel@gnu.org
> 
> > Define some kind of priority between them?
> I really doubt we can define priorities such that the user will always 
> want to see the errors of some "kind" before the others.

Then maybe this problem has no workable solution at all, and we
shouldn't try running all those features from the same command.

Re: next-error use cases

[Charles A. Roelli]

> Date: Mon, 09 Apr 2018 17:48:20 +0300
> From: Eli Zaretskii <eliz@gnu.org>
>
> > > Define some kind of priority between them?
> > I really doubt we can define priorities such that the user will always 
> > want to see the errors of some "kind" before the others.
> 
> Then maybe this problem has no workable solution at all, and we
> shouldn't try running all those features from the same command.

We could try, but in cases of doubt, ask the user what "next-error"
function he wants to use (and provide some command for actively
switching between them).  For example, after calling "next-error" in
some dubious case, the user could be offered a list of functions to
choose between:

- next Compile error from *compile*<dir>
- next Occur result from *scratch*
- next Grep result from *grep*<dir2>
...

There could also be other interfaces to pick a next-error function (a
menu, or something like M-x list-buffers).

Re: next-error use cases

[Juri Linkov]

>> > > Define some kind of priority between them?
>> > I really doubt we can define priorities such that the user will always
>> > want to see the errors of some "kind" before the others.
>>
>> Then maybe this problem has no workable solution at all, and we
>> shouldn't try running all those features from the same command.
>
> We could try, but in cases of doubt, ask the user what "next-error"
> function he wants to use (and provide some command for actively
> switching between them).  For example, after calling "next-error" in
> some dubious case, the user could be offered a list of functions to
> choose between:
>
> - next Compile error from *compile*<dir>
> - next Occur result from *scratch*
> - next Grep result from *grep*<dir2>
> ...

Answering this after every next-error command would be too tedious for users,
and it's not clear how to reduce the frequency of such questions, i.e.
how to detect only dubious cases.

> There could also be other interfaces to pick a next-error function (a
> menu, or something like M-x list-buffers).

Recently we created such command with the name ‘next-error-select-buffer’.
Still too inconvenient for users to run this command explicitly.
We need next-error to be more DWIM.

Re: next-error use cases

[Stephen Leake]

Dmitry Gutov <dgutov@yandex.ru> writes:

> On 4/8/18 11:26 PM, Clément Pit-Claudel wrote:
>
>> Or we could query all backends and go to the closest location. It
>> would match with what the user sees on the screen: if I have a
>> flyspell underline followed by a FlyCheck error, it would visit one
>> then the other.
>
> I'm afraid that won't match the user's intention: if I just run Grep,
> I probably want to jump between its results, and not between Flyspell
> markers.

+1. I often have several compilation-mode buffers active (one from a
compile, one from a cross-reference tool, one from grep), and I expect
next-error to use the one I last chose.

I chose one by visiting the buffer, and using <ret> to go to an error.

So just be sure the user has a way to indicate which buffer/location
list is "current".

-- 
-- Stephe

Re: next-error use cases

[Stephen Leake]

Juri Linkov <juri@linkov.net> writes:

>>> > > Define some kind of priority between them?
>>> > I really doubt we can define priorities such that the user will always
>>> > want to see the errors of some "kind" before the others.
>>>
>>> Then maybe this problem has no workable solution at all, and we
>>> shouldn't try running all those features from the same command.
>>
>> We could try, but in cases of doubt, ask the user what "next-error"
>> function he wants to use (and provide some command for actively
>> switching between them).  For example, after calling "next-error" in
>> some dubious case, the user could be offered a list of functions to
>> choose between:
>>
>> - next Compile error from *compile*<dir>
>> - next Occur result from *scratch*
>> - next Grep result from *grep*<dir2>
>> ...
>
> Answering this after every next-error command would be too tedious for users,
> and it's not clear how to reduce the frequency of such questions, i.e.
> how to detect only dubious cases.
>
>> There could also be other interfaces to pick a next-error function (a
>> menu, or something like M-x list-buffers).
>
> Recently we created such command with the name ‘next-error-select-buffer’.
> Still too inconvenient for users to run this command explicitly.
> We need next-error to be more DWIM.

That's what 'compilation-last-buffer' is for; it indicates the buffer
containing the location list that is current.

As long as the user has a convenient way to set that, there's no
problem. In all my current use cases, it is first set by the command
that generated the location list. I can reset it by navigating to a
buffer, and using <return> to goto a location; after that, 'next-error'
continues using that list.

-- 
-- Stephe

RE: next-error use cases

[Drew Adams]

> just be sure the user has a way to indicate which buffer/location
> list is "current".

+1.  Any "handy DWIM" needs to also give users an easy
way to control things when they want to.

Smart software doesn't think it is smarter than its user
or that she never wants to express her will on the fly.

Re: next-error use cases

[Juri Linkov]

>> Recently we created such command with the name ‘next-error-select-buffer’.
>> Still too inconvenient for users to run this command explicitly.
>> We need next-error to be more DWIM.
>
> That's what 'compilation-last-buffer' is for; it indicates the buffer
> containing the location list that is current.

I don't understand what do you mean by indication?  Is it a visual
indication somewhere?  I grepped for 'compilation-last-buffer', but in
compile.el 'compilation-last-buffer' is an alias for 'next-error-last-buffer',
and grep.el contains commented out definition whose purpose I don't know:

  ;;;; TODO --- refine this!!
  ;; (defcustom grep-use-compilation-buffer t
  ;;   "When non-nil, grep specific commands update `compilation-last-buffer'.
  ;; This means that standard compile commands like \\[next-error] and \\[compile-goto-error]
  ;; can be used to navigate between grep matches (the default).
  ;; Otherwise, the grep specific commands like \\[grep-next-match] must
  ;; be used to navigate between grep matches."
  ;;   :type 'boolean
  ;;   :group 'grep)
  ;; override compilation-last-buffer
  (defvar grep-last-buffer nil
    "The most recent grep buffer.
  A grep buffer becomes most recent when you select Grep mode in it.
  Notice that using \\[next-error] or \\[compile-goto-error] modifies
  `compilation-last-buffer' rather than `grep-last-buffer'.")

> As long as the user has a convenient way to set that, there's no
> problem. In all my current use cases, it is first set by the command
> that generated the location list. I can reset it by navigating to a
> buffer, and using <return> to goto a location; after that, 'next-error'
> continues using that list.

But how can you reset it when the location list is in the same buffer
like in case of flyspell where locations of misspelled words are
in the same buffer where next-error navigates, or flymake where
locations of errors/warnings are in the same buffer?

Re: next-error use cases

[Dmitry Gutov]

On 4/9/18 5:48 PM, Eli Zaretskii wrote:
>> From: Dmitry Gutov <dgutov@yandex.ru>
>> Date: Sun, 8 Apr 2018 23:36:37 +0300
>> Cc: emacs-devel@gnu.org
>>
>>> Define some kind of priority between them?
>> I really doubt we can define priorities such that the user will always
>> want to see the errors of some "kind" before the others.
> 
> Then maybe this problem has no workable solution at all, and we
> shouldn't try running all those features from the same command.

We already do, though. So far we've come up with a manual switching 
solution (see next-error-select-buffer in master), and it's fine to a 
degree.

Having Flymake and Flyspell use the same mechanism shows its 
inefficiencies, however, so that's what I think this discussion is 
mostly about.

Re: next-error use cases

[Dmitry Gutov]

On 4/11/18 6:26 AM, Stephen Leake wrote:

>> But how can you reset it when the location list is in the same buffer
>> like in case of flyspell where locations of misspelled words are
>> in the same buffer where next-error navigates, or flymake where
>> locations of errors/warnings are in the same buffer?
> 
> Position point on an error message, hit enter.

The error message is usually visible only in the minibuffer, and only 
when point is over the error.

Re: next-error use cases

[Juri Linkov]

>>> But how can you reset it when the location list is in the same buffer
>>> like in case of flyspell where locations of misspelled words are
>>> in the same buffer where next-error navigates, or flymake where
>>> locations of errors/warnings are in the same buffer?
>>
>> Position point on an error message, hit enter.
>
> The error message is usually visible only in the minibuffer, and only when
> point is over the error.

If it's impossible to reset it by hitting enter, then I think we have
only these remaining options:

1. If next-error can't work without the requirement to have a list of
Flyspell mistakes or Flycheck/Flymake errors/warnings printed in a separate
buffer (like "*Occur*"), then such buffer can be possibly implemented
as a hidden buffer with a leading space in its name like for example
" *Flycheck locations*".  ‘next-error-select-buffer’ still should
propose it as a candidate for selection.

2. Flycheck provides its own keybinding ‘C-c ! n’ for ‘flycheck-next-error’
that can be used to reset the navigation.  There are also analogous functions
‘flymake-goto-next-error’ and ‘flyspell-goto-next-error’.

3. Maybe providing only the command next-error-select-buffer to switch
the current navigation is enough even in case when next-error-function
is defined in the same buffer that will be proposed as a candidate.

4. Otherwise, if all else fails, try harder to use visibility as DWIM,
i.e. hide all other navigation windows to start using Flycheck
navigation from the current buffer.

Re: next-error use cases

[Stephen Leake]

Dmitry Gutov <dgutov@yandex.ru> writes:

> On 4/11/18 6:26 AM, Stephen Leake wrote:
>
>>> But how can you reset it when the location list is in the same buffer
>>> like in case of flyspell where locations of misspelled words are
>>> in the same buffer where next-error navigates, or flymake where
>>> locations of errors/warnings are in the same buffer?
>>
>> Position point on an error message, hit enter.
>
> The error message is usually visible only in the minibuffer, and only
> when point is over the error.

Ok. So add a keybinding that says "there is an error under point; use
the list it is part of as the current error list".

Presumably you already have "goto error under point"; that should also
set the current list.

(How do you notice there is an error there, so you know to position point?
a marker in the margin?)

I gather there might be more than one such list in a single buffer, so
compilation-last-buffer is insufficient.

You might be able to extend that; if it is a cons, the car is the
buffer, the car the root list element, or the list type tag, or
something.

-- 
-- Stephe

Re: next-error use cases

[Stephen Leake]

Juri Linkov <juri@linkov.net> writes:

>>>> But how can you reset it when the location list is in the same buffer
>>>> like in case of flyspell where locations of misspelled words are
>>>> in the same buffer where next-error navigates, or flymake where
>>>> locations of errors/warnings are in the same buffer?
>>>
>>> Position point on an error message, hit enter.
>>
>> The error message is usually visible only in the minibuffer, and only when
>> point is over the error.
>
> If it's impossible to reset it by hitting enter, then I think we have
> only these remaining options:
>
> 1. If next-error can't work without the requirement to have a list of
> Flyspell mistakes or Flycheck/Flymake errors/warnings printed in a separate
> buffer (like "*Occur*"), then such buffer can be possibly implemented
> as a hidden buffer with a leading space in its name like for example
> " *Flycheck locations*".  ‘next-error-select-buffer’ still should
> propose it as a candidate for selection.

That can work, as long as the buffer is truly hidden. The leading space
disables font-lock, so errors don't look like errors. And the leading
space may have other surprising consequences.

> 2. Flycheck provides its own keybinding ‘C-c ! n’ for ‘flycheck-next-error’
> that can be used to reset the navigation.  There are also analogous functions
> ‘flymake-goto-next-error’ and ‘flyspell-goto-next-error’.

It is not "goto-next-error" that should set the list, but rather
"goto-error-at-point". That's what "enter" does in a compilation-mode buffer.

"goto-next-error" can be invoked from some far away place, but must
refer to the current error list.

-- 
-- Stephe

Re: next-error use cases

[Dmitry Gutov]

On 4/12/18 6:25 PM, Stephen Leake wrote:

> Ok. So add a keybinding that says "there is an error under point; use
> the list it is part of as the current error list".

A new key binding or several is an obvious solution. It's also the 
clunkiest one (one: key bindings are precious, two: that means extra key 
presses to use, and three: extra stuff for the users to memorize).

So of course we can fall back to this approach, but it would be great to 
discuss something less obvious first.

> Presumably you already have "goto error under point"; that should also
> set the current list.
> 
> (How do you notice there is an error there, so you know to position point?
> a marker in the margin?)

The error is not always near point, or even visible in the window. To 
get to it, we need a key binding. So, next-error or previous-error 
working already would be helpful.

> I gather there might be more than one such list in a single buffer, so
> compilation-last-buffer is insufficient.

That's a problem for another day, I think. Lists in the same buffer can 
probably be combined anyway.

Re: next-error use cases

[Juri Linkov]

Since we removed the rule of visibility of next-error-function buffers
as a criteria for deciding what navigation to use for next-error,
so e.g. next-error in *Occur* or *Diff* that is the current buffer
and a single visible buffer on the frame will still use the last next-error
buffer that might be e.g. *compilation* or *grep* buffer buried long ago,
now we need to support the same way to switch next-error navigation as
already is possible in *compilation* and *grep* by typing RET or ‘C-c C-c’
that sets next-error-last-buffer in next-error-internal among other things.

Tried to pass goto-source commands thru next-error-internal that works fine
for change-log-goto-source but has problems in diff-goto-source
because next-error-internal has no arguments but diff-goto-source
expects ‘other-file’ as an arg.

Another solution is to call a special function like next-error-internal
explicitly from all such goto-source commands.  This function could also
set next-error-last-buffer to the current buffer.

Anyway here is the patch that shows the problem with the first solution:

diff --git a/lisp/vc/add-log.el b/lisp/vc/add-log.el
index 175c82f..671f971 100644
--- a/lisp/vc/add-log.el
+++ b/lisp/vc/add-log.el
@@ -471,6 +471,9 @@ change-log-goto-source
 comma-separated list.  If no suitable tag can be found nearby,
 try to visit the file for the change under `point' instead."
   (interactive)
+  (next-error-internal))
+
+(defun change-log-goto-source-internal ()
   (if (and (eq last-command 'change-log-goto-source)
 	   change-log-find-tail)
       (setq change-log-find-tail
@@ -539,7 +542,7 @@ change-log-next-error
   ;; if we found a place to visit...
   (when (looking-at change-log-file-names-re)
     (let (change-log-find-window)
-      (change-log-goto-source)
+      (change-log-goto-source-internal)
       (when change-log-find-window
 	;; Select window displaying source file.
 	(select-window change-log-find-window)))))
diff --git a/lisp/vc/diff-mode.el b/lisp/vc/diff-mode.el
index ef13f55..2a70563 100644
--- a/lisp/vc/diff-mode.el
+++ b/lisp/vc/diff-mode.el
@@ -1349,7 +1349,7 @@ diff-next-error
   (pop-to-buffer (current-buffer))
   (if reset (goto-char (point-min)))
   (diff-hunk-next arg)
-  (diff-goto-source))
+  (diff-goto-source-internal))
 
 (defvar whitespace-style)
 (defvar whitespace-trailing-regexp)
@@ -1874,6 +1874,11 @@ diff-goto-source
   ;; the old location, and else to the new (i.e. as if reverting).
   ;; This is a convenient detail when using smerge-diff.
   (if event (posn-set-point (event-end event)))
+  (if other-file
+      (diff-goto-source-internal other-file)
+    (next-error-internal)))
+
+(defun diff-goto-source-internal (&optional other-file)
   (let ((rev (not (save-excursion (beginning-of-line) (looking-at "[-<]")))))
     (pcase-let ((`(,buf ,line-offset ,pos ,src ,_dst ,switched)
                  (diff-find-source-location other-file rev)))

Re: next-error use cases

[Eli Zaretskii]

> From: Juri Linkov <juri@linkov.net>
> Date: Thu, 12 Apr 2018 23:27:16 +0300
> 
> Since we removed the rule of visibility of next-error-function buffers
> as a criteria for deciding what navigation to use for next-error,
> so e.g. next-error in *Occur* or *Diff* that is the current buffer
> and a single visible buffer on the frame will still use the last next-error
> buffer that might be e.g. *compilation* or *grep* buffer buried long ago,
> now we need to support the same way to switch next-error navigation as
> already is possible in *compilation* and *grep* by typing RET or ‘C-c C-c’
> that sets next-error-last-buffer in next-error-internal among other things.

Sorry, I don't understand what kind of solution are you proposing in
user terms.  Can you please describe what the user should do under the
proposed solution in various cases, such as if they need to invoke
navigation in the current Occur buffer or in a Compilation/Grep
buffer, either shown or not shown?

Re: next-error use cases

[Stephen Leake]

Dmitry Gutov <dgutov@yandex.ru> writes:

> On 4/12/18 6:25 PM, Stephen Leake wrote:
>
>> Presumably you already have "goto error under point"; that should also
>> set the current list.
>>
>> (How do you notice there is an error there, so you know to position point?
>> a marker in the margin?)
>
> The error is not always near point, or even visible in the window. To
> get to it, we need a key binding. So, next-error or previous-error
> working already would be helpful.

The command that created the error list can set compilation-last-buffer;
that's what grep does.


-- 
-- Stephe

Re: next-error use cases

[Dmitry Gutov]

On 4/13/18 7:22 PM, Stephen Leake wrote:

>> The error is not always near point, or even visible in the window. To
>> get to it, we need a key binding. So, next-error or previous-error
>> working already would be helpful.
> 
> The command that created the error list can set compilation-last-buffer;
> that's what grep does.

Of course it can. When will it do that? Every time a buffer is opened or 
saved? That's when Flymake runs.

What if the user want to continue navigation through their Grep results 
instead?

BTW, please take a look at 
https://debbugs.gnu.org/cgi/bugreport.cgi?bug=30674, for some previous 
discussion.

Re: next-error use cases

[Juri Linkov]

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

>> Since we removed the rule of visibility of next-error-function buffers
>> as a criteria for deciding what navigation to use for next-error,
>> so e.g. next-error in *Occur* or *Diff* that is the current buffer
>> and a single visible buffer on the frame will still use the last next-error
>> buffer that might be e.g. *compilation* or *grep* buffer buried long ago,
>> now we need to support the same way to switch next-error navigation as
>> already is possible in *compilation* and *grep* by typing RET or ‘C-c C-c’
>> that sets next-error-last-buffer in next-error-internal among other things.
>
> Sorry, I don't understand what kind of solution are you proposing in
> user terms.  Can you please describe what the user should do under the
> proposed solution in various cases, such as if they need to invoke
> navigation in the current Occur buffer or in a Compilation/Grep
> buffer, either shown or not shown?

I propose to set the global value of next-error-last-buffer after typing
one of the following keybindings in the corresponding buffers:

in *xref* xref-goto-xref: RET
in *Diff* diff-goto-source: RET, C-c C-c, o
in ChangeLogs change-log-goto-source: C-c C-c
in *Occur* occur-mode-goto-occurrence: RET, C-c C-c,
           occur-mode-goto-occurrence-other-window: o
           occur-mode-display-occurrence: C-o

IOW, doing the same what RET and C-c C-c in *compilation* and *grep* already do.

Here is the final tested patch with these changes:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: next-error.2.patch --]
[-- Type: text/x-diff, Size: 12070 bytes --]

diff --git a/lisp/simple.el b/lisp/simple.el
index efe5406..272905e 100644
--- a/lisp/simple.el
+++ b/lisp/simple.el
@@ -122,11 +122,13 @@ next-error-last-buffer
 similar mode is started, or when it is used with \\[next-error]
 or \\[compile-goto-error].")
 
-;; next-error-last-buffer is made buffer-local to keep the reference
+(defvar next-error-buffer nil
+  "The buffer-local value of the most recent `next-error' buffer.")
+;; next-error-buffer is made buffer-local to keep the reference
 ;; to the parent buffer used to navigate to the current buffer, so the
 ;; next call of next-buffer will use the same parent buffer to
 ;; continue navigation from it.
-(make-variable-buffer-local 'next-error-last-buffer)
+(make-variable-buffer-local 'next-error-buffer)
 
 (defvar next-error-function nil
   "Function to use to find the next error in the current buffer.
@@ -184,6 +186,15 @@ next-error-find-buffer-function
   :group 'next-error
   :version "27.1")
 
+(defcustom next-error-found-function #'ignore
+  "Function called when a next locus is found and displayed.
+Function is called with two arguments: a FROM-BUFFER buffer
+from which next-error navigated, and a target buffer TO-BUFFER."
+  :type '(choice (const :tag "No default" ignore)
+                 (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)
@@ -220,6 +231,14 @@ next-error-find-buffer
    (funcall next-error-find-buffer-function avoid-current
                                             extra-test-inclusive
                                             extra-test-exclusive)
+   ;; 1a. If next-error-buffer has no buffer-local value
+   ;; (i.e. never navigated to the current buffer from another),
+   ;; and the current buffer is a `next-error' capable buffer,
+   ;; use it unconditionally, so next-error will always use it.
+   (if (and (not (local-variable-p 'next-error-buffer))
+            (next-error-buffer-p (current-buffer) avoid-current
+			         extra-test-inclusive extra-test-exclusive))
+       (current-buffer))
    ;; 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
@@ -284,37 +303,35 @@ next-error
     (when buffer
       ;; We know here that next-error-function is a valid symbol we can funcall
       (with-current-buffer buffer
-        ;; Allow next-error to be used from the next-error capable buffer.
-        (setq next-error-last-buffer buffer)
         (funcall next-error-function (prefix-numeric-value arg) reset)
-        ;; Override possible change of next-error-last-buffer in next-error-function
-        (setq next-error-last-buffer buffer)
-        (setq-default next-error-last-buffer buffer)
-        (when next-error-recenter
-          (recenter next-error-recenter))
-        (message "%s error from %s"
+        (next-error-found buffer (current-buffer))
+        (message "%s locus from %s"
                  (cond (reset                             "First")
                        ((eq (prefix-numeric-value arg) 0) "Current")
                        ((< (prefix-numeric-value arg) 0)  "Previous")
                        (t                                 "Next"))
-                 next-error-last-buffer)
-        (run-hooks 'next-error-hook)))))
+                 next-error-last-buffer)))))
 
 (defun next-error-internal ()
   "Visit the source code corresponding to the `next-error' message at point."
   (let ((buffer (current-buffer)))
     ;; We know here that next-error-function is a valid symbol we can funcall
-    (with-current-buffer buffer
-      ;; Allow next-error to be used from the next-error capable buffer.
-      (setq next-error-last-buffer buffer)
-      (funcall next-error-function 0 nil)
-      ;; Override possible change of next-error-last-buffer in next-error-function
-      (setq next-error-last-buffer buffer)
-      (setq-default next-error-last-buffer buffer)
-      (when next-error-recenter
-        (recenter next-error-recenter))
-      (message "Current error from %s" next-error-last-buffer)
-      (run-hooks 'next-error-hook))))
+    (funcall next-error-function 0 nil)
+    (next-error-found buffer (current-buffer))
+    (message "Current locus from %s" next-error-last-buffer)))
+
+(defun next-error-found (&optional from-buffer to-buffer)
+  "Function to call when the next locus is found and displayed.
+FROM-BUFFER is a buffer from which next-error navigated,
+and TO-BUFFER is a target buffer."
+  (setq next-error-last-buffer (or from-buffer (current-buffer)))
+  (when to-buffer
+    (with-current-buffer to-buffer
+      (setq next-error-buffer from-buffer)))
+  (when next-error-recenter
+    (recenter next-error-recenter))
+  (funcall next-error-found-function from-buffer to-buffer)
+  (run-hooks 'next-error-hook))
 
 (defun next-error-select-buffer (buffer)
   "Select a `next-error' capable buffer and set it as the last used."
@@ -322,8 +339,7 @@ next-error-select-buffer
    (list (get-buffer
           (read-buffer "Select next-error buffer: " nil nil
                        (lambda (b) (next-error-buffer-p (cdr b)))))))
-  (setq next-error-last-buffer buffer)
-  (setq-default next-error-last-buffer buffer))
+  (setq next-error-last-buffer buffer))
 
 (defalias 'goto-next-locus 'next-error)
 (defalias 'next-match 'next-error)
diff --git a/lisp/progmodes/xref.el b/lisp/progmodes/xref.el
index 5a9a7a9..9a437b6 100644
--- a/lisp/progmodes/xref.el
+++ b/lisp/progmodes/xref.el
@@ -540,9 +540,11 @@ xref-goto-xref
 Non-interactively, non-nil QUIT means to first quit the *xref*
 buffer."
   (interactive)
-  (let ((xref (or (xref--item-at-point)
+  (let ((buffer (current-buffer))
+        (xref (or (xref--item-at-point)
                   (user-error "No reference at point"))))
-    (xref--show-location (xref-item-location xref) (if quit 'quit t))))
+    (xref--show-location (xref-item-location xref) (if quit 'quit t))
+    (next-error-found buffer (current-buffer))))
 
 (defun xref-quit-and-goto-xref ()
   "Quit *xref* buffer, then jump to xref on current line."
diff --git a/lisp/vc/add-log.el b/lisp/vc/add-log.el
index 175c82f..d423706 100644
--- a/lisp/vc/add-log.el
+++ b/lisp/vc/add-log.el
@@ -471,6 +471,11 @@ change-log-goto-source
 comma-separated list.  If no suitable tag can be found nearby,
 try to visit the file for the change under `point' instead."
   (interactive)
+  (let ((buffer (current-buffer)))
+    (change-log-goto-source-internal)
+    (next-error-found buffer (current-buffer))))
+
+(defun change-log-goto-source-internal ()
   (if (and (eq last-command 'change-log-goto-source)
 	   change-log-find-tail)
       (setq change-log-find-tail
@@ -539,7 +545,7 @@ change-log-next-error
   ;; if we found a place to visit...
   (when (looking-at change-log-file-names-re)
     (let (change-log-find-window)
-      (change-log-goto-source)
+      (change-log-goto-source-internal)
       (when change-log-find-window
 	;; Select window displaying source file.
 	(select-window change-log-find-window)))))
@@ -1067,8 +1073,7 @@ change-log-mode
   (set (make-local-variable 'end-of-defun-function)
        'change-log-end-of-defun)
   ;; next-error function glue
-  (setq next-error-function 'change-log-next-error)
-  (setq next-error-last-buffer (current-buffer)))
+  (setq next-error-function 'change-log-next-error))
 
 (defun change-log-next-buffer (&optional buffer wrap)
   "Return the next buffer in the series of ChangeLog file buffers.
diff --git a/lisp/vc/diff-mode.el b/lisp/vc/diff-mode.el
index ef13f55..1e2fbb9 100644
--- a/lisp/vc/diff-mode.el
+++ b/lisp/vc/diff-mode.el
@@ -1874,11 +1874,13 @@ diff-goto-source
   ;; the old location, and else to the new (i.e. as if reverting).
   ;; This is a convenient detail when using smerge-diff.
   (if event (posn-set-point (event-end event)))
-  (let ((rev (not (save-excursion (beginning-of-line) (looking-at "[-<]")))))
+  (let ((buffer (when event (current-buffer)))
+        (rev (not (save-excursion (beginning-of-line) (looking-at "[-<]")))))
     (pcase-let ((`(,buf ,line-offset ,pos ,src ,_dst ,switched)
                  (diff-find-source-location other-file rev)))
       (pop-to-buffer buf)
       (goto-char (+ (car pos) (cdr src)))
+      (when buffer (next-error-found buffer (current-buffer)))
       (diff-hunk-status-msg line-offset (diff-xor rev switched) t))))
 
 
diff --git a/lisp/replace.el b/lisp/replace.el
index 4916cb1..5835c9c 100644
--- a/lisp/replace.el
+++ b/lisp/replace.el
@@ -1192,7 +1194,8 @@ 'occur-mode-mouse-goto
 (defun occur-mode-goto-occurrence (&optional event)
   "Go to the occurrence on the current line."
   (interactive (list last-nonmenu-event))
-  (let ((pos
+  (let ((buffer (when event (current-buffer)))
+        (pos
          (if (null event)
              ;; Actually `event-end' works correctly with a nil argument as
              ;; well, so we could dispense with this test, but let's not
@@ -1204,26 +1207,31 @@ occur-mode-goto-occurrence
                (occur-mode-find-occurrence))))))
     (pop-to-buffer (marker-buffer pos))
     (goto-char pos)
+    (when buffer (next-error-found buffer (current-buffer)))
     (run-hooks 'occur-mode-find-occurrence-hook)))
 
 (defun occur-mode-goto-occurrence-other-window ()
   "Go to the occurrence the current line describes, in another window."
   (interactive)
-  (let ((pos (occur-mode-find-occurrence)))
+  (let ((buffer (current-buffer))
+        (pos (occur-mode-find-occurrence)))
     (switch-to-buffer-other-window (marker-buffer pos))
     (goto-char pos)
+    (next-error-found buffer (current-buffer))
     (run-hooks 'occur-mode-find-occurrence-hook)))
 
 (defun occur-mode-display-occurrence ()
   "Display in another window the occurrence the current line describes."
   (interactive)
-  (let ((pos (occur-mode-find-occurrence))
+  (let ((buffer (current-buffer))
+        (pos (occur-mode-find-occurrence))
 	window)
     (setq window (display-buffer (marker-buffer pos) t))
     ;; This is the way to set point in the proper window.
     (save-selected-window
       (select-window window)
       (goto-char pos)
+      (next-error-found buffer (current-buffer))
       (run-hooks 'occur-mode-find-occurrence-hook))))
 
 (defun occur-find-match (n search message)
@@ -1254,28 +1262,20 @@ occur-next-error
 Compatibility function for \\[next-error] invocations."
   (interactive "p")
   ;; we need to run occur-find-match from within the Occur buffer
-  (with-current-buffer
-      ;; Choose the buffer and make it current.
-      (if (next-error-buffer-p (current-buffer))
-	  (current-buffer)
-	(next-error-find-buffer nil nil
-				(lambda ()
-				  (eq major-mode 'occur-mode))))
-
-    (goto-char (cond (reset (point-min))
-		     ((< argp 0) (line-beginning-position))
-		     ((> argp 0) (line-end-position))
-		     ((point))))
-    (occur-find-match
-     (abs argp)
-     (if (> 0 argp)
-	 #'previous-single-property-change
-       #'next-single-property-change)
-     "No more matches")
-    ;; In case the *Occur* buffer is visible in a nonselected window.
-    (let ((win (get-buffer-window (current-buffer) t)))
-      (if win (set-window-point win (point))))
-    (occur-mode-goto-occurrence)))
+  (goto-char (cond (reset (point-min))
+		   ((< argp 0) (line-beginning-position))
+		   ((> argp 0) (line-end-position))
+		   ((point))))
+  (occur-find-match
+   (abs argp)
+   (if (> 0 argp)
+       #'previous-single-property-change
+     #'next-single-property-change)
+   "No more matches")
+  ;; In case the *Occur* buffer is visible in a nonselected window.
+  (let ((win (get-buffer-window (current-buffer) t)))
+    (if win (set-window-point win (point))))
+  (occur-mode-goto-occurrence))
 \f
 (defface match
   '((((class color) (min-colors 88) (background light))

Re: next-error use cases

[Dmitry Gutov]

Hi Juri,

On 13.04.2018 22:48, Juri Linkov wrote:
> in*xref*  xref-goto-xref: RET
> in*Diff*  diff-goto-source: RET, C-c C-c, o
> in ChangeLogs change-log-goto-source: C-c C-c
> in*Occur*  occur-mode-goto-occurrence: RET, C-c C-c,
>             occur-mode-goto-occurrence-other-window: o
>             occur-mode-display-occurrence: C-o

Only 2 years passed, and I finally read the new code.

I have to say it's a solid improvement over the previous related change.

And one peculiarity: if I'm not mistaken, the value of next-error-buffer 
itself is never used in the current code, there's only the check 
(local-variable-p 'next-error-buffer) that references it, effectively 
turning it into a buffer-local boolean.

But it could probably still be used in custom values of 
next-error-find-buffer-function.

Too bad it didn't solve the flymake+next-error-function problem, but 
that seems more difficult.

Some rough idea: pick some similar keybinding in flymake-supported 
buffers ('C-c C-c'? wouldn't it be too presumptuous to take over?)... 
and make it, I don't know, echo the error text? Or (!) show the 
diagnostics buffer, for which there's no default key binding still. And 
in the end, call next-error-found.

Re: next-error use cases

[Juri Linkov]

> I have to say it's a solid improvement over the previous related change.
>
> And one peculiarity: if I'm not mistaken, the value of next-error-buffer
> itself is never used in the current code, there's only the check 
> (local-variable-p 'next-error-buffer) that references it, effectively
> turning it into a buffer-local boolean.
>
> But it could probably still be used in custom values of
> next-error-find-buffer-function.
>
> Too bad it didn't solve the flymake+next-error-function problem, but that
> seems more difficult.
>
> Some rough idea: pick some similar keybinding in flymake-supported buffers
> ('C-c C-c'? wouldn't it be too presumptuous to take over?)... and make it,

And bind it to next-error-select-buffer.

> I don't know, echo the error text? Or (!) show the diagnostics buffer, for
> which there's no default key binding still. And in the end, call
> next-error-found.

Indeed, using the diagnostics buffer is a good idea (even without showing it).

Re: next-error use cases

[Dmitry Gutov]

On 20.05.2020 01:12, Juri Linkov wrote:

>> Some rough idea: pick some similar keybinding in flymake-supported buffers
>> ('C-c C-c'? wouldn't it be too presumptuous to take over?)... and make it,
> 
> And bind it to next-error-select-buffer.
> 
>> I don't know, echo the error text? Or (!) show the diagnostics buffer, for
>> which there's no default key binding still. And in the end, call
>> next-error-found.
> 
> Indeed, using the diagnostics buffer is a good idea (even without showing it).

We could use the diagnostics in a similar fashion to compilation, xref, 
occur, etc, buffers to detect that the user definitely wants 
next-error-last-buffer changed.

And either set that variable when the user displays the diagnostics 
buffer, or when they use one of the "buttons" inside (but IME it's a 
very rare even, I never use them, unlike Compilation or Grep "buttons").

That would require showing it, however. But otherwise could enable a 
streamlined approach to all next-error-function enabled buffers, where 
the behavior of Flymake based ones doesn't much differ from others.

In all this, I'm most concerned about the keybinding: 'C-c C-c' would 
follow an existing example, but it's seems too valuable to take over in 
all programming major modes.

It calls comment-region in c-mode (and a few others), but that 
duplicates an existing binding.

In prolog-mode, ps-mode, sql-mode it invokes some unique commands, however.

Re: next-error use cases

[Vladimir Sedach]

Dmitry Gutov <dgutov@yandex.ru> writes:
> In all this, I'm most concerned about the keybinding: 'C-c C-c' would
> follow an existing example, but it's seems too valuable to take over
> in all programming major modes.
>
> It calls comment-region in c-mode (and a few others), but that
> duplicates an existing binding.
>
> In prolog-mode, ps-mode, sql-mode it invokes some unique commands, however.

C-c C-c is also already used for the various Lisp language modes. Is
there even a prog-mode that does not use C-c C-c for something?

Why put this under the C-c prefix at all? C-c is for
mode-specific-map. This is not specific to a major mode. There are
next-error bindings already in M-g goto-map; that seems like the
appropriate place, and it is not very busy right now. In the keymap,
the binding itself should also be more mnemonic than C-c.

--
Vladimir Sedach
Software engineering services in Los Angeles https://oneofus.la

Re: next-error use cases

[Stefan Monnier]

> C-c C-c is also already used for the various Lisp language modes. Is
> there even a prog-mode that does not use C-c C-c for something?

IMO, `C-c C-c` should be bound to the command "OK, I'm done for now, please
use it".  That's what it does in things like `message-mode` and `log-edit-mode`.
In programming modes, that would map to something like `compile` or
"send to repl"; and that's indeed what it does in AUCTeX and in tex-mode.el.


        Stefan

Re: next-error use cases

[Dmitry Gutov]

On 21.05.2020 23:33, Vladimir Sedach wrote:
> There are
> next-error bindings already in M-g goto-map; that seems like the
> appropriate place, and it is not very busy right now.

A binding for flymake-show-diagnostics-buffer in goto-map?

The only mnemonic I can suggest is Flycheck's "C-c !". The latter uses 
it for the whole map, though.

The corresponding binding there is "C-c ! l".

Re: next-error use cases

[Juri Linkov]

> A binding for flymake-show-diagnostics-buffer in goto-map?

Good idea, not only for flymake, but for all next-error capable buffers:
a keybinding in goto-map to show the buffer with a list of errors/matches.
For example, the user is navigating using 'M-g M-n' when the next-errors buffer
is not visible, then wants to see all errors/matches, so the user needs to type
some M-g key to show it.

Re: next-error use cases

[Dmitry Gutov]

On 22.05.2020 01:06, Juri Linkov wrote:
>> A binding for flymake-show-diagnostics-buffer in goto-map?
> Good idea, not only for flymake, but for all next-error capable buffers:
> a keybinding in goto-map to show the buffer with a list of errors/matches.
> For example, the user is navigating using 'M-g M-n' when the next-errors buffer
> is not visible, then wants to see all errors/matches, so the user needs to type
> some M-g key to show it.

So we'll have 'M-g l' to pop up the current 'next-error-last-buffer'.

And 'C-c ! l' to pop Flymake's diagnostics buffer specifically (and 
change next-error-last-buffer's value appropriately). 'C-c ! n' and 'C-c 
! p' could work along the same idea.

Re: next-error use cases

[Clément Pit-Claudel]

On 21/05/2020 18.49, Dmitry Gutov wrote:
> On 22.05.2020 01:06, Juri Linkov wrote:
>>> A binding for flymake-show-diagnostics-buffer in goto-map?
>> Good idea, not only for flymake, but for all next-error capable buffers:
>> a keybinding in goto-map to show the buffer with a list of errors/matches.
>> For example, the user is navigating using 'M-g M-n' when the next-errors buffer
>> is not visible, then wants to see all errors/matches, so the user needs to type
>> some M-g key to show it.
> 
> So we'll have 'M-g l' to pop up the current 'next-error-last-buffer'.
> 
> And 'C-c ! l' to pop Flymake's diagnostics buffer specifically (and change next-error-last-buffer's value appropriately). 'C-c ! n' and 'C-c ! p' could work along the same idea.

Does this mean that Flycheck should override M-g l too, or will that work automatically (how will M-g l know which one to pick?)

These are the relevant bits of Flycheck's map, I think:

    (define-key map "c"         #'flycheck-buffer)
    (define-key map "C"         #'flycheck-clear)
    (define-key map "n"         #'flycheck-next-error)
    (define-key map "p"         #'flycheck-previous-error)
    (define-key map "l"         #'flycheck-list-errors)
    (define-key map (kbd "C-w") #'flycheck-copy-errors-as-kill)
    (define-key map "s"         #'flycheck-select-checker)
    (define-key map "h"         #'flycheck-display-error-at-point)
    (define-key map "H"         #'display-local-help)

Re: next-error use cases

[Vladimir Sedach]

Clément Pit-Claudel <cpitclaudel@gmail.com> writes:
> Does this mean that Flycheck should override M-g l too, or will
> that work automatically (how will M-g l know which one to pick?)

next-error-last-buffer, if I understand things correctly.

--
Vladimir Sedach
Software engineering services in Los Angeles https://oneofus.la

Re: next-error use cases

[Dmitry Gutov]

On 22.05.2020 04:31, Clément Pit-Claudel wrote:
> On 21/05/2020 18.49, Dmitry Gutov wrote:
>> On 22.05.2020 01:06, Juri Linkov wrote:
>>>> A binding for flymake-show-diagnostics-buffer in goto-map?
>>> Good idea, not only for flymake, but for all next-error capable buffers:
>>> a keybinding in goto-map to show the buffer with a list of errors/matches.
>>> For example, the user is navigating using 'M-g M-n' when the next-errors buffer
>>> is not visible, then wants to see all errors/matches, so the user needs to type
>>> some M-g key to show it.
>>
>> So we'll have 'M-g l' to pop up the current 'next-error-last-buffer'.
>>
>> And 'C-c ! l' to pop Flymake's diagnostics buffer specifically (and change next-error-last-buffer's value appropriately). 'C-c ! n' and 'C-c ! p' could work along the same idea.
> 
> Does this mean that Flycheck should override M-g l too, or will that work automatically (how will M-g l know which one to pick?)

I think this will work simply like:

(defun next-error-show-list ()
   (interactive)
   (pop-to-buffer next-error-last-buffer))

But for this to be okay, we'd have to decide on a principle, and 
Flycheck would have to agree, that next-error-function is set in 
flycheck-error-list-mode, and not in flycheck-mode.

> These are the relevant bits of Flycheck's map, I think:
> 
>      (define-key map "c"         #'flycheck-buffer)
>      (define-key map "C"         #'flycheck-clear)
>      (define-key map "n"         #'flycheck-next-error)
>      (define-key map "p"         #'flycheck-previous-error)
>      (define-key map "l"         #'flycheck-list-errors)
>      (define-key map (kbd "C-w") #'flycheck-copy-errors-as-kill)
>      (define-key map "s"         #'flycheck-select-checker)
>      (define-key map "h"         #'flycheck-display-error-at-point)
>      (define-key map "H"         #'display-local-help)

The copy-errors-as-kill command sounds nice. Not sure if any others can 
easily apply to Flymake too.

Re: next-error use cases

[Clément Pit-Claudel]

On 22/05/2020 15.10, Vladimir Sedach wrote:
> 
> Clément Pit-Claudel <cpitclaudel@gmail.com> writes:
>> Does this mean that Flycheck should override M-g l too, or will
>> that work automatically (how will M-g l know which one to pick?)
> 
> next-error-last-buffer, if I understand things correctly.

That doesn't sound right: next-error-last-buffer should be the source buffer, not the error list, shouldn't it?

Re: next-error use cases

[Clément Pit-Claudel]

On 22/05/2020 19.50, Dmitry Gutov wrote:
> On 22.05.2020 04:31, Clément Pit-Claudel wrote:
>> On 21/05/2020 18.49, Dmitry Gutov wrote:
>>> On 22.05.2020 01:06, Juri Linkov wrote:
>>>>> A binding for flymake-show-diagnostics-buffer in goto-map?
>>>> Good idea, not only for flymake, but for all next-error capable buffers:
>>>> a keybinding in goto-map to show the buffer with a list of errors/matches.
>>>> For example, the user is navigating using 'M-g M-n' when the next-errors buffer
>>>> is not visible, then wants to see all errors/matches, so the user needs to type
>>>> some M-g key to show it.
>>>
>>> So we'll have 'M-g l' to pop up the current 'next-error-last-buffer'.
>>>
>>> And 'C-c ! l' to pop Flymake's diagnostics buffer specifically (and change next-error-last-buffer's value appropriately). 'C-c ! n' and 'C-c ! p' could work along the same idea.
>>
>> Does this mean that Flycheck should override M-g l too, or will that work automatically (how will M-g l know which one to pick?)
> 
> I think this will work simply like:
> 
> (defun next-error-show-list ()
>   (interactive)
>   (pop-to-buffer next-error-last-buffer))
> 
> But for this to be okay, we'd have to decide on a principle, and Flycheck would have to agree, that next-error-function is set in flycheck-error-list-mode, and not in flycheck-mode.

Ah, how surprising.  Currently we set next-error-function in the main buffer, not in the error list, in part because the error list is built lazily (we don't construct the error list buffer until you've requested it, so next-error-last-buffer would be nil most of the time)

Re: next-error use cases

[Dmitry Gutov]

On 23.05.2020 04:22, Clément Pit-Claudel wrote:
> On 22/05/2020 15.10, Vladimir Sedach wrote:
>>
>> Clément Pit-Claudel <cpitclaudel@gmail.com> writes:
>>> Does this mean that Flycheck should override M-g l too, or will
>>> that work automatically (how will M-g l know which one to pick?)
>>
>> next-error-last-buffer, if I understand things correctly.
> 
> That doesn't sound right: next-error-last-buffer should be the source buffer, not the error list, shouldn't it?

Not if we try to streamline the next-error navigation and try to make it 
work (almost) the same way across different next-error sources.

Re: next-error use cases

[Dmitry Gutov]

On 23.05.2020 04:25, Clément Pit-Claudel wrote:
> Ah, how surprising.  Currently we set next-error-function in the main buffer, not in the error list, in part because the error list is built lazily (we don't construct the error list buffer until you've requested it, so next-error-last-buffer would be nil most of the time)

Yeah, so then next-error navigation won't work before the user calls 
flycheck-error-list explicitly.

I agree that might be suboptimal, so maybe some more advanced design is 
required.

RE: next-error use cases

[Drew Adams]

> Hopefully the following isn't too silly.  Right now we use C-x ` for
> two fairly different purposes:
> 
> 1. We have a list of locations (typically lines and columns), and we
> want jump from one to the next, opening files and buffers as needed
> (that's the grep, occur, and compilation-mode cases).
> 2. We have a buffer with interesting locations, and we want to cycle
> through them (that's the flyspell, flymake, flycheck cases).
> 
> This sounds quite similar to what happens with the global and the 
> local mark rings, actually.  To cycle through marks in the current 
> buffer you use C-u C-SPC; to cycle through global marks you use
> C-x C-SPC.  So I wonder if it would make sense to have C-x ` cycle 
> through locations that come from "location lists" (grep) and
> having, say, C-c ` cycle through locations that come from the
> current buffer (flyspell).
> 
> That doesn't fully solve the problem of picking which location
> list to cycle through, but it eliminates the confusion between
> local and global locations (as a logical consequence, I imagine
> C-c ` in a location list would most of the time just move to the
> next line, though in an occur or grep buffer with multiple lines
> of context it would jump to the next match)

Sorry to barge in here, especially having not read
the thread till here.

Just thought I'd mention what I do in Bookmark+ and
zones.el, in this regard.  There too you can cycle
among things: bookmarks, buffer zones (regions/areas).

In each case, you can have a list of same-buffer
thingies to cycle among or a list of multiple-buffer
thingies to cycle among.

In each case, there are default lists of same-buffer
and multiple-buffer thingies, and you can define
other, named lists.

In each case, it's easy to switch among such lists.
For navigation purposes (such lists/sets can be used
for other purposes too), call them "navigation lists".

So in each case there are various ways to choose a
navigation list, to make it current.

In the case of Bookmark+:

* You can pick a set of bookmarks that are all of a
  given type (e.g. Info, Dired, EWW)

* You can pick them by picking a bookmark-list bookmark.

* You can pick them by choosing the current bookmark
  list (as displayed: sorted, respecting omissions etc.).

* You can use the currently defined global bookmark
  list (ignoring what's in the displayed bookmark list).

* There are separate keys for cycling among bookmarks
  for the current file/buffer and for cycling among
  another chosen navigation list.  IOW, you can do the
  former without unchoosing the current navigation list.

In the case of zones.el, you choose a zone-set variable.
Such a variable can be buffer-local, so its zones are
only for the current buffer, or it can be global, i.e.,
across buffers.

So the ways of picking navigation lists are different
for bookmarks and zones, as they should be.  As I said,
such sets serve other purposes, besides just navigation,
and those purposes are very different for zones and
bookmarks.

Likewise, the keys for cycling are different for zones
and bookmarks.  The keys for each fit in with the other
keys (other zone keys, other bookmark keys).

Dunno whether this info helps at all, either as food
for thought (e.g., generalizing the view) or as an
argument that it does not necessarily make sense to
search for a single way to use such things (e.g. single
set of cycling keys, or even similar cycling keys).

Note: "not necessarily".  Other things being equal, of
course it helps users if the same or similar keys do
the same or similar things in different contexts.  But
other things might well not be equal (they are not
equal in the case of zones and bookmarks).  Different
kinds of thing-sets offer different possibilities.

Bookmark+ description of this:

https://www.emacswiki.org/emacs/BookmarkPlus#CyclingNavlist

zones.el description:

https://www.emacswiki.org/emacs/Zones

Re: next-error use cases

[Dmitry Gutov]

On 23.05.2020 05:26, Clément Pit-Claudel wrote:

>> Oh, its beginning was only a tiny bit earlier:
>>
>> https://lists.gnu.org/archive/html/emacs-devel/2018-04/msg00207.html
> 
> Thanks a lot.  I think I understand the issue better.
> 
> Hopefully the following isn't too silly.  Right now we use C-x ` for two fairly different purposes:
> 
> 1. We have a list of locations (typically lines and columns), and we want jump from one to the next, opening files and buffers as needed (that's the grep, occur, and compilation-mode cases).
> 2. We have a buffer with interesting locations, and we want to cycle through them (that's the flyspell, flymake, flycheck cases).
> 
> This sounds quite similar to what happens with the global and the local mark rings, actually.  To cycle through marks in the current buffer you use C-u C-SPC; to cycle through global marks you use C-x C-SPC.  So I wonder if it would make sense to have C-x ` cycle through locations that come from "location lists" (grep) and having, say, C-c ` cycle through locations that come from the current buffer (flyspell).
> 
> That doesn't fully solve the problem of picking which location list to cycle through, but it eliminates the confusion between local and global locations (as a logical consequence, I imagine C-c ` in a location list would most of the time just move to the next line, though in an occur or grep buffer with multiple lines of context it would jump to the next match)
> 
> Does this make sense?

I'm not a big fan of mark rings, personally. Never use them, and if they 
were good enough, would we still have the xref marker ring? Formerly 
known as find-tag marker ring.

For error navigation, it /could/ be a better idea, but IMHO it's like 
we'd be giving up and creating a parallel set of variables and commands 
for "local" errors. And necessitate a "muscle memory" context switch 
when one goes from "local" errors to "global" or back. One set of 
bindings would also have to be more awkward than the other.

How to evaluate this option objectively, though? Maybe we should ask 
what kind of user behavior we expect most of the time.

If, when working with local errors, we expect the user to go between 
them, then use the "global" navigation commands to jump between 
locations from e.g. Compilation, then back to "local" errors, in a short 
period of time, then having two sets of commands might be optimal.

If, on the other hand, we rather expect the user not to interleave the 
uses of "local" and "global" location lists as often, that the switches 
between sources of errors would be mostly performed automatically (e.g. 
when one calls 'M-x rgrep'), and that the necessary manual invocations 
of next-error-select-buffer would be infrequent enough. Or that the user 
would be switching between "global" sources of errors at least as often 
as between "local" and "global" ones, we should probably optimize for 
having just one set of commands.

There's also another wrinkle: I think there was a proposed feature for 
Flycheck to list errors for multiple files (or the whole project) 
together? 'next-error' could be handy for jumping between those too.

Re: next-error use cases

[Clément Pit-Claudel]

On 23/05/2020 21.36, Dmitry Gutov wrote:

> For error navigation, it /could/ be a better idea, but IMHO it's like we'd be giving up and creating a parallel set of variables and commands for "local" errors. And necessitate a "muscle memory" context switch when one goes from "local" errors to "global" or back. One set of bindings would also have to be more awkward than the other.

Good points all around, and I don't have much more to contribute :)

> There's also another wrinkle: I think there was a proposed feature for Flycheck to list errors for multiple files (or the whole project) together? 'next-error' could be handy for jumping between those too.
We do this currently already, and the idea is that all external errors are converted into local errors on line 1.
In the error list they appear with a file name and a line number in that file, so if you were to cycle errors relative to that list you would visit files one by one, and if you were to visit errors relative to the buffer you'd go through all local errors then jump to another file with errors, if any.

Re: next-error use cases

[Dmitry Gutov]

On 24.05.2020 04:41, Clément Pit-Claudel wrote:
> On 23/05/2020 21.36, Dmitry Gutov wrote:
> 
>> For error navigation, it /could/ be a better idea, but IMHO it's like we'd be giving up and creating a parallel set of variables and commands for "local" errors. And necessitate a "muscle memory" context switch when one goes from "local" errors to "global" or back. One set of bindings would also have to be more awkward than the other.
> 
> Good points all around, and I don't have much more to contribute :)

Perhaps you have thoughts on which particular usage pattern is more 
prevalent?

I should add that we're currently defaulting to approach #1. Even if 
Flymake doesn't have bindings by default, we will add some, and they'll 
mirror Flycheck's ones by default.

As a result, we'll already have two parallel sets of bindings for 
"local" and other errors: next/previous-error and 
flymake-next/previous-error.

So the question is, is there a reasonable scheme to use 
next-error/previous-error for Flymake's errors without severe 
side-effects, or we should stay with the current situation.

Whereby I'll bind M-n and M-p to Flymake's commands locally and leave 
next-error/previous-error on M-g M-n/p.

>> There's also another wrinkle: I think there was a proposed feature for Flycheck to list errors for multiple files (or the whole project) together? 'next-error' could be handy for jumping between those too.
> We do this currently already, and the idea is that all external errors are converted into local errors on line 1.
> In the error list they appear with a file name and a line number in that file, so if you were to cycle errors relative to that list you would visit files one by one, and if you were to visit errors relative to the buffer you'd go through all local errors then jump to another file with errors, if any.

In that case, maybe one could say that Flycheck errors are not always 
"local" either?