Re: [Emacs-diffs] master d7df36e: Rewrite elisp--xref-find-definitions to handle many more cases; add tests.

[Dmitry Gutov]

Thank you, this is a considerable improvement. See comments below.

On 08/11/2015 05:56 AM, Stephen Leake wrote:
> branch: master
> commit d7df36e745a5ba480559b6c8b5ebc93a18fe9bd1
> Author: Stephen Leake <address@hidden>
> Commit: Stephen Leake <address@hidden>
>
>      Always output all available definitions.

How come you saw fit to undo the tweaks that I've added over time?

In particular, now jumping to whitespace-mode will show two entries, 
even though they both lead to one location. That's a waste of time.

> -(defvar elisp--xref-format
> +(defconst elisp--xref-format
>     (let ((str "(%s %s)"))
>       (put-text-property 1 3 'face 'font-lock-keyword-face str)
>       (put-text-property 4 6 'face 'font-lock-function-name-face str)
>       str))

I'm not sure which part of the change did that, but now I don't see the 
colors in the output.

Even though the approach is questionable, the result should be worth 
keeping.

> +(defconst elisp--xref-format-cl-defmethod
> +  (let ((str "(%s %s %s)"))
> +    (put-text-property 1 3 'face 'font-lock-keyword-face str)
> +    (put-text-property 4 6 'face 'font-lock-function-name-face str)
> +    str))
> +
> +(defcustom find-feature-regexp

I think these should still go into find-func.el. Even if you can't 
require it at the top of elisp-mode.el, you can do that at the beginning 
of elisp--xref-find-definitions. If that's actually needed.

> +          ((setq generic (cl--generic symbol))
> +           (dolist (method (cl--generic-method-table generic))
> +             (let* ((info (cl--generic-method-info method))
> +                    (met-name (cons symbol (cl--generic-method-specializers 
> method)))
> +                    (descr (format elisp--xref-format-cl-defmethod 
> 'cl-defmethod symbol (nth 1 info)))
> +                    (file (find-lisp-object-file-name met-name 'cl-defmethod)))

This should also account, if possible, for the default method 
definitions performed by cl-defgeneric (and hide them).

For instance, looking for project-search-path will display three 
entries, but the last two both lead to its generic definition. This is 
actually inaccurate as well, since both the first and the second item 
come from cl-defgeneric.

> +;; When tests are run from the Makefile, 'default-directory' is $HOME,
> +;; so we must provide this dir to expand-file-name in the expected
> +;; results. The Makefile sets EMACS_TEST_DIRECTORY.
> +(defconst emacs-test-dir (getenv "EMACS_TEST_DIRECTORY"))

You could also use (or load-file-name (buffer-file-name)). That has the 
benefit of being usable when tests are run inside the interactive 
session. Not via Makefile.

> +;; Source for both variable and defun is "(define-minor-mode
> +;; compilation-minor-mode". There is no way to tell that from the
> +;; symbol.

(and (fboundp sym)
     (memq sym minor-mode-list))

seemed to be a decent indicator.