all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
From: joaotavora@gmail.com (João Távora)
To: Dmitry Gutov <dgutov@yandex.ru>
Cc: Vitalie Spinu <spinuvit@gmail.com>,
	Stefan Monnier <monnier@IRO.UMontreal.CA>,
	emacs-devel@gnu.org
Subject: xref backends for elisp-related modes Was: Re: Bad moves with xref-find-definitions
Date: Sun, 26 Apr 2015 12:51:00 +0100	[thread overview]
Message-ID: <m1vbgjw1jv.fsf_-_@holy.lan> (raw)
In-Reply-To: <553C285B.4070400@yandex.ru> (Dmitry Gutov's message of "Sun, 26 Apr 2015 02:50:51 +0300")

Dmitry Gutov <dgutov@yandex.ru> writes:

> On 04/25/2015 09:56 PM, joaotavora@gmail.com (João Távora) wrote:
>
>> While we're on the subject, is a patch welcome to set
>> `xref-find-function' to `elisp-xref-find' more pervasively in the
>> apropos, debugger and help buffers?
>
> I'm fine with that, but the effect will also make it harder for Eli to
> undo that change. :) Dunno how important that is.
>
> See http://debbugs.gnu.org/19466.

I've read/skimmed it. Indeed, it's not acceptable to force users to
customize their xref-finding backend in emacs-lisp-mode, its derived
modes (lisp-interaction-mode), or its related modes (elisp apropos,
help, debugger), if, for some reason, they believe etags is a better
backend for them in certain contexts.

BTW the necessary level of indirection is already present with
`advice-add': we needn't necessarily add more variables. I mean:

   (advice-add 'elisp-xref-find :override #'etags-xref-find)

Should be enough and quite readable. Conversely, I wouldn't be very
annoyed if etags were the default method of finding elisp xrefs. Then I
would use advice-add to set my behaviour, perhaps by doing

   (advice-add 'elisp-xref-find :override
   #'elisp-compiled-identifier-locations) ; better name pending

Additionally, if `add-function' allowed `:append' for its WHERE arg,
like Common Lisp's method combinations, it would be even cleaner to get
the behaviour desired by Eli (and Vitalie, I think), of multiple
backends.

   (advice-add 'elisp-xref-find :append #'etags-xref-find)

>> The patch attached does this to a certain degree, but takes some care to
>> not do this is the major mode's definitions, since in theory these modes
>> could be used for something other than emacs-lisp.
>
> Makes sense.

It does, I know, but it would be a lot simpler, especially for functions
spawning *Help* buffers, to just assume we're in an elisp-related
context. Packages like SLIME or SLY or CIDER that make use of these
major modes should be the ones overriding the backend.

>> +(advice-add 'describe-mode :after #'help--setup-xref-find-function)
>> +(advice-add 'describe-function :after #'help--setup-xref-find-function)
>> +(advice-add 'describe-variable :after #'help--setup-xref-find-function)
>> +(advice-add 'describe-bindings :after #'help--setup-xref-find-function)
>
> Why not add the call to the command definitions directly?

Because there are a lot more "describe-*" functions, and ideally, one
would want to advise them on a predictable pattern, not clutter each
`describe-*''s sometimes intricate design with pin-pointed calls to a
setup function.

Anyway, this problem goes away if we take the simpler approach of just
setting it in the major-mode function. It makes the resulting patch a
lot simpler.

João

PS: here's my latest version

diff --git a/lisp/apropos.el b/lisp/apropos.el
index 023ba4b..492d9d7 100644
--- a/lisp/apropos.el
+++ b/lisp/apropos.el
@@ -475,7 +475,8 @@ This requires at least two keywords (unless only one was given)."
 (define-derived-mode apropos-mode special-mode "Apropos"
   "Major mode for following hyperlinks in output of apropos commands.
 
-\\{apropos-mode-map}")
+\\{apropos-mode-map}"
+  (elisp--setup-xref-backend))
 
 (defvar apropos-multi-type t
   "If non-nil, this apropos query concerns multiple types.
diff --git a/lisp/emacs-lisp/debug.el b/lisp/emacs-lisp/debug.el
index ce5c786..e4c59f0 100644
--- a/lisp/emacs-lisp/debug.el
+++ b/lisp/emacs-lisp/debug.el
@@ -708,6 +708,7 @@ Complete list of commands:
 \\{debugger-mode-map}"
   (setq truncate-lines t)
   (set-syntax-table emacs-lisp-mode-syntax-table)
+  (elisp--setup-xref-backend)
   (use-local-map debugger-mode-map))
 \f
 (defcustom debugger-record-buffer "*Debugger-record*"
diff --git a/lisp/help-mode.el b/lisp/help-mode.el
index d6679e9..6fc63ea 100644
--- a/lisp/help-mode.el
+++ b/lisp/help-mode.el
@@ -288,7 +288,8 @@ Commands:
   (set (make-local-variable 'revert-buffer-function)
        'help-mode-revert-buffer)
   (set (make-local-variable 'bookmark-make-record-function)
-       'help-bookmark-make-record))
+       'help-bookmark-make-record)
+  (elisp--setup-xref-backend))
 
 ;;;###autoload
 (defun help-mode-setup ()
diff --git a/lisp/progmodes/elisp-mode.el b/lisp/progmodes/elisp-mode.el
index ad35c48..98a3427 100644
--- a/lisp/progmodes/elisp-mode.el
+++ b/lisp/progmodes/elisp-mode.el
@@ -218,6 +218,8 @@ Comments in the form will be lost."
   :type 'hook
   :group 'lisp)
 
+(declare-function 'elisp--setup-xref-backend "elisp-mode")
+
 ;;;###autoload
 (define-derived-mode emacs-lisp-mode prog-mode "Emacs-Lisp"
   "Major mode for editing Lisp code to run in Emacs.
@@ -227,8 +229,6 @@ Blank lines separate paragraphs.  Semicolons start comments.
 
 \\{emacs-lisp-mode-map}"
   :group 'lisp
-  (defvar xref-find-function)
-  (defvar xref-identifier-completion-table-function)
   (lisp-mode-variables nil nil 'elisp)
   (add-hook 'after-load-functions #'elisp--font-lock-flush-elisp-buffers)
   (setq-local electric-pair-text-pairs
@@ -236,9 +236,7 @@ Blank lines separate paragraphs.  Semicolons start comments.
   (setq imenu-case-fold-search nil)
   (add-function :before-until (local 'eldoc-documentation-function)
                 #'elisp-eldoc-documentation-function)
-  (setq-local xref-find-function #'elisp-xref-find)
-  (setq-local xref-identifier-completion-table-function
-              #'elisp--xref-identifier-completion-table)
+  (elisp--setup-xref-backend)
   (add-hook 'completion-at-point-functions
             #'elisp-completion-at-point nil 'local))
 
@@ -581,6 +579,11 @@ It can be quoted, or be inside a quoted form."
 (declare-function xref-make-bogus-location "xref" (message))
 (declare-function xref-make "xref" (description location))
 
+(defun elisp--setup-xref-backend ()
+  (setq-local xref-find-function #'elisp-xref-find)
+  (setq-local xref-identifier-completion-table-function
+              #'elisp--xref-identifier-completion-table))
+
 (defun elisp-xref-find (action id)
   (require 'find-func)
   (pcase action



  reply	other threads:[~2015-04-26 11:51 UTC|newest]

Thread overview: 63+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-04-23 15:07 Bad moves with xref-find-definitions Vitalie Spinu
2015-04-25 14:24 ` Stefan Monnier
2015-04-25 16:25   ` Dmitry Gutov
2015-04-25 17:42   ` Vitalie Spinu
2015-04-25 18:49     ` Vitalie Spinu
2015-04-25 19:07       ` Dmitry Gutov
2015-04-25 18:56     ` João Távora
2015-04-25 23:50       ` Dmitry Gutov
2015-04-26 11:51         ` João Távora [this message]
2015-04-26 13:56           ` xref backends for elisp-related modes Was: " Dmitry Gutov
2015-04-26 14:51           ` Eli Zaretskii
2015-04-28 11:06             ` Vitalie Spinu
2015-04-28 11:41               ` João Távora
2015-04-28 11:59                 ` Vitalie Spinu
2015-04-28 15:17                   ` Eli Zaretskii
2015-04-28 15:45                     ` Vitalie Spinu
2015-04-28 16:01                       ` Eli Zaretskii
2015-04-28 13:27                 ` Stefan Monnier
2015-04-28 21:28                   ` Dmitry Gutov
2015-04-29 12:35                     ` Vitalie Spinu
2015-04-29 15:45                     ` Eli Zaretskii
2015-04-28 15:15               ` Eli Zaretskii
2015-04-28 15:47                 ` Vitalie Spinu
2015-04-28 16:03                   ` Eli Zaretskii
2015-04-29  6:55               ` Helmut Eller
2015-04-29 12:40                 ` Vitalie Spinu
2015-04-29 13:01                   ` Helmut Eller
2015-04-29 15:30                     ` Vitalie Spinu
2015-04-29 17:09                       ` Dmitry Gutov
2015-04-29 12:47                 ` Dmitry Gutov
2015-04-29 13:04                   ` Helmut Eller
2015-04-29 13:12                     ` Dmitry Gutov
2015-04-27  2:20           ` Stefan Monnier
2015-04-25 19:11     ` Dmitry Gutov
2015-04-25 20:43       ` Vitalie Spinu
2015-04-26  3:37         ` Dmitry Gutov
2015-04-26  6:38           ` Bozhidar Batsov
2015-04-26 18:41             ` Dmitry Gutov
2015-04-27 19:36               ` Bozhidar Batsov
2015-04-28  1:23                 ` Dmitry Gutov
2015-04-28 11:30               ` Vitalie Spinu
2015-04-26 10:44           ` Vitalie Spinu
2015-04-26 13:14             ` Vitalie Spinu
2015-04-26 15:09             ` Dmitry Gutov
2015-04-26 16:23               ` Vitalie Spinu
2015-04-26 17:51                 ` Dmitry Gutov
2015-04-26 20:56                   ` Vitalie Spinu
2015-04-27 21:57                     ` Dmitry Gutov
2015-04-26  3:34     ` Stefan Monnier
2015-04-26 11:31       ` Vitalie Spinu
2015-04-26 14:50         ` Eli Zaretskii
2015-04-26 15:12           ` Dmitry Gutov
2015-04-26 15:32             ` Eli Zaretskii
2015-04-26 15:20         ` Dmitry Gutov
2015-04-26 16:01           ` Vitalie Spinu
2015-04-26 17:26             ` Dmitry Gutov
2015-04-27  2:26         ` Stefan Monnier
2015-04-25 19:01 ` Dmitry Gutov
2015-04-25 20:34   ` Vitalie Spinu
2015-04-25 21:44     ` Vitalie Spinu
2015-04-26  0:20     ` Dmitry Gutov
2015-04-26  0:28       ` Dmitry Gutov
2015-04-28 15:31       ` Vitalie Spinu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=m1vbgjw1jv.fsf_-_@holy.lan \
    --to=joaotavora@gmail.com \
    --cc=dgutov@yandex.ru \
    --cc=emacs-devel@gnu.org \
    --cc=monnier@IRO.UMontreal.CA \
    --cc=spinuvit@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.