xref overrides user xref-backend-functions

xref overrides user xref-backend-functions

[Stephen Leake]

This commit:

   commit a5fd38c3a6f263185ce5838010e7a0d64b49bec2
   Committer : Dmitry Gutov <dgutov@yandex.ru>
   CommitDate: Sat Nov 21 01:57:15 2015 +0200

      Add xref--etags-backend to xref-backing-functions using add-hook

      * lisp/progmodes/xref.el (xref-backend-functions): Move the
      default value into a separate `add-hook' call (bug#21964).

overrides setting xref-backend-functions in ~/.emacs-init. For example,
from emacs 25 -Q, evaluate:

    (progn
     (setq xref-backend-functions (lambda () 'my-xref-backend))
     (require 'xref)
     xref-backend-functions)

This gives:

(etags--xref-backend (lambda nil (quote my-xref-backend)))

which is not what I want.

This needs a comment on why etags--xref-backend is not set in the
default value of the variable; it's not clear to me based on the bug
report, and the bug report is not referenced from the code.


I think a good fix is to not add the hook if already set:

(unless xref-backend-functions
 (add-hook 'xref-backend-functions #'etags--xref-backend))

This works for my use case, and for bug#21964.

An alternate work-around is to require xref before setting
xref-backend-functions.

--
-- Stephe

Re: xref overrides user xref-backend-functions

[Dmitry Gutov]

On 12/01/2015 08:15 AM, Stephen Leake wrote:

>      (progn
>       (setq xref-backend-functions (lambda () 'my-xref-backend))
>       (require 'xref)
>       xref-backend-functions)

I don't think we've ever discussed being able to override the backend 
from the init file like that.

> This gives:
>
> (etags--xref-backend (lambda nil (quote my-xref-backend)))
>
> which is not what I want.

There's nothing surprising about it, though.

> This needs a comment on why etags--xref-backend is not set in the
> default value of the variable; it's not clear to me based on the bug
> report, and the bug report is not referenced from the code.

Either we do it like this, or we ask every piece of code that wants to 
(add-hook 'xref-backend-functions ...), to (require 'xref) first. And if 
it doesn't do that, the default value of xref-backend-functions is lost.

Almost none hook variables have default values, but many add to it after 
the definition using `add-hook'. So I did the same.

> I think a good fix is to not add the hook if already set:
>
> (unless xref-backend-functions
>   (add-hook 'xref-backend-functions #'etags--xref-backend))
>
> This works for my use case, and for bug#21964.

This will make things worse when someone else writes (add-hook 
'xref-backend-functions 'bar-backend) with the intention of seeing the 
element added to the beginning of the list, but still fall back to etags 
when their function fails.

> An alternate work-around is to require xref before setting
> xref-backend-functions.

That's one valid approach.

How about this, though?

diff --git a/lisp/progmodes/xref.el b/lisp/progmodes/xref.el
index 8a69b07..cddb017 100644
--- a/lisp/progmodes/xref.el
+++ b/lisp/progmodes/xref.el
@@ -210,7 +210,7 @@ xref-backend-functions

  ;; We make the etags backend the default for now, until something
  ;; better comes along.
-(add-hook 'xref-backend-functions #'etags--xref-backend)
+(add-hook 'xref-backend-functions #'etags--xref-backend t)

  ;;;###autoload
  (defun xref-find-backend ()

Re: xref overrides user xref-backend-functions

[Stephen Leake]

Dmitry Gutov <dgutov@yandex.ru> writes:

> On 12/01/2015 08:15 AM, Stephen Leake wrote:
>
>>      (progn
>>       (setq xref-backend-functions (lambda () 'my-xref-backend))
>>       (require 'xref)
>>       xref-backend-functions)
>
> I don't think we've ever discussed being able to override the backend
> from the init file like that.

Yes, we did settle on a globalized minor mode. That's what I'm
actually doing.

Ah; the code that defines the minor mode does not have "(require
'xref)"; it should. Then this issue is moot.

> How about this, though?
>
> diff --git a/lisp/progmodes/xref.el b/lisp/progmodes/xref.el
> index 8a69b07..cddb017 100644
> --- a/lisp/progmodes/xref.el
> +++ b/lisp/progmodes/xref.el
> @@ -210,7 +210,7 @@ xref-backend-functions
>
>  ;; We make the etags backend the default for now, until something
>  ;; better comes along.
> -(add-hook 'xref-backend-functions #'etags--xref-backend)
> +(add-hook 'xref-backend-functions #'etags--xref-backend t)

So etags--xref-backend is appended rather than prepended. That's a
little better, but I'd prefer to have complete control. I think (require
'xref) is acceptable.

-- 
-- Stephe

Re: xref overrides user xref-backend-functions

[Dmitry Gutov]

On 12/02/2015 01:41 AM, Stephen Leake wrote:

> So etags--xref-backend is appended rather than prepended. That's a
> little better, but I'd prefer to have complete control. I think (require
> 'xref) is acceptable.

All right. But I'll take "a little better" too, so we have a slightly 
more natural result if the add-hook caller forgets to (require 'xref).

Re: xref overrides user xref-backend-functions

[Dmitry Gutov]

Hi Anders,

On 12/02/2015 08:08 AM, Anders Lindgren wrote:
	
> Have you considered using the "###autoload" construct? This way, the
> default value is available to all users when Emacs start and they don't
> have to remember to require xref before changing the variable. At the
> same time, if a user sets the variable, xref will not modify it when loaded.

That should fit the bill too, indeed.

I've just read the advice against autoloading variables too many times 
to consider it. But searching through the codebase now, we do have quite 
a few such variables.