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

[Dmitry Gutov]

On 10/30/17 11:14 PM, Juri Linkov wrote:
> > > 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.
> >
> > Doesn't sound very elegant: we accumulate hidden state as the time passes,
> > and the user calls 'next-error' with compilations, grep, and so on. But
> > this is better than nothing, of course.
> >
> > Does it also mean that the effect of next-error-select-buffer will also be
> > buffer-local?
>
> Yes, next-error-select-buffer sets both buffer-local and global values,
> exactly like in next-error.

Then the aforementioned concern stands for it as well.

> We could also add an alternative function based on window-local values.
> At least, when I tried this code, it works as expected:

Cool. Not something I've ever asked for, but could be helpful for some 
people.

In general, next-error can jump between windows, so window-local is not 
a good fit for my mental model.

> > So we ignore the next-error-last-buffer change during a call to
> > next-error-last-function, but not in any other circumstances? Like visiting
> > a ChangeLog file manually. Maybe that's okay...
>
> Oh, sorry, there was a typo: it should be (with-current-buffer next-error-buffer
> Fixed in the next version of the patch below.

Thanks. But if we already ignore the change to next-error-last-buffer 
during a call to next-error-function, that should fix the currently 
discussed bug, right? Maybe even the similar behavior with next-error, too?

Do we need the buffer-local-ity changes to next-error-last-buffer for 
that? Because if we do, that's okay, let's commit it and everything, but 
otherwise I'd rather wait and look for a more elegant approach (for 
other issues aside from this one).

> +        (message "Showing %s error from %s"
> +                 (cond (reset                            "first")
> +                       ((< (prefix-numeric-value arg) 0) "previous")
> +                       (t                                "next"))

Can arg be 0? Do we have a word for that? We might use it below, too.

> +(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 "Showing another error from %s" next-error-last-buffer)

Is it really "another"? Or maybe it's "current", "last" or "closest" error?