From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!.POSTED.blaine.gmane.org!not-for-mail From: Dmitry Gutov Newsgroups: gmane.emacs.bugs Subject: bug#35702: xref revert-buffer Date: Fri, 24 May 2019 18:15:57 +0300 Message-ID: <9869e4ac-1b36-b605-22a8-b8bab4910132@yandex.ru> References: <87tvdzv4m2.fsf@mail.linkov.net> <838suw5jvh.fsf@gnu.org> <835zq059az.fsf@gnu.org> <710f0ac1-80d7-6db8-7653-c58f93b6f4ab@yandex.ru> <831s0o54g7.fsf@gnu.org> Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="------------0B7C6A5C3FF3DA92DE031816" Injection-Info: blaine.gmane.org; posting-host="blaine.gmane.org:195.159.176.226"; logging-data="137661"; mail-complaints-to="usenet@blaine.gmane.org" User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:60.0) Gecko/20100101 Thunderbird/60.6.1 Cc: 35702@debbugs.gnu.org, juri@linkov.net To: Eli Zaretskii Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Fri May 24 17:17:19 2019 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([209.51.188.17]) by blaine.gmane.org with esmtps (TLS1.0:RSA_AES_256_CBC_SHA1:256) (Exim 4.89) (envelope-from ) id 1hUBwc-000Za0-VM for geb-bug-gnu-emacs@m.gmane.org; Fri, 24 May 2019 17:17:19 +0200 Original-Received: from localhost ([127.0.0.1]:56162 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hUBwb-0003lR-Hp for geb-bug-gnu-emacs@m.gmane.org; Fri, 24 May 2019 11:17:17 -0400 Original-Received: from eggs.gnu.org ([209.51.188.92]:56554) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1hUBwN-0003jj-Oz for bug-gnu-emacs@gnu.org; Fri, 24 May 2019 11:17:05 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1hUBwM-00061o-6E for bug-gnu-emacs@gnu.org; Fri, 24 May 2019 11:17:03 -0400 Original-Received: from debbugs.gnu.org ([209.51.188.43]:33507) by eggs.gnu.org with esmtps (TLS1.0:RSA_AES_128_CBC_SHA1:16) (Exim 4.71) (envelope-from ) id 1hUBwL-00060w-WC for bug-gnu-emacs@gnu.org; Fri, 24 May 2019 11:17:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1hUBwL-0007Q6-O8 for bug-gnu-emacs@gnu.org; Fri, 24 May 2019 11:17:01 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Dmitry Gutov Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org Resent-Date: Fri, 24 May 2019 15:17:01 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 35702 X-GNU-PR-Package: emacs Original-Received: via spool by 35702-submit@debbugs.gnu.org id=B35702.155871096928443 (code B ref 35702); Fri, 24 May 2019 15:17:01 +0000 Original-Received: (at 35702) by debbugs.gnu.org; 24 May 2019 15:16:09 +0000 Original-Received: from localhost ([127.0.0.1]:47050 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1hUBvV-0007Og-5K for submit@debbugs.gnu.org; Fri, 24 May 2019 11:16:09 -0400 Original-Received: from mail-wm1-f52.google.com ([209.85.128.52]:52101) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1hUBvS-0007OA-UO for 35702@debbugs.gnu.org; Fri, 24 May 2019 11:16:07 -0400 Original-Received: by mail-wm1-f52.google.com with SMTP id f10so2350511wmb.1 for <35702@debbugs.gnu.org>; Fri, 24 May 2019 08:16:06 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=sender:subject:to:cc:references:from:message-id:date:user-agent :mime-version:in-reply-to:content-language; bh=iXwtvloD59c2IJai9prURrPsSx2V5ckO87VBy+yg/h0=; b=WPfiF1p3q53VSdT8rAIYUIsZ2f8QE3ruCteDehKqCmIamIEWTVr+jqSddRH0dhJeN6 pgDEi+G2jcRS9I3/e7qIkbxCWaYCkoBVcFPunDcZVfx8VF3O3nF4uGiHpEJNTFmySNRG ZAEdNj9qF7xrUTpUzyGZwYsZrY+VoJzh9M2F3+cbuPwnAhZetiRmX2hoW0fLt8gME/vl UxD6lBVm1A0O4z8bwXxuHZYDFJSTCI+9FRkZJzyYdXPYF61RpM6Qwu3duo9RLdHSeSN7 juvZE+4DGFLZ2YQ7ELBjpftgxGsMELd8+DqZMX0iTv8o+QFIvPBFFQFxppcIouRPiqp3 ldKQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:sender:subject:to:cc:references:from:message-id :date:user-agent:mime-version:in-reply-to:content-language; bh=iXwtvloD59c2IJai9prURrPsSx2V5ckO87VBy+yg/h0=; b=IEEckePUEvftq5XaCC2nu86CAXhGZcvD2LepYppBCspdQHNZ8hcJ8/safpdrRk0Y+i NaBIle3ltzwGuhmr7fmwqRzIN9PPcKXlR7wE1gkRGMMcnaRdpoCLw8cHddEpju5RD5Ow QnZWZb6D5bKcAQZNiWt0XV/SdwYP1n6LIYB/WD8hVoo3pSnQKRfTGmXKR5O7CFG7GEbk DHwhf0Awhl7Sv6JLED45hCn7EH2Fvm0RDWjrPA/sLJh9p+wb5871LtW8SyIHYhGxWXdM z54fwBptQxFElifZ4UoFfLzG7LdGSRLwvl7ZZuuoryctGKj/rBtxlbGWOaEOQPrE0Ql3 gfAA== X-Gm-Message-State: APjAAAUt6UUJFwfsHo9asJzGCiz5OuV2ZNhRhlG/o3+aUEYsp1/DfXww 9lLq+u6v1jg1KwrRVQyYvGs= X-Google-Smtp-Source: APXvYqxfVf5zMy6coF2LPMXmJkaZJ8+V6f4K7Wnf3LpEQkdHMNnoBeMDgQsj+meSX8VwAXbx+W9BRw== X-Received: by 2002:a1c:48d7:: with SMTP id v206mr17135777wma.38.1558710960759; Fri, 24 May 2019 08:16:00 -0700 (PDT) Original-Received: from [192.168.0.195] ([109.110.245.170]) by smtp.googlemail.com with ESMTPSA id 65sm3888859wro.85.2019.05.24.08.15.58 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 24 May 2019 08:15:59 -0700 (PDT) In-Reply-To: <831s0o54g7.fsf@gnu.org> Content-Language: en-US X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 209.51.188.43 X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.org gmane.emacs.bugs:159716 Archived-At: This is a multi-part message in MIME format. --------------0B7C6A5C3FF3DA92DE031816 Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 7bit On 24.05.2019 17:10, Eli Zaretskii wrote: >> I'm not sure we should document that in the command's docstring, though, >> because I think we'll end up with a more different UI for >> xref-find-definitions results, with a different major mode and a keymap >> where 'g' is simply not bound. > > I'm not a great fan of obfuscating the docs, except for reasons of > hiding internal implementation details. First: my point is, it's possible that it *will* always work wherever you are able to invoke it with 'g'. The Xref buffers where it wouldn't work wouldn't bind 'g'. Second: we can "obfuscate" the docs for things we're not sure about. Think forward: if we expose the Xref UI as a library for other packages in the future (and we probably will), should we allow some of them to opt out of revert-ability (for simplicity, let's say), or not? And if we do, we won't really be able to enumerate all the commands that opted out in xref--revert-xref-buffer's docstring. > Why is it a problem to say > that XREF buffers created by xref-find-definitions currently don't > support 'g'? Because it feels ad-hoc and kinda weird. The intention is to support reverting in "search results" buffers, not intentionally refuse support it in xref-find-definitions. But we can do that. > For that matter, why shouldn't the error message be > explicit about that, instead of saying something technically accurate > but quite obscure from user's POV? How would you phrase it? >> *** Refreshing the search results >> When an Xref buffer shows search results (e.g. from >> xref-find-references or project-find-regexp) you can type 'g' to >> repeat the search and refresh the results. > > This should say explicitly that only some Xref functions can support > refreshing the results. "E.g." can be interpreted as just an example, > not that some other examples don't support this. The intent was to introduce a kind of classification. That is, to call all commands that do a "wide" search as "commands that show search results". xref-find-definitions, meanwhile, might be considered a "jump with completions" command. >> Right, but how often would the event of updading the TAGS file with >> coincide with you wanting to refresh the xref-find-definitions results? > > When the original M-. doesn't show what I expected, for example. Hmm. So it's something you would really find useful? >> Wouldn't you just do the search again with 'M-.'? > > Yes, but that argument is true for any 'revert' action as well, isn't > it? And we still have revert actions. Not exactly: first, M-. is easier to invoke and re-invoke than project-find-regexp, and you are likely to edit the regexp before searching. Second, I quite frequently do something like this: - Do a search for a string with project-find-regexp, - Do *something* with the results, e.g. rename them all, - Repeat the search, to make sure I have dealt with all occurrences. So 'g' helps considerably there. And I don't have any similar scenarios for xref-find-definitions. To be clear, though, we *can* add support for reverting to xref-find-definitions as well, if you want. Just at the cost of a certain increase of complexity, see if you like the patch. xref-find-defitions-revertable.diff is attached. > OK, but (a) the heading sentence should end in a period; (b) please > use 2 spaces between sentences. OK. >> You can probably see a certain level of duplication there, though. > > I don't. I see one entry referencing another, that's all. Just to be clear: I'm referring to two of the three entries I've showed in the previous email mentioning "search-type Xref commands". >> This idea is pretty simple: instead of passing a list of search results >> to xref--show-xrefs, we pass an anonymous function that can be called to >> do the search, as well as repeat it, and returns the said list. > > > The idea is simple and clear, but trying to understand what it does in > any particular invocation isn't. I tried to figure out what happens > after M-., which required me to understand when is the xref--fetcher > variable set and to what values. There's no easy answer to that > question, neither in comments nor in the doc strings. Would a docstring saying "Function that returns a list of xrefs containing the search results" change things? > The value is > set from a function passed as an argument to a couple of other > internal functions, so now I need to go deeper into the rabbit hole > and understand when and how these two functions are called, and with > what function as that argument. Where the fetcher is created is not to hard to find, I think (only a few local variables with that name). > Etc. etc. -- it feels like following > a deeply OO C++ code, where basically the only way to figure out how > the code works is to step through the code with a debugger. It's actually more "functional programming" than OOP, and not the worst example of it (very little function composition, for instance). I can feel your pain (there's a lot of the code I have difficulty following as well), but I'm not sure where I could add comments where they wouldn't be self-evident. A couple of docstrings wouldn't hurt, though. > Except > that Edebug doesn't support generics well enough to do that, so I'm > screwed even if I have the time and energy to go down that hole. Well, at least in this case there are no generic calls between xref-find-references and xref--show-xrefs and the fetcher creation. > It used to be possible to understand what happens in a Lisp program by > just reading the code, but it no longer is, in many cases. Err, my experience of reading old Lisp code in various packages doesn't really agree. > Useful comments can go a long way towards making such investigations > much easier and more efficient, but I guess I will hear > counter-arguments about the need to keep comments up-to-date with the > code... It's not like I'm against those, but it might take a different person to identify the places that need to be commented. --------------0B7C6A5C3FF3DA92DE031816 Content-Type: text/x-patch; name="xref-find-definitions-revertable.diff" Content-Transfer-Encoding: 7bit Content-Disposition: attachment; filename="xref-find-definitions-revertable.diff" diff --git a/lisp/progmodes/xref.el b/lisp/progmodes/xref.el index 6a4906a232..0fd59dc330 100644 --- a/lisp/progmodes/xref.el +++ b/lisp/progmodes/xref.el @@ -782,7 +782,10 @@ xref--analyze #'equal)) (defun xref--show-xref-buffer (fetcher alist) - (let* ((xrefs (if (functionp fetcher) (funcall fetcher) fetcher)) + (let* ((xrefs + (or + (assoc-default 'xrefs alist) + (funcall fetcher))) (xref-alist (xref--analyze xrefs))) (with-current-buffer (get-buffer-create xref-buffer-name) (setq buffer-undo-list nil) @@ -817,13 +820,16 @@ xref--revert-xref-buffer 'face 'error)))) (goto-char (point-min))))) -(defun xref--show-defs-buffer (xrefs alist) - (cond - ((not (cdr xrefs)) - (xref--pop-to-location (car xrefs) - (assoc-default 'display-action alist))) - (t - (xref--show-xref-buffer xrefs alist)))) +(defun xref--show-defs-buffer (fetcher alist) + (let ((xrefs (funcall fetcher))) + (cond + ((not (cdr xrefs)) + (xref--pop-to-location (car xrefs) + (assoc-default 'display-action alist))) + (t + (xref--show-xref-buffer fetcher + (cons (cons 'xrefs xrefs) + alist)))))) (defvar xref-show-xrefs-function 'xref--show-xref-buffer @@ -885,28 +891,29 @@ xref--read-identifier ;;; Commands (defun xref--find-xrefs (input kind arg display-action) + (xref--show-xrefs + (xref--create-fetcher input kind arg) + display-action)) + +(defun xref--find-definitions (id display-action) + (xref--show-defs + (xref--create-fetcher id 'definitions id) + display-action)) + +(defun xref--create-fetcher (input kind arg) (let* ((orig-buffer (current-buffer)) (orig-position (point)) (backend (xref-find-backend)) - (method (intern (format "xref-backend-%s" kind))) - (fetcher (lambda () - (save-excursion - (when (buffer-live-p orig-buffer) - (set-buffer orig-buffer) - (ignore-errors (goto-char orig-position))) - (let ((xrefs (funcall method backend arg))) - (unless xrefs - (xref--not-found-error kind input)) - xrefs))))) - (xref--show-xrefs fetcher display-action))) - -(defun xref--find-definitions (id display-action) - (let ((xrefs (funcall #'xref-backend-definitions - (xref-find-backend) - id))) - (unless xrefs - (xref--not-found-error 'definitions id)) - (xref--show-defs xrefs display-action))) + (method (intern (format "xref-backend-%s" kind)))) + (lambda () + (save-excursion + (when (buffer-live-p orig-buffer) + (set-buffer orig-buffer) + (ignore-errors (goto-char orig-position))) + (let ((xrefs (funcall method backend arg))) + (unless xrefs + (xref--not-found-error kind input)) + xrefs))))) (defun xref--not-found-error (kind input) (user-error "No %s found for: %s" (symbol-name kind) input)) --------------0B7C6A5C3FF3DA92DE031816--