Subject: Re: bug#76440: 30.1; different behavaiour of 29.4 and 30 for ivy completition of absolute paths

From: "Basil L. Contovounesios" <[email-masked]>
Date: Mon, 24 Feb 2025 12:51:25 +0100

tags 76440 + patch
quit

Evgeniy Dushistov [2025-02-20 22:50 +0300] wrote:
>> > Evgeniy Dushistov writes:
>> >
>> >> After start open "/tmp" directory, and then press
>> >> C-x C-f and then type /var, the content of minibuffer would be
>> >> "/tmp//var",
>> >> if after that press TAB
>> >> the result would be "/tmp/var"
>> >> while expected "/var".
>> >>
>> >> emacs 29.4 gives "/var", while emacs 30.1-rc1 gives "/tmp/var".
>
> The similar, but much worse issue competition of absolute paths
> with gud-gdb+ivy:
>
> M-x gud-gdb
> In minibuffer:
> "gdb --fullname /tmp"
>
> press TAB, and for some reason get:
> "gdb --fullname tmp/"
>
> ok, I fix this and add almost all path to 
> existing directory (build-dir):
> "gdb --fullname /tmp/build-d"
>
> press TAB, and get:
> "gdb --fullname build-dir"
>
> so it just remove for some reason "/tmp" when complete path,
> again I fix it in minibuffer, and add almost all path to existing
> binary(exe):
> "gdb --fullname /tmp/build-dir/ex"
>
> press TAB and get:
> "gdb --fullname /tmp/build-direxe"
>
> instead of
> "gdb --fullname /tmp/build-dir/exe"
>
> So with gdb-gud+ivy+emacs 30.1 TAB almost always gives not
> valid path for some reason, while with emacs 29.4 all works perfect.

Could you please check whether

  https://github.com/abo-abo/swiper/pull/3065

fixes these issues for you?

I attach the constituent patches for convenience.

Thanks,
-- 
Basil

>From 576f421b024eee7d087ab204f54071fa93dd0493 Mon Sep 17 00:00:00 2001
From: "Basil L. Contovounesios" <[email-masked]>
Date: Fri, 21 Feb 2025 23:39:52 +0100
Subject: [PATCH 1/5] Fix ivy-completion-in-region return value

* ivy.el (ivy-completion-in-region): Return nil/t on no/sole match
as well, not just in the multi-match case.  Tiny local variable
renaming and simplifications.  Add FIXME for initial-input woes.
---
 ivy.el | 39 +++++++++++++++++++++------------------
 1 file changed, 21 insertions(+), 18 deletions(-)

diff --git a/ivy.el b/ivy.el
index 7a11c96..38a4575 100644
--- a/ivy.el
+++ b/ivy.el
@@ -2663,43 +2663,43 @@ ivy-completion-in-region
 PREDICATE (a function called with no arguments) says when to exit.
 See `completion-in-region' for further information."
   (let* ((enable-recursive-minibuffers t)
+         (reg (- end start))
          (str (buffer-substring-no-properties start end))
          (completion-ignore-case (ivy--case-fold-p str))
          (md (completion-metadata str collection predicate))
-         (reg (- end start))
-         (comps (completion-all-completions str collection predicate reg md))
          (try (completion-try-completion str collection predicate reg md))
+         (comps (completion-all-completions str collection predicate reg md))
          (ivy--minibuffer-table collection)
          (ivy--minibuffer-pred predicate))
     (cond ((null comps)
-           (message "No matches"))
+           (message "No matches")
+           nil)
           ((progn
              (nconc comps nil)
              (and (null (cdr comps))
                   (string= str (car comps))))
-           (message "Sole match"))
+           (message "Sole match")
+           t)
           (t
            (when (eq collection 'crm--collection-fn)
              (setq comps (delete-dups comps)))
-           (let* ((len (ivy-completion-common-length (car comps)))
-                  (initial (cond ((= len 0)
+           (let* ((cmn (ivy-completion-common-length (car comps)))
+                  (initial (cond ((= cmn 0)
                                   "")
-                                 ((let ((str-len (length str)))
-                                    (when (> len str-len)
-                                      (setq len str-len)
-                                      str)))
+                                 ((>= cmn reg)
+                                  (setq cmn reg)
+                                  str)
                                  (t
-                                  (substring str (- len))))))
-             (delete-region (- end len) end)
-             (setq ivy-completion-beg (- end len))
+                                  (substring str (- cmn))))))
+             (delete-region (- end cmn) end)
+             (setq ivy-completion-beg (- end cmn))
              (setq ivy-completion-end ivy-completion-beg)
              (if (null (cdr comps))
-                 (progn
+                 (let ((ivy--minibuffer-try try))
                    (unless (minibuffer-window-active-p (selected-window))
                      (setf (ivy-state-window ivy-last) (selected-window)))
-                   (let ((ivy--minibuffer-try try))
-                     (ivy-completion-in-region-action
-                      (substring-no-properties (car comps)))))
+                   (ivy-completion-in-region-action
+                    (substring-no-properties (car comps))))
                (dolist (s comps)
                  ;; Remove face `completions-first-difference'.
                  (ivy--remove-props s 'face))
@@ -2713,6 +2713,9 @@ ivy-completion-in-region
                  (setq predicate nil))
                (ivy-read (format "(%s): " str) collection
                          :predicate predicate
+                         ;; FIXME: The anchor is intrusive and not easily
+                         ;; configurable by `ivy-initial-inputs-alist' or
+                         ;; `ivy-hooks-alist'.
                          :initial-input (concat
                                          (and (derived-mode-p #'emacs-lisp-mode)
                                               "^")
@@ -2724,7 +2727,7 @@ ivy-completion-in-region
                                      (when initial
                                        (insert initial))))
                          :caller 'ivy-completion-in-region)))
-           ;; Return value should be non-nil on valid completion;
+           ;; Return value should be t on valid completion;
            ;; see `completion-in-region'.
            t))))
 
-- 
2.47.2

>From 05f1d8218180457c6cbd72ae1c7c3355b574f8bc Mon Sep 17 00:00:00 2001
From: "Basil L. Contovounesios" <[email-masked]>
Date: Sun, 23 Feb 2025 12:43:21 +0100
Subject: [PATCH 2/5] Fix some point placement on exact completion exit

When completing ~/foo and ~/foobar is the only completion,
vanilla completion inserts a trailing space on exit to indicate an
exact match, even though completion-try-completion returned a cons.

This is determined by completion--done which calls try-completion on
the completion-try-completion return value i.s.o. the
completion-all-completions candidate that was inserted.

* ivy.el (ivy-completion-in-region-action): Prefer passing
completion-try-completion 'new string' to completion--done when
possible.
---
 ivy.el | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

diff --git a/ivy.el b/ivy.el
index 38a4575..a109025 100644
--- a/ivy.el
+++ b/ivy.el
@@ -2622,10 +2622,11 @@ ivy-completion-in-region-action
                                            (ivy-state-collection ivy-last)))
             (minibuffer-completion-predicate (if (boundp 'ivy--minibuffer-pred)
                                                  ivy--minibuffer-pred
-                                               (ivy-state-predicate ivy-last))))
-        (completion--done str (cond ((eq ivy--minibuffer-try t) 'finished)
-                                    ((eq ivy-exit 'done) 'unknown)
-                                    ('exact))))
+                                               (ivy-state-predicate ivy-last)))
+            (newstr (or (car-safe ivy--minibuffer-try) str)))
+        (completion--done newstr (cond ((eq ivy--minibuffer-try t) 'finished)
+                                       ((eq ivy-exit 'done) 'unknown)
+                                       ('exact))))
       (setq ivy-completion-end (point))
       (save-excursion
         (dolist (cursor fake-cursors)
-- 
2.47.2

>From 65db8f0245dd8d3735ab9aee546590a578f7501a Mon Sep 17 00:00:00 2001
From: "Basil L. Contovounesios" <[email-masked]>
Date: Thu, 20 Feb 2025 22:53:56 +0100
Subject: [PATCH 3/5] Add ivy-completion-common-length regression tests

* ivy-test.el (ivy-completion-common-length): Extend tests.
---
 ivy-test.el | 76 +++++++++++++++++++++++++++++++++++++++++++----------
 1 file changed, 62 insertions(+), 14 deletions(-)

diff --git a/ivy-test.el b/ivy-test.el
index 87dda18..e33eccb 100644
--- a/ivy-test.el
+++ b/ivy-test.el
@@ -1007,20 +1007,68 @@ ivy-completing-read-handlers
       (ivy-mode ivy-mode-reset-arg))))
 
 (ert-deftest ivy-completion-common-length ()
-  (should (= 2
-             (ivy-completion-common-length
-              #("test/"
-                0 2 (face completions-common-part)
-                2 3 (face (completions-first-difference))))))
-  (should (= 5
-             (ivy-completion-common-length
-              #("Math/E"
-                0 5 (face (completions-common-part))
-                5 6 (face (completions-first-difference))))))
-  (should (= 3
-             (ivy-completion-common-length
-              #("vec"
-                0 3 (face (completions-common-part)))))))
+  (mapc (lambda (pair)
+          (dolist (str (cdr pair))
+            (ert-info ((format "%S" str) :prefix "String: ")
+              (should (= (ivy-completion-common-length str) (car pair))))))
+        '((0 ""
+             #("a"   0 1 (face (completions-first-difference)))
+             #("ab"  0 1 (face (completions-first-difference)))
+             #("abc" 0 1 (face (completions-first-difference)))
+             #("a"   0 1 (font-lock-face (completions-first-difference)))
+             #("ab"  0 1 (font-lock-face (completions-first-difference)))
+             #("abc" 0 1 (font-lock-face (completions-first-difference)))
+             #("abc"
+               0 1 (face (completions-first-difference))
+               1 2 (face default))
+             #("abc"
+               0 1 (face (completions-first-difference))
+               2 3 (face default)))
+          (1 "a"
+             #("a"   0 1 (face default))
+             #("ab"  1 2 (face (completions-first-difference)))
+             #("ab"  0 2 (face (completions-first-difference)))
+             #("abc" 1 2 (face (completions-first-difference)))
+             #("abc" 0 2 (face (completions-first-difference)))
+             #("ab"  1 2 (font-lock-face (completions-first-difference)))
+             #("ab"  0 2 (font-lock-face (completions-first-difference)))
+             #("abc" 1 2 (font-lock-face (completions-first-difference)))
+             #("abc" 0 2 (font-lock-face (completions-first-difference)))
+             #("abc"
+               0 1 (face default)
+               1 2 (face (completions-first-difference)))
+             #("abc"
+               1 2 (face (completions-first-difference))
+               2 3 (face default)))
+          (2 "ab"
+             #("ab"  0 1 (face default))
+             #("ab"  1 2 (face default))
+             #("ab"  0 2 (face default))
+             #("abc" 2 3 (face (completions-first-difference)))
+             #("abc" 1 3 (face (completions-first-difference)))
+             #("abc" 0 3 (face (completions-first-difference)))
+             #("abc" 2 3 (font-lock-face (completions-first-difference)))
+             #("abc" 1 3 (font-lock-face (completions-first-difference)))
+             #("abc" 0 3 (font-lock-face (completions-first-difference)))
+             #("abc"
+               0 1 (face default)
+               2 3 (face (completions-first-difference)))
+             #("abc"
+               1 2 (face default)
+               2 3 (face (completions-first-difference)))
+             #("test/"
+               0 2 (face completions-common-part)
+               2 3 (face (completions-first-difference))))
+          (3 "abc"
+             #("abc" 0 1 (face default))
+             #("abc" 1 2 (face default))
+             #("abc" 2 3 (face default))
+             #("abc" 0 2 (face default))
+             #("abc" 1 3 (face default))
+             #("abc" 0 3 (face default)))
+          (5 #("Math/E"
+               0 5 (face completions-common-part)
+               5 6 (face (completions-first-difference)))))))
 
 (ert-deftest ivy--sort-function ()
   "Test `ivy--sort-function' behavior."
-- 
2.47.2

>From 5a041b11aa68359a0d027adfba43d2c4aa59e622 Mon Sep 17 00:00:00 2001
From: "Basil L. Contovounesios" <[email-masked]>
Date: Fri, 21 Feb 2025 23:39:00 +0100
Subject: [PATCH 4/5] Fix ivy-completion-common-length

Thanks to Kien Nguyen <[email-masked]> and
Madhu <[email-masked]> for suggestions.

Fixes parts of #1361, #1755, #2879, #3051, #3056,
and https://bugs.gnu.org/76440.
Relates to https://bugs.gnu.org/71419.

* ivy.el (ivy--face-list-p): New compatibility shim.
(ivy-completion-common-length): Use it to handle both atom and list
values for the face property, since it varies across Emacs versions.
Use previous-single-property-change instead of iterating by char.
Improve documentation.
(ivy-completion-in-region): Translate a 'not found' result of
ivy-completion-common-length into a first difference at index 0, not
at the end of the string.  That means, interpret a lack of
completions-first-difference as no commonality.  This is more
consistent with both the definition of ivy-completion-common-length
and completion changes in Emacs 30.

* ivy-test.el (ivy-completion-common-length): Extend tests.
---
 ivy-test.el | 34 +++++++++++++++++++++++++++++++
 ivy.el      | 58 +++++++++++++++++++++++++++++++++++++----------------
 2 files changed, 75 insertions(+), 17 deletions(-)

diff --git a/ivy-test.el b/ivy-test.el
index e33eccb..e22645d 100644
--- a/ivy-test.el
+++ b/ivy-test.el
@@ -1012,12 +1012,21 @@ ivy-completion-common-length
             (ert-info ((format "%S" str) :prefix "String: ")
               (should (= (ivy-completion-common-length str) (car pair))))))
         '((0 ""
+             #("a"   0 1 (face completions-first-difference))
+             #("ab"  0 1 (face completions-first-difference))
+             #("abc" 0 1 (face completions-first-difference))
              #("a"   0 1 (face (completions-first-difference)))
              #("ab"  0 1 (face (completions-first-difference)))
              #("abc" 0 1 (face (completions-first-difference)))
              #("a"   0 1 (font-lock-face (completions-first-difference)))
              #("ab"  0 1 (font-lock-face (completions-first-difference)))
              #("abc" 0 1 (font-lock-face (completions-first-difference)))
+             #("abc"
+               0 1 (face completions-first-difference)
+               1 2 (face default))
+             #("abc"
+               0 1 (face completions-first-difference)
+               2 3 (face default))
              #("abc"
                0 1 (face (completions-first-difference))
                1 2 (face default))
@@ -1026,6 +1035,10 @@ ivy-completion-common-length
                2 3 (face default)))
           (1 "a"
              #("a"   0 1 (face default))
+             #("ab"  1 2 (face completions-first-difference))
+             #("ab"  0 2 (face completions-first-difference))
+             #("abc" 1 2 (face completions-first-difference))
+             #("abc" 0 2 (face completions-first-difference))
              #("ab"  1 2 (face (completions-first-difference)))
              #("ab"  0 2 (face (completions-first-difference)))
              #("abc" 1 2 (face (completions-first-difference)))
@@ -1034,6 +1047,12 @@ ivy-completion-common-length
              #("ab"  0 2 (font-lock-face (completions-first-difference)))
              #("abc" 1 2 (font-lock-face (completions-first-difference)))
              #("abc" 0 2 (font-lock-face (completions-first-difference)))
+             #("abc"
+               0 1 (face default)
+               1 2 (face completions-first-difference))
+             #("abc"
+               1 2 (face completions-first-difference)
+               2 3 (face default))
              #("abc"
                0 1 (face default)
                1 2 (face (completions-first-difference)))
@@ -1044,18 +1063,30 @@ ivy-completion-common-length
              #("ab"  0 1 (face default))
              #("ab"  1 2 (face default))
              #("ab"  0 2 (face default))
+             #("abc" 2 3 (face completions-first-difference))
+             #("abc" 1 3 (face completions-first-difference))
+             #("abc" 0 3 (face completions-first-difference))
              #("abc" 2 3 (face (completions-first-difference)))
              #("abc" 1 3 (face (completions-first-difference)))
              #("abc" 0 3 (face (completions-first-difference)))
              #("abc" 2 3 (font-lock-face (completions-first-difference)))
              #("abc" 1 3 (font-lock-face (completions-first-difference)))
              #("abc" 0 3 (font-lock-face (completions-first-difference)))
+             #("abc"
+               0 1 (face default)
+               2 3 (face completions-first-difference))
+             #("abc"
+               1 2 (face default)
+               2 3 (face completions-first-difference))
              #("abc"
                0 1 (face default)
                2 3 (face (completions-first-difference)))
              #("abc"
                1 2 (face default)
                2 3 (face (completions-first-difference)))
+             #("test/"
+               0 2 (face completions-common-part)
+               2 3 (face completions-first-difference))
              #("test/"
                0 2 (face completions-common-part)
                2 3 (face (completions-first-difference))))
@@ -1067,6 +1098,9 @@ ivy-completion-common-length
              #("abc" 1 3 (face default))
              #("abc" 0 3 (face default)))
           (5 #("Math/E"
+               0 5 (face completions-common-part)
+               5 6 (face completions-first-difference))
+             #("Math/E"
                0 5 (face completions-common-part)
                5 6 (face (completions-first-difference)))))))
 
diff --git a/ivy.el b/ivy.el
index a109025..fe3f58a 100644
--- a/ivy.el
+++ b/ivy.el
@@ -2639,24 +2639,44 @@ ivy-completion-in-region-action
           (set-marker (overlay-get cursor 'point) (point))
           (set-marker (overlay-get cursor 'mark) (point)))))))
 
+(defalias 'ivy--face-list-p
+  (if (fboundp 'face-list-p)
+      #'face-list-p
+    (lambda (face)
+      (and (listp face)
+           (listp (cdr face))
+           (not (keywordp (car face))))))
+  "Compatibility shim for Emacs 25 `face-list-p'.")
+
+;; FIXME: Should this return the smallest such index instead?
+;; Usually the two are equal, but perhaps there exist more
+;; exotic applications of `completions-first-difference'.
+;;
+;; Completing files under a directory foo/ can have a first difference at
+;; index 0 in some Emacs versions, and no such property in other versions.
+;; So perhaps this function should return 0 instead of (length str) when no
+;; property is found?  That still follows the 'largest index' definition.
 (defun ivy-completion-common-length (str)
-  "Return the amount of characters that match in  STR.
-
-`completion-all-completions' computes this and returns the result
-via text properties.
-
-The first non-matching part is propertized:
-- either with: (face (completions-first-difference))
-- or: (font-lock-face completions-first-difference)."
-  (let ((char-property-alias-alist '((face font-lock-face)))
-        (i (1- (length str))))
-    (catch 'done
-      (while (>= i 0)
-        (when (equal (get-text-property i 'face str)
-                     '(completions-first-difference))
-          (throw 'done i))
-        (cl-decf i))
-      (throw 'done (length str)))))
+  "Return the length of the completion-matching prefix of STR.
+
+That is, return the largest index into STR at which either the
+`face' or `font-lock-face' property value contains the face
+`completions-first-difference'.
+If no such index is found, return the length of STR.
+
+Typically the completion-matching parts of STR have previously been
+propertized by `completion-all-completions'."
+  (let* ((char-property-alias-alist '((face font-lock-face)))
+         (cmn (length str))
+         (i cmn))
+    (when (> i 0)
+      (while (if (let ((face (get-text-property (1- i) 'face str)))
+                   (or (eq 'completions-first-difference face)
+                       (and (ivy--face-list-p face)
+                            (memq 'completions-first-difference face))))
+                 (ignore (setq cmn (1- i)))
+               (setq i (previous-single-property-change i 'face str)))))
+    cmn))
 
 (defun ivy-completion-in-region (start end collection &optional predicate)
   "An Ivy function suitable for `completion-in-region-function'.
@@ -2685,6 +2705,10 @@ ivy-completion-in-region
            (when (eq collection 'crm--collection-fn)
              (setq comps (delete-dups comps)))
            (let* ((cmn (ivy-completion-common-length (car comps)))
+                  ;; Translate a 'not found' result to 0.  Do this here (instead
+                  ;; of fixing `ivy-completion-common-length') for backward
+                  ;; compatibility, since it's a potentially public function.
+                  (cmn (if (= cmn (length (car comps))) 0 cmn))
                   (initial (cond ((= cmn 0)
                                   "")
                                  ((>= cmn reg)
-- 
2.47.2

>From d5e3974cc47d21b66cfeadaceffc7e3bf6e267a3 Mon Sep 17 00:00:00 2001
From: "Basil L. Contovounesios" <[email-masked]>
Date: Mon, 24 Feb 2025 09:47:29 +0100
Subject: [PATCH 5/5] Prefer base-size over ivy-completion-common-length

The latter is very problematic: it is specific to a single
candidate, does not necessarily correspond to the string being
completed, complicates bounds calculations, relies on only partially
documented presentation details rather than the usual API, etc.

When completion-all-completions returns a base-size (which in
practice seems to be most if not all the time), it corresponds more
closely to the substring that is being completed and will be
replaced.

This should address most of the remaining issues in #1361, #1755,

* ivy.el (ivy-completion-common-length): Discourage use.
(ivy-completion-in-region): When completion-all-completions
returns a base-size, prefer that over the
completions-first-difference face to determine the completion
boundaries and initial-input.

Fixes #1755.
Fixes #2879.
Closes #3051.
Fixes #3056.
---
 ivy.el | 26 +++++++++++++++++---------
 1 file changed, 17 insertions(+), 9 deletions(-)

diff --git a/ivy.el b/ivy.el
index fe3f58a..5f8e608 100644
--- a/ivy.el
+++ b/ivy.el
@@ -2665,7 +2665,9 @@ ivy-completion-common-length
 If no such index is found, return the length of STR.
 
 Typically the completion-matching parts of STR have previously been
-propertized by `completion-all-completions'."
+propertized by `completion-all-completions', but then the base-size
+returned by that function should be preferred over
+`ivy-completion-common-length'."
   (let* ((char-property-alias-alist '((face font-lock-face)))
          (cmn (length str))
          (i cmn))
@@ -2690,15 +2692,16 @@ ivy-completion-in-region
          (md (completion-metadata str collection predicate))
          (try (completion-try-completion str collection predicate reg md))
          (comps (completion-all-completions str collection predicate reg md))
+         (last (last comps))
+         (base-size (cdr last))
          (ivy--minibuffer-table collection)
          (ivy--minibuffer-pred predicate))
+    (when last (setcdr last ()))
     (cond ((null comps)
            (message "No matches")
            nil)
-          ((progn
-             (nconc comps nil)
-             (and (null (cdr comps))
-                  (string= str (car comps))))
+          ((and (null (cdr comps))
+                (string= str (car comps)))
            (message "Sole match")
            t)
           (t
@@ -2709,15 +2712,20 @@ ivy-completion-in-region
                   ;; of fixing `ivy-completion-common-length') for backward
                   ;; compatibility, since it's a potentially public function.
                   (cmn (if (= cmn (length (car comps))) 0 cmn))
-                  (initial (cond ((= cmn 0)
+                  (initial (cond (base-size (substring str base-size))
+                                 ;; The remaining clauses should hopefully never
+                                 ;; be taken, since they rely on
+                                 ;; `ivy-completion-common-length'.
+                                 ((= cmn 0)
                                   "")
                                  ((>= cmn reg)
                                   (setq cmn reg)
                                   str)
                                  (t
-                                  (substring str (- cmn))))))
-             (delete-region (- end cmn) end)
-             (setq ivy-completion-beg (- end cmn))
+                                  (substring str (- cmn)))))
+                  (base-pos (if base-size (+ start base-size) (- end cmn))))
+             (delete-region base-pos end)
+             (setq ivy-completion-beg base-pos)
              (setq ivy-completion-end ivy-completion-beg)
              (if (null (cdr comps))
                  (let ((ivy--minibuffer-try try))
-- 
2.47.2