emacs-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Emacs-diffs] master d7df36e: Rewrite elisp--xref-find-definitions t


From: Dmitry Gutov
Subject: Re: [Emacs-diffs] master d7df36e: Rewrite elisp--xref-find-definitions to handle many more cases; add tests.
Date: Tue, 11 Aug 2015 19:13:36 +0300
User-agent: Mozilla/5.0 (X11; Linux x86_64; rv:40.0) Gecko/20100101 Thunderbird/40.0

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.



reply via email to

[Prev in Thread] Current Thread [Next in Thread]