From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Stephen Leake Newsgroups: gmane.emacs.devel Subject: Re: [Emacs-diffs] master d7df36e: Rewrite elisp--xref-find-definitions to handle many more cases; add tests. Date: Tue, 11 Aug 2015 18:30:57 -0500 Message-ID: <86fv3pxv0u.fsf@stephe-leake.org> References: <20150811025613.30799.89490@vcs.savannah.gnu.org> <55CA1F30.6050402@yandex.ru> NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: text/plain X-Trace: ger.gmane.org 1439335910 2068 80.91.229.3 (11 Aug 2015 23:31:50 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Tue, 11 Aug 2015 23:31:50 +0000 (UTC) To: emacs-devel@gnu.org Original-X-From: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Wed Aug 12 01:31:36 2015 Return-path: Envelope-to: ged-emacs-devel@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by plane.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1ZPJ15-0005Eo-H5 for ged-emacs-devel@m.gmane.org; Wed, 12 Aug 2015 01:31:35 +0200 Original-Received: from localhost ([::1]:36137 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZPJ13-00021F-Sk for ged-emacs-devel@m.gmane.org; Tue, 11 Aug 2015 19:31:33 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:55347) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1ZPJ0q-0001w3-Vx for emacs-devel@gnu.org; Tue, 11 Aug 2015 19:31:22 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1ZPJ0l-0002wf-R3 for emacs-devel@gnu.org; Tue, 11 Aug 2015 19:31:20 -0400 Original-Received: from gproxy9-pub.mail.unifiedlayer.com ([69.89.20.122]:35923) by eggs.gnu.org with smtp (Exim 4.71) (envelope-from ) id 1ZPJ0l-0002wQ-JN for emacs-devel@gnu.org; Tue, 11 Aug 2015 19:31:15 -0400 Original-Received: (qmail 25207 invoked by uid 0); 11 Aug 2015 23:31:07 -0000 Original-Received: from unknown (HELO cmgw3) (10.0.90.84) by gproxy9.mail.unifiedlayer.com with SMTP; 11 Aug 2015 23:31:07 -0000 Original-Received: from host114.hostmonster.com ([74.220.207.114]) by cmgw3 with id 3hX21r00C2UdiVW01hX5Xt; Tue, 11 Aug 2015 23:31:06 -0600 X-Authority-Analysis: v=2.1 cv=Qc314Krv c=1 sm=1 tr=0 a=CQdxDb2CKd3SRg4I0/XZPQ==:117 a=CQdxDb2CKd3SRg4I0/XZPQ==:17 a=DsvgjBjRAAAA:8 a=f5113yIGAAAA:8 a=9i_RQKNPAAAA:8 a=y7kgw_RnJtkA:10 a=hEr_IkYJT6EA:10 a=x_XPkuGwIRMA:10 a=uRRa74qj2VoA:10 a=vaJtXVxTAAAA:8 a=ezk2FTXe1Gr_znYjubQA:9 Original-Received: from [76.218.37.33] (port=64369 helo=TAKVER2) by host114.hostmonster.com with esmtpa (Exim 4.84) (envelope-from ) id 1ZPJ0Y-00015E-Tu for emacs-devel@gnu.org; Tue, 11 Aug 2015 17:31:03 -0600 In-Reply-To: <55CA1F30.6050402@yandex.ru> (Dmitry Gutov's message of "Tue, 11 Aug 2015 19:13:36 +0300") User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/24.5 (windows-nt) X-Identified-User: {2442:host114.hostmonster.com:stephele:stephe-leake.org} {sentby:smtp auth 76.218.37.33 authed with stephen_leake@stephe-leake.org} X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x X-Received-From: 69.89.20.122 X-BeenThere: emacs-devel@gnu.org X-Mailman-Version: 2.1.14 Precedence: list List-Id: "Emacs development discussions." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Original-Sender: emacs-devel-bounces+ged-emacs-devel=m.gmane.org@gnu.org Xref: news.gmane.org gmane.emacs.devel:188739 Archived-At: Dmitry Gutov writes: > 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 >> Commit: Stephen Leake >> >> Always output all available definitions. > > How come you saw fit to undo the tweaks that I've added over time? Because they got in the way of some of my uses of xref-find-definitions. In general, rather than using similar heuristics in the low level code, we should be adding hints from the source being searched, and/or the user. For example, one heuristic returned only the function when there are both function and feature with the same name. But there are times when I want to see both, or one or the other. So if I'm searching for the identifier at point, in code like this: (dvc-log-edit ...) ;; point on '-l'; show defun (require 'dvc-log-edit) ;; point on '-l'; show feature In both cases, we can easily tell from the source near point what the user is searching for. > 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. Yes. But I thought the heuristic the previous code used was: "if there is both a variable and a function by the same name, _assume_ they are located in the same place, so only return the function". That assumption is broken for some of my code, and I assume in many others as well. However, you point out later that you used (memq sym minor-mode-list) to determine if this is from define-minor-mode. I didn't realize why that was there when reading the code. So I'll put that back, with a comment this 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. I noticed that as well; I tracked it down. See bug#21237; if you use defconst here, the text properties disappear when emacs is dumped. So I've changed it back to defvar in my local code; it will be pushed soon. >> +(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. The other one is find-alias-regexp. Yes; then other code could use it as well. Should the default value of find-function-regexp-alist contain them? I don't see any harm in that. >> + ((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). Yes, I just ran across that. I'll add a test case. I'm not sure how to detect the default methods, but maybe there is a way. > For instance, looking for project-search-path will display three > entries, but the last two both lead to its generic definition. The three are: (cl-defgeneric project-search-path) (cl-defmethod project-search-path (project)) (cl-defmethod project-search-path ((project (head vc)))) the first shows the cl-defgeneric location. the second os the default method (no specializer arg), but it shows the cl-defmethod for (head vc); I'm not clear why. the third shows the cl-defmethod for (head vc), as it should. >> +;; 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. Ah, that's a good idea. I have a "(setenv "EMACS_TEST_DIRECTORY")" in my notes file for the iteractive case. -- -- Stephe