bug#43086: [PATCH] Allow tags backend to not query for TAGS file

bug#43086: [PATCH] Allow tags backend to not query for TAGS file

[Philip K.]

[-- Attachment #1: Type: text/plain, Size: 634 bytes --]


Hi,

the xref backend for etags can be annoying at times, especially in
combination with other backends. This patch should improve the
situation, by allowing the user to configure how and when the etags
backend is activated. The new user option etags-query-file would allow
the backend to never query a TAGS file, or conditionally, depending on
the existence of a TAGS file (in which case it can also be automatically
loaded).

I could imagine this might be extended to allow an auto-generate option,
but that feature seems out of scope of this patch, and probably would
require some interoperation with project.el.

-- 
	Philip K.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Allow-tags-backend-to-not-query-for-TAGS-file.patch --]
[-- Type: text/x-patch, Size: 2381 bytes --]

From 6b141be5123d4cf37d743a9a12818442333658ed Mon Sep 17 00:00:00 2001
From: Philip K <philipk@posteo.net>
Date: Fri, 28 Aug 2020 14:20:56 +0200
Subject: [PATCH] Allow tags backend to not query for TAGS file

* lisp/progmodes/etags.el (etags-query-file): Add variable
(etags--xref-backend): Respect etags-query-file
---
 lisp/progmodes/etags.el | 33 ++++++++++++++++++++++++++++++++-
 1 file changed, 32 insertions(+), 1 deletion(-)

diff --git a/lisp/progmodes/etags.el b/lisp/progmodes/etags.el
index 2c5c36504a..60b162f19e 100644
--- a/lisp/progmodes/etags.el
+++ b/lisp/progmodes/etags.el
@@ -2069,8 +2069,39 @@ etags-xref-find-definitions-tag-order
 If you want `xref-find-definitions' to find the tagged files by their
 file name, add `tag-partial-file-name-match-p' to the list value.")
 
+(defcustom etags-query-file t
+  "Specify how and when to query TAGS file.
+If t, always query if no tags file has been loaded.
+If `check', only query if a TAGS file exists.
+If `check-and-set', automatically set TAGS file if exists, and don't
+query otherwise.
+If `set-or-check', set TAGS file if exists, or query user if not.
+If nil, a new table can be loaded using `visit-tags-table'."
+  :type '(choice (const :tag "Always query" t)
+                 (const :tag "Never query" nil)
+                 (const :tag "Query if exists" check)
+                 (const :tag "Set if exists" check-and-set)
+                 (const :tag "Query if not exists" set-or-check))
+  :version "28.1")
+
 ;;;###autoload
-(defun etags--xref-backend () 'etags)
+(defun etags--xref-backend ()
+  (and (cond ((or (null etags-query-file)
+                  tags-table-computed-list))
+             ((eq etags-query-file 'check)
+              (locate-dominating-file default-directory "TAGS"))
+             ((eq etags-query-file 'check-and-set)
+              (let ((dir (locate-dominating-file default-directory "TAGS")))
+                (when dir
+                  (visit-tags-table dir)
+                  t)))
+             ((eq etags-query-file 'set-or-check)
+              (let ((dir (locate-dominating-file default-directory "TAGS")))
+                (when dir
+                  (visit-tags-table dir))
+                t))
+             (etags-query-file))
+       'etags))
 
 (cl-defmethod xref-backend-identifier-at-point ((_backend (eql etags)))
   (find-tag--default))
-- 
2.26.2

bug#43086: [PATCH] Allow tags backend to not query for TAGS file

[Dmitry Gutov]

Hi!

On 28.08.2020 15:50, Philip K. wrote:

> the xref backend for etags can be annoying at times, especially in
> combination with other backends. This patch should improve the
> situation, by allowing the user to configure how and when the etags
> backend is activated. The new user option etags-query-file would allow
> the backend to never query a TAGS file, or conditionally, depending on
> the existence of a TAGS file (in which case it can also be automatically
> loaded).

This is a interesting patch, but it calls for some discussion:

- The possible values all look pretty clever, but there are a lot of 
them! Do we expect them all to be in demand? Ideally, I'd only leave 2-3 
of them, to reduce the number of workflows we need to care about. The 
rest could probably be set up in individual user configurations in 
find-file-hook (like Projectile does).

- The variable name implies it affects how etags.el works globally, but 
the actual effect seems limited to the xref backend function. We should 
either rename it to something like etags-xref-query-file, or consider 
having it affect tags-completion-at-point-function as well. Maybe 
find-tag too. But given that tags-completion-at-point-function has for a 
long time behaved in the "never query" fashion, perhaps the easiest and 
most backward-compatible option is the former.

- One current persistent annoyance is that currently 
xref-find-references doesn't work well in many files where the xref 
backend is the default one (etags) when ido-mode or icomplete-mode are 
enabled because it prompts for the tags file to do identifier 
completion. I wonder if the "no query" option will help with this, too.

> I could imagine this might be extended to allow an auto-generate option,
> but that feature seems out of scope of this patch, and probably would
> require some interoperation with project.el.

Indeed. Actually, I have an old, WIP patch for tag file auto-generation 
which, yes, uses project.el. I can post it again if you're curious.

bug#43086: [PATCH] Allow tags backend to not query for TAGS file

[Philip K.]

Dmitry Gutov <dgutov@yandex.ru> writes:

> Hi!
>
> On 28.08.2020 15:50, Philip K. wrote:
>
>> the xref backend for etags can be annoying at times, especially in
>> combination with other backends. This patch should improve the
>> situation, by allowing the user to configure how and when the etags
>> backend is activated. The new user option etags-query-file would allow
>> the backend to never query a TAGS file, or conditionally, depending on
>> the existence of a TAGS file (in which case it can also be automatically
>> loaded).
>
> This is a interesting patch, but it calls for some discussion:
>
> - The possible values all look pretty clever, but there are a lot of 
> them! Do we expect them all to be in demand? Ideally, I'd only leave 2-3 
> of them, to reduce the number of workflows we need to care about.

I'm ok with that, the variable could also be turned into a hook if
reducing preconfigured options while making it easy to add new
behaviours.

> The rest could probably be set up in individual user configurations in
> find-file-hook (like Projectile does).

I'm not familiar with how Projectile does this, or how this would work
in general? Could you give an example?

> - The variable name implies it affects how etags.el works globally, but 
> the actual effect seems limited to the xref backend function. We should 
> either rename it to something like etags-xref-query-file, or consider 
> having it affect tags-completion-at-point-function as well.

I'm fine with either, but the first option seems simpler, unless there
is still interest in maintaining the non-xref interface.

> - One current persistent annoyance is that currently 
> xref-find-references doesn't work well in many files where the xref 
> backend is the default one (etags) when ido-mode or icomplete-mode are 
> enabled because it prompts for the tags file to do identifier 
> completion. I wonder if the "no query" option will help with this, too.

Unless I'm misunderstanding something, that's exactly the situation
that motivated me to write these patches (just because of Ivy not Ido).

>> I could imagine this might be extended to allow an auto-generate option,
>> but that feature seems out of scope of this patch, and probably would
>> require some interoperation with project.el.
>
> Indeed. Actually, I have an old, WIP patch for tag file auto-generation 
> which, yes, uses project.el. I can post it again if you're curious.

Sure, why not?

-- 
	Philip K.

bug#43086: [PATCH] Allow tags backend to not query for TAGS file

[Dmitry Gutov]

[-- Attachment #1: Type: text/plain, Size: 4399 bytes --]

On 07.09.2020 00:50, Philip K. wrote:
> Dmitry Gutov <dgutov@yandex.ru> writes:
> 
>> Hi!
>>
>> On 28.08.2020 15:50, Philip K. wrote:
>>
>>> the xref backend for etags can be annoying at times, especially in
>>> combination with other backends. This patch should improve the
>>> situation, by allowing the user to configure how and when the etags
>>> backend is activated. The new user option etags-query-file would allow
>>> the backend to never query a TAGS file, or conditionally, depending on
>>> the existence of a TAGS file (in which case it can also be automatically
>>> loaded).
>>
>> This is a interesting patch, but it calls for some discussion:
>>
>> - The possible values all look pretty clever, but there are a lot of
>> them! Do we expect them all to be in demand? Ideally, I'd only leave 2-3
>> of them, to reduce the number of workflows we need to care about.
> 
> I'm ok with that, the variable could also be turned into a hook if
> reducing preconfigured options while making it easy to add new
> behaviours.

Not sure how the direct conversion to a hook would look like.

In any case, when I talked about using a hook (find-file-hook in 
particular), the main benefit I had in mind is that the effect would 
cover all uses of etags.el. Meaning, also tags-completion-at-point-function.

>> The rest could probably be set up in individual user configurations in
>> find-file-hook (like Projectile does).
> 
> I'm not familiar with how Projectile does this, or how this would work
> in general? Could you give an example?

projectile-mode adds projectile-find-file-hook-function to 
find-file-hook, which, upon visiting any file, looks for 
projectile-tags-file-name (usually "TAGS")'s presence in the root of the 
project. When such file exists, it calls (visit-tags-table tags-file t), 
to visit the tags table "locally".

Since project.el does not have a minor mode, and out of general 
(admittedly handwavy) considerations of performance, we don't do that. 
But a user could customize their find-file-hook with a similar logic. If 
you like, you could also add a pre-existing function like that to 
project.el that a user could then just add to find-file-hook.

>> - The variable name implies it affects how etags.el works globally, but
>> the actual effect seems limited to the xref backend function. We should
>> either rename it to something like etags-xref-query-file, or consider
>> having it affect tags-completion-at-point-function as well.
> 
> I'm fine with either, but the first option seems simpler, unless there
> is still interest in maintaining the non-xref interface.

There might be some other benefit to the latter, but it doesn't seem 
trivial, so the former sounds fine to me as well.

>> - One current persistent annoyance is that currently
>> xref-find-references doesn't work well in many files where the xref
>> backend is the default one (etags) when ido-mode or icomplete-mode are
>> enabled because it prompts for the tags file to do identifier
>> completion. I wonder if the "no query" option will help with this, too.
> 
> Unless I'm misunderstanding something, that's exactly the situation
> that motivated me to write these patches (just because of Ivy not Ido).

I set etags-query-file to nil and call xref-find-references with 
ido-mode enabled. And still get queried for a tags file.

Because etags--xref-backend still returns 'etags, but then 
xref-backend-identifier-completion-table calls 
tags-lazy-completion-table, which does the prompting.

If I make etags--xref-backend return nil, I simply get "No applicable 
method: xref-backend-identifier-completion-table, nil" instead.

So a deeper change is needed.

>>> I could imagine this might be extended to allow an auto-generate option,
>>> but that feature seems out of scope of this patch, and probably would
>>> require some interoperation with project.el.
>>
>> Indeed. Actually, I have an old, WIP patch for tag file auto-generation
>> which, yes, uses project.el. I can post it again if you're curious.
> 
> Sure, why not?

Here it is. It might even be functional.

The reason it's not installed yet, is I was looking to create a seamless 
user experience with it, when they don't need to know which tags file is 
visited, and when. And with tags being quickly updated after a file is 
changed and saved.

The related discussion about necessary changes to etags fizzled out, 
however.

[-- Attachment #2: etags-project.diff --]
[-- Type: text/x-patch, Size: 3058 bytes --]

diff --git a/lisp/progmodes/etags.el b/lisp/progmodes/etags.el
index a31668e1ba..9da2143525 100644
--- a/lisp/progmodes/etags.el
+++ b/lisp/progmodes/etags.el
@@ -2109,7 +2109,9 @@ etags-xref-find-definitions-tag-order
   "Tag order used in `xref-backend-definitions' to look for definitions.")
 
 ;;;###autoload
-(defun etags--xref-backend () 'etags)
+(defun etags--xref-backend ()
+  (etags--maybe-use-project-tags)
+  'etags)
 
 (cl-defmethod xref-backend-identifier-at-point ((_backend (eql etags)))
   (find-tag--default))
@@ -2180,6 +2182,58 @@ xref-make-etags-location
     (nth 1 tag-info)))
 
 \f
+;;; Simple tags generation, with automatic invalidation
+
+(defvar etags--project-tags-file nil)
+(defvar etags--project-tags-root nil)
+
+(defun etags--maybe-use-project-tags ()
+  (let (proj)
+    (when (and etags--project-tags-root
+               (not (file-in-directory-p default-directory
+                                         etags--project-tags-root)))
+      (etags--project-tags-cleanup))
+    (when (and (not (or tags-file-name
+                        tags-table-list))
+               (setq proj (project-current)))
+      (etags--project-tags-generate proj)
+      ;; Invalidate the scanned tags after any change is written to disk.
+      (add-hook 'after-save-hook #'etags--project-tags-cleanup)
+      (visit-tags-table etags--project-tags-file))))
+
+(defun etags--project-tags-generate (proj)
+  (let* ((root (cl-find default-directory
+                        (project-roots proj)
+                        :test #'file-in-directory-p))
+         (default-directory root)
+         (files (all-completions "" (project-file-completion-table proj (list root))))
+         (etags-command (executable-find "etags"))
+         ;; FIXME: List all extensions, or wait for etags fix.
+         ;; http://lists.gnu.org/archive/html/emacs-devel/2018-01/msg00323.html
+         (extensions '("rb" "js" "py" "pl" "el" "c" "cpp" "cc" "h" "hh" "hpp"
+                       "java" "go" "cl" "lisp" "prolog" "php" "erl" "hrl"
+                       "F" "f" "f90" "for" "cs" "a" "asm" "ads" "adb" "ada"))
+         (file-regexp (format "\\.%s\\'" (regexp-opt extensions))))
+    (setq etags--project-tags-file (make-temp-file "emacs-project-tags-")
+          etags--project-tags-root root)
+    (with-temp-buffer
+      (mapc (lambda (f)
+              (when (string-match-p file-regexp f)
+                (insert f "\n")))
+            files)
+      (shell-command-on-region (point-min) (point-max)
+                               (format "%s - -o %s" etags-command etags--project-tags-file)
+                               nil nil "*etags-project-tags-errors*" t))))
+
+(defun etags--project-tags-cleanup ()
+  (when etags--project-tags-file
+    (delete-file etags--project-tags-file)
+    (setq tags-file-name nil
+          tags-table-list nil
+          etags--project-tags-file nil
+          etags--project-tags-root nil))
+  (remove-hook 'after-save-hook #'etags--project-tags-cleanup))
+
 (provide 'etags)
 
 ;;; etags.el ends here

bug#43086: [PATCH] Allow tags backend to not query for TAGS file

[Lars Ingebrigtsen]

Dmitry Gutov <dgutov@yandex.ru> writes:

>>> - The possible values all look pretty clever, but there are a lot of
>>> them! Do we expect them all to be in demand? Ideally, I'd only leave 2-3
>>> of them, to reduce the number of workflows we need to care about.
>> I'm ok with that, the variable could also be turned into a hook if
>> reducing preconfigured options while making it easy to add new
>> behaviours.
>
> Not sure how the direct conversion to a hook would look like.
>
> In any case, when I talked about using a hook (find-file-hook in
> particular), the main benefit I had in mind is that the effect would
> cover all uses of etags.el. Meaning, also
> tags-completion-at-point-function.

This was over a year ago, and it's unclear whether we wanted to do in
Philip's patch's direction or not?  

-- 
(domestic pets only, the antidote for overdose, milk.)
   bloggy blog: http://lars.ingebrigtsen.no

bug#43086: [PATCH] Allow tags backend to not query for TAGS file

[Philip Kaludercic]

Lars Ingebrigtsen <larsi@gnus.org> writes:

> Dmitry Gutov <dgutov@yandex.ru> writes:
>
>>>> - The possible values all look pretty clever, but there are a lot of
>>>> them! Do we expect them all to be in demand? Ideally, I'd only leave 2-3
>>>> of them, to reduce the number of workflows we need to care about.
>>> I'm ok with that, the variable could also be turned into a hook if
>>> reducing preconfigured options while making it easy to add new
>>> behaviours.
>>
>> Not sure how the direct conversion to a hook would look like.
>>
>> In any case, when I talked about using a hook (find-file-hook in
>> particular), the main benefit I had in mind is that the effect would
>> cover all uses of etags.el. Meaning, also
>> tags-completion-at-point-function.
>
> This was over a year ago, and it's unclear whether we wanted to do in
> Philip's patch's direction or not?  

What exactly was the issue with my suggestion?

-- 
	Philip Kaludercic

bug#43086: [PATCH] Allow tags backend to not query for TAGS file

[Lars Ingebrigtsen]

Philip Kaludercic <philipk@posteo.net> writes:

>> This was over a year ago, and it's unclear whether we wanted to do in
>> Philip's patch's direction or not?  
>
> What exactly was the issue with my suggestion?

Dmitry said:

> I set etags-query-file to nil and call xref-find-references with
> ido-mode enabled. And still get queried for a tags file.
> 
> Because etags--xref-backend still returns 'etags, but then
> xref-backend-identifier-completion-table calls
> tags-lazy-completion-table, which does the prompting.
> 
> If I make etags--xref-backend return nil, I simply get "No applicable
> method: xref-backend-identifier-completion-table, nil" instead.
> 
> So a deeper change is needed.

bug#43086: [PATCH] Allow tags backend to not query for TAGS file

[Richard Stallman]

[[[ To any NSA and FBI agents reading my email: please consider    ]]]
[[[ whether defending the US Constitution against all enemies,     ]]]
[[[ foreign or domestic, requires you to follow Snowden's example. ]]]

if we want to support a decision not to query, is it clear that this
decision should be determined by the choice of back end>?
Might it be, rather, a personal preference?

-- 
Dr Richard Stallman (https://stallman.org)
Chief GNUisance of the GNU Project (https://gnu.org)
Founder, Free Software Foundation (https://fsf.org)
Internet Hall-of-Famer (https://internethalloffame.org)