Subject: bug#37820: 25.2; xref using etags incorrect case fold during completion

From: Dmitry Gutov <[email-masked]>
Date: Fri, 17 Jan 2020 04:15:07 +0300

On 26.12.2019 23:56, Radey Shouman wrote:
> Dmitry Gutov writes:
>   > Hi Radey,
>   >
>   > sorry for the late reply.
> 
> Not at all.

And once again. Upon some consideration, I have settled on the most 
backward-compatible option, I think.

> No.  I believe that completion-ignore-case should always be bound to
> the value of tags-case-fold-search when doing a tags completion.  If
> this is not so then completion is not consistent with the tags search,
> which is the bug in question.

Yes, OK. See the attached patch.

The alternative would be to create a new user option local to the xref 
package that would affect both identifier completion and search, but a) 
that will likely be an incompatible change unless tags-case-fold-search 
becomes an obsolete-alias, b) it seems unrealistic to have all backends 
adhere to it anyway.

So let's ask the backends instead.

> On the other hand, it might well be useful to make
> tags-case-fold-search or any xref replacement a safe local variable.

Yes. The patch also does that.

> BTW, how might one find the list of safe local variables?

Well, um. You can evaluate this expression:

(seq-filter
  (lambda (s) (get (intern-soft s) 'safe-local-variable))
  (all-completions "" obarray))

Eli, is this patch okay for emacs-27?

On the one hand, the issue seems minor-ish.

On the other, it's a backward compatibility problem. And the patch must 
be pretty safe because it's basically additive: the only real change is 
to xref--read-identifier, and it calls 
xref-backend-identifier-completion-ignore-case which will default to the 
current behavior for all backends except etags.
diff --git a/lisp/progmodes/etags.el b/lisp/progmodes/etags.el
index fe64895725..897f105019 100644
--- a/lisp/progmodes/etags.el
+++ b/lisp/progmodes/etags.el
@@ -60,7 +60,8 @@ tags-case-fold-search
   :type '(choice (const :tag "Case-sensitive" nil)
 		 (const :tag "Case-insensitive" t)
 		 (other :tag "Use default" default))
-  :version "21.1")
+  :version "21.1"
+  :safe 'symbolp)
 
 ;;;###autoload
 ;; Use `visit-tags-table-buffer' to cycle through tags tables in this list.
@@ -819,9 +820,7 @@ tags-completion-at-point-function
   "Using tags, return a completion table for the text around point.
 If no tags table is loaded, do nothing and return nil."
   (when (or tags-table-list tags-file-name)
-    (let ((completion-ignore-case (if (memq tags-case-fold-search '(t nil))
-				      tags-case-fold-search
-				    case-fold-search))
+    (let ((completion-ignore-case (find-tag--completion-ignore-case))
 	  (pattern (find-tag--default))
 	  beg)
       (when pattern
@@ -836,9 +835,7 @@ tags-completion-at-point-function
 
 (defun find-tag-tag (string)
   "Read a tag name, with defaulting and completion."
-  (let* ((completion-ignore-case (if (memq tags-case-fold-search '(t nil))
-				     tags-case-fold-search
-				   case-fold-search))
+  (let* ((completion-ignore-case (find-tag--completion-ignore-case))
 	 (default (find-tag--default))
 	 (spec (completing-read (if default
 				    (format "%s (default %s): "
@@ -851,6 +848,11 @@ find-tag-tag
 	(or default (user-error "There is no default tag"))
       spec)))
 
+(defun find-tag--completion-ignore-case ()
+  (if (memq tags-case-fold-search '(t nil))
+      tags-case-fold-search
+    case-fold-search))
+
 (defun find-tag--default ()
   (funcall (or find-tag-default-function
                (get major-mode 'find-tag-default-function)
@@ -2072,6 +2074,9 @@ xref-backend-identifier-at-point
 (cl-defmethod xref-backend-identifier-completion-table ((_backend (eql etags)))
   (tags-lazy-completion-table))
 
+(cl-defmethod xref-backend-identifier-completion-ignore-case ((_backend (eql etags)))
+  (find-tag--completion-ignore-case))
+
 (cl-defmethod xref-backend-definitions ((_backend (eql etags)) symbol)
   (etags--xref-find-definitions symbol))
 
@@ -2086,9 +2091,7 @@ etags--xref-find-definitions
          (first-time t)
          (search-fun (if regexp? #'re-search-forward #'search-forward))
          (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 (find-tag--completion-ignore-case))
          (cbuf (current-buffer)))
     (save-excursion
       (while (visit-tags-table-buffer (not first-time) cbuf)
diff --git a/lisp/progmodes/xref.el b/lisp/progmodes/xref.el
index 4fbcd08506..1a34456340 100644
--- a/lisp/progmodes/xref.el
+++ b/lisp/progmodes/xref.el
@@ -287,6 +287,10 @@ xref-backend-identifier-at-point
 (cl-defgeneric xref-backend-identifier-completion-table (backend)
   "Return the completion table for identifiers.")
 
+(cl-defgeneric xref-backend-identifier-completion-ignore-case (_backend)
+  "Return t if case is not significant in identifier completion."
+  completion-ignore-case)
+
 
 ;;; misc utilities
 (defun xref--alistify (list key test)
@@ -967,7 +971,9 @@ xref--prompt-p
 (defun xref--read-identifier (prompt)
   "Return the identifier at point or read it from the minibuffer."
   (let* ((backend (xref-find-backend))
-         (def (xref-backend-identifier-at-point backend)))
+         (def (xref-backend-identifier-at-point backend))
+         (completion-ignore-case
+          (xref-backend-identifier-completion-ignore-case backend)))
     (cond ((or current-prefix-arg
                (not def)
                (xref--prompt-p this-command))