Re: Bugs related to buffer-local tags-file-name

[Eli Zaretskii]

> Date: Fri, 25 Nov 2016 09:38:03 +0200
> From: Eli Zaretskii <address@hidden>
> Cc: address@hidden, address@hidden
> 
> > From: Dmitry Gutov <address@hidden>
> > Date: Fri, 25 Nov 2016 00:10:18 +0200
> > Cc: address@hidden
> > 
> > On 24.11.2016 19:34, Eli Zaretskii wrote:
> > 
> > > I'm working on these bugs.  I hope to be ready with a patch in a day
> > > or two.  It's just a matter of getting one or two key issues right in
> > > etags.el.
> > 
> > Thanks, Eli. Looking forward to testing your patch.
> 
> Glad to help, stay tuned.

The patch is below.  I will try to come up with a few tests later.

For the record, I found 2 issues with local tags tables:

  . Several functions called visit-tags-table-buffer in a loop, but
    since that function returns with current buffer set to the tags
    table buffer it visits, the next call in the loop starts from a
    different buffer, which doesn't DTRT when the tags table is local
    to the buffer where the user invoked the calling command.  The
    patch below teaches visit-tags-table-buffer to start from a
    certain buffer specified by the caller.

  . Somewhat orthogonal, but related problem: tags-completion-table
    was set up as a buffer-local variable in the buffer visiting the
    tags table itself, which makes no sense at all, especially when
    there's more than one tags table, and it gets in the way when the
    tags table is buffer-local.  (It somehow succeeded to work with
    global tags tables, but that's by sheer luck.)

After applying the patch below, all the test cases posted or discussed
in the referenced bugs work as I'd expect.  There's a gray area in
setting a global tags table after you set a local tags table in some
buffer, but the semantics of this is unclear to begin with, and the
results with the patch below make sense to me, as at least one
possible interpretation of what that means.

Please test.

2016-11-26  Eli Zaretskii  <address@hidden>

        * lisp/progmodes/etags.el (visit-tags-table): After
        'visit-tags-table-buffer' returns, retrieve the value of
        'tags-file-name' from the buffer we started in.  Force
        recomputation of 'tags-completion-table' next time it is used,
        since the list of tags table has changed.
        (visit-tags-table-buffer): Accept an additional optional argument
        CBUF, the buffer in which to start processing, and switch to that
        buffer if CBUF is non-nil.  All callers changed to supply a
        non-nil CBUF when they call 'visit-tags-table-buffer' in a loop.
        (tags-completion-table): Accept an argument, the buffer for which
        to build 'tags-completion-table', and that buffer's completion
        table.
        (tags-lazy-completion-table): Pass the current buffer to
        'tags-completion-table'.

diff --git a/lisp/progmodes/etags.el b/lisp/progmodes/etags.el
index 7d4521c..0bd3695 100644
--- a/lisp/progmodes/etags.el
+++ b/lisp/progmodes/etags.el
@@ -304,19 +304,28 @@ visit-tags-table
   ;; Calling visit-tags-table-buffer with tags-file-name set to FILE will
   ;; initialize a buffer for FILE and set tags-file-name to the
   ;; fully-expanded name.
-  (let ((tags-file-name file))
+  (let ((tags-file-name file)
+        (cbuf (current-buffer)))
     (save-excursion
       (or (visit-tags-table-buffer file)
          (signal 'file-missing (list "Visiting tags table"
                                      "No such file or directory"
                                      file)))
-      ;; Set FILE to the expanded name.
-      (setq file tags-file-name)))
+      ;; Set FILE to the expanded name.  Do that in the buffer we
+      ;; started from, because visit-tags-table-buffer switches
+      ;; buffers after updating tags-file-name, so if tags-file-name
+      ;; is local in the buffer we started, that value is only visible
+      ;; in that buffer.
+      (setq file (with-current-buffer cbuf tags-file-name))))
   (if local
-      ;; Set the local value of tags-file-name.
-      (set (make-local-variable 'tags-file-name) file)
+      (progn
+        ;; Force recomputation of tags-completion-table.
+        (setq-local tags-completion-table nil)
+        ;; Set the local value of tags-file-name.
+        (setq-local tags-file-name file))
     ;; Set the global value of tags-file-name.
-    (setq-default tags-file-name file)))
+    (setq-default tags-file-name file)
+    (setq tags-completion-table nil)))
 
 (defun tags-table-check-computed-list ()
   "Compute `tags-table-computed-list' from `tags-table-list' if necessary."
@@ -540,17 +549,20 @@ tags-next-table
     (setq tags-file-name (car tags-table-list-pointer))))
 
 ;;;###autoload
-(defun visit-tags-table-buffer (&optional cont)
+(defun visit-tags-table-buffer (&optional cont cbuf)
   "Select the buffer containing the current tags table.
-If optional arg is a string, visit that file as a tags table.
-If optional arg is t, visit the next table in `tags-table-list'.
-If optional arg is the atom `same', don't look for a new table;
+If optional arg CONT is a string, visit that file as a tags table.
+If optional arg CONT is t, visit the next table in `tags-table-list'.
+If optional arg CONT is the atom `same', don't look for a new table;
  just select the buffer visiting `tags-file-name'.
-If arg is nil or absent, choose a first buffer from information in
+If CONT is nil or absent, choose a first buffer from information in
  `tags-file-name', `tags-table-list', `tags-table-list-pointer'.
+Optional second arg CBUF, if non-nil, specifies the initial buffer,
+which is important if that buffer has a local value of `tags-file-name'.
 Returns t if it visits a tags table, or nil if there are no more in the list."
 
   ;; Set tags-file-name to the tags table file we want to visit.
+  (if cbuf (set-buffer cbuf))
   (cond ((eq cont 'same)
         ;; Use the ambient value of tags-file-name.
         (or tags-file-name
@@ -752,28 +764,30 @@ tags-included-tables
   (or tags-included-tables
       (setq tags-included-tables (funcall tags-included-tables-function))))
 
-(defun tags-completion-table ()
-  "Build `tags-completion-table' on demand.
+(defun tags-completion-table (buf)
+  "Build `tags-completion-table' on demand for BUF's tags tables.
 The tags included in the completion table are those in the current
-tags table and its (recursively) included tags tables."
-  (or tags-completion-table
-      ;; No cached value for this buffer.
-      (condition-case ()
-         (let (tables cont)
-           (message "Making tags completion table for %s..." buffer-file-name)
-           (save-excursion
-             ;; Iterate over the current list of tags tables.
-             (while (visit-tags-table-buffer cont)
-               ;; Find possible completions in this table.
-                (push (funcall tags-completion-table-function) tables)
-                (setq cont t)))
-           (message "Making tags completion table for %s...done"
-                    buffer-file-name)
-           ;; Cache the result in a buffer-local variable.
-           (setq tags-completion-table
-                  (nreverse (delete-dups (apply #'nconc tables)))))
-       (quit (message "Tags completion table construction aborted.")
-             (setq tags-completion-table nil)))))
+tags table for BUF and its (recursively) included tags tables."
+  (with-current-buffer buf
+    (or tags-completion-table
+        ;; No cached value for this buffer.
+        (condition-case ()
+            (let (tables cont)
+              (message "Making tags completion table for %s..."
+                       buffer-file-name)
+              (save-excursion
+                ;; Iterate over the current list of tags tables.
+                (while (visit-tags-table-buffer cont buf)
+                  ;; Find possible completions in this table.
+                  (push (funcall tags-completion-table-function) tables)
+                  (setq cont t)))
+              (message "Making tags completion table for %s...done"
+                       buffer-file-name)
+              ;; Cache the result in a variable.
+              (setq tags-completion-table
+                    (nreverse (delete-dups (apply #'nconc tables)))))
+          (quit (message "Tags completion table construction aborted.")
+                (setq tags-completion-table nil))))))
 
 ;;;###autoload
 (defun tags-lazy-completion-table ()
@@ -784,7 +798,9 @@ tags-lazy-completion-table
           ;; If we need to ask for the tag table, allow that.
           (let ((enable-recursive-minibuffers t))
             (visit-tags-table-buffer))
-          (complete-with-action action (tags-completion-table) string 
pred))))))
+          (complete-with-action action
+                                (tags-completion-table buf)
+                                string pred))))))
 
 ;;;###autoload (defun tags-completion-at-point-function ()
 ;;;###autoload   (if (or tags-table-list tags-file-name)
@@ -1084,6 +1100,7 @@ find-tag-in-order
        (case-fold-search (if (memq tags-case-fold-search '(nil t))
                              tags-case-fold-search
                            case-fold-search))
+        (cbuf (current-buffer))
        )
     (save-excursion
 
@@ -1104,8 +1121,7 @@ find-tag-in-order
       (catch 'qualified-match-found
 
        ;; Iterate over the list of tags tables.
-       (while (or first-table
-                  (visit-tags-table-buffer t))
+       (while (or first-table (visit-tags-table-buffer t cbuf))
 
          (and first-search first-table
               ;; Start at beginning of tags file.
@@ -1707,25 +1723,26 @@ next-file
        ((eq initialize t)
         ;; Initialize the list from the tags table.
         (save-excursion
-          ;; Visit the tags table buffer to get its list of files.
-          (visit-tags-table-buffer)
-          ;; Copy the list so we can setcdr below, and expand the file
-          ;; names while we are at it, in this buffer's default directory.
-          (setq next-file-list (mapcar 'expand-file-name (tags-table-files)))
-          ;; Iterate over all the tags table files, collecting
-          ;; a complete list of referenced file names.
-          (while (visit-tags-table-buffer t)
-            ;; Find the tail of the working list and chain on the new
-            ;; sublist for this tags table.
-            (let ((tail next-file-list))
-              (while (cdr tail)
-                (setq tail (cdr tail)))
-              ;; Use a copy so the next loop iteration will not modify the
-              ;; list later returned by (tags-table-files).
-              (if tail
-                  (setcdr tail (mapcar 'expand-file-name (tags-table-files)))
-                (setq next-file-list (mapcar 'expand-file-name
-                                             (tags-table-files))))))))
+           (let ((cbuf (current-buffer)))
+             ;; Visit the tags table buffer to get its list of files.
+             (visit-tags-table-buffer)
+             ;; Copy the list so we can setcdr below, and expand the file
+             ;; names while we are at it, in this buffer's default directory.
+             (setq next-file-list (mapcar 'expand-file-name 
(tags-table-files)))
+             ;; Iterate over all the tags table files, collecting
+             ;; a complete list of referenced file names.
+             (while (visit-tags-table-buffer t cbuf)
+               ;; Find the tail of the working list and chain on the new
+               ;; sublist for this tags table.
+               (let ((tail next-file-list))
+                 (while (cdr tail)
+                   (setq tail (cdr tail)))
+                 ;; Use a copy so the next loop iteration will not modify the
+                 ;; list later returned by (tags-table-files).
+                 (if tail
+                     (setcdr tail (mapcar 'expand-file-name 
(tags-table-files)))
+                   (setq next-file-list (mapcar 'expand-file-name
+                                                (tags-table-files)))))))))
        (t
         ;; Initialize the list by evalling the argument.
         (setq next-file-list (eval initialize))))
@@ -1921,8 +1938,9 @@ list-tags
     (princ (substitute-command-keys "':\n\n"))
     (save-excursion
       (let ((first-time t)
-           (gotany nil))
-       (while (visit-tags-table-buffer (not first-time))
+           (gotany nil)
+            (cbuf (current-buffer)))
+       (while (visit-tags-table-buffer (not first-time) cbuf)
          (setq first-time nil)
          (if (funcall list-tags-function file)
              (setq gotany t)))
@@ -1945,8 +1963,9 @@ tags-apropos
     (tags-with-face 'highlight (princ regexp))
     (princ (substitute-command-keys "':\n\n"))
     (save-excursion
-      (let ((first-time t))
-       (while (visit-tags-table-buffer (not first-time))
+      (let ((first-time t)
+            (cbuf (current-buffer)))
+       (while (visit-tags-table-buffer (not first-time) cbuf)
          (setq first-time nil)
          (funcall tags-apropos-function regexp))))
     (etags-tags-apropos-additional regexp))
@@ -2107,9 +2126,10 @@ etags--xref-find-definitions
          (marks (make-hash-table :test 'equal))
          (case-fold-search (if (memq tags-case-fold-search '(nil t))
                                tags-case-fold-search
-                             case-fold-search)))
+                             case-fold-search))
+         (cbuf (current-buffer)))
     (save-excursion
-      (while (visit-tags-table-buffer (not first-time))
+      (while (visit-tags-table-buffer (not first-time) cbuf)
         (setq first-time nil)
         (dolist (order-fun (cond (regexp? find-tag-regexp-tag-order)
                                  (t etags-xref-find-definitions-tag-order)))