Fix completion boundaries for TRAMP file names

Previously, we assumed (roughly) that substitute-in-file-name
always returns a suffix of the original string.  But
substitute-in-file-name on "/ssh:user@host:/~/" returns
"/ssh:user@host:~/", preserving the TRAMP magic prefix.  Weaken
the assertion in completion--sifn-boundaries to allow this; the
new assertion is more clear about the property we care about,
anyway.

* lisp/minibuffer.el (completion--sifn-boundaries): Weaken
assertion slightly.

* test/lisp/net/tramp-tests.el (tramp--test-emacs31-p)
(tramp--split-on-boundary)
(tramp-test51-file-name-completion-boundaries): Add.

(cherry picked from commit d56d7ca35cf3c0185fe42bf7f64e3aa6a5281e3e)

diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el
index f738d2d65118afdffae0f891879c12c4e160db82..4856079ab4c734cbe9e1014691660d642effd5f2 100644 (file)
--- a/lisp/minibuffer.el
+++ b/lisp/minibuffer.el
@@ -3775,16 +3775,19 @@ boundaries for the original string."
   ;; and then transform that into an offset in STRING instead.  We can't do this
   ;; if we expand environment variables, so double the $s to prevent that.
   (let* ((doubled-string (replace-regexp-in-string "\\$" "$$" string t t))
-         ;; sifn will change $$ back into $, so the result is a suffix of STRING
-         ;; (in fact, it's the last absolute file name in STRING).
-         (last-file-name (substitute-in-file-name doubled-string))
-         (bounds (completion-boundaries last-file-name table pred suffix)))
-    (cl-assert (string-suffix-p last-file-name string) t)
-    ;; BOUNDS contains the start boundary in LAST-FILE-NAME; adjust it to be an
-    ;; offset in STRING instead.
-    (cons (+ (- (length string) (length last-file-name)) (car bounds))
-          ;; No special processing happens on SUFFIX and the end boundary.
-          (cdr bounds))))
+         ;; sifn will change $$ back into $, so SIFNED is mostly the
+         ;; same as STRING, with some text deleted.
+         (sifned (substitute-in-file-name doubled-string))
+         (bounds (completion-boundaries sifned table pred suffix))
+         (sifned-start (car bounds))
+         ;; Adjust SIFNED-START to be an offset in STRING instead of in SIFNED.
+         (string-start (+ (- sifned-start (length sifned)) (length string))))
+    ;; The text within the boundaries should be identical.
+    (cl-assert
+     (eq t (compare-strings sifned sifned-start nil string string-start nil))
+     t)
+    ;; No special processing happens on SUFFIX and the end boundary.
+    (cons string-start (cdr bounds))))
 
 (defun completion--file-name-table (orig pred action)
   "Internal subroutine for `read-file-name'.  Do not call this.

diff --git a/test/lisp/net/tramp-tests.el b/test/lisp/net/tramp-tests.el
index 4e8640d4303cb1f70fdbb7997060c742cb8ff268..56d696d507423673e1a837fbc1eac72f4bf0b2d5 100644 (file)
--- a/test/lisp/net/tramp-tests.el
+++ b/test/lisp/net/tramp-tests.el
@@ -8207,6 +8207,29 @@ Since it unloads Tramp, it shall be the last test to run."
   (should (featurep 'tramp))
   (should (featurep 'tramp-archive)))
 
+(defun tramp--test-emacs31-p ()
+  "Check for Emacs version >= 31.1."
+  (>= emacs-major-version 31))
+
+(defun tramp--split-on-boundary (s)
+  (let ((bounds (completion-boundaries s #'completion--file-name-table nil "")))
+    (cons (substring s nil (car bounds)) (substring s (car bounds)))))
+
+(ert-deftest tramp-test51-file-name-completion-boundaries ()
+  "Test file name completion boundaries are correct in various cases."
+  ;; `completion--file-name-table' was reworked in 31, the boundaries
+  ;; are always incorrect before that.
+  (skip-unless (tramp--test-emacs31-p))
+
+  (should (equal (tramp--split-on-boundary "/ssh:user@host:foo")
+                 '("/ssh:user@host:" . "foo")))
+  (should (equal (tramp--split-on-boundary "/ssh:user@host:/~/foo")
+                 '("/ssh:user@host:/~/" . "foo")))
+  (should (equal (tramp--split-on-boundary "/ssh:user@host:/usr//usr/foo")
+                 '("/ssh:user@host:/usr//usr/" . "foo")))
+  (should (equal (tramp--split-on-boundary "/ssh:user@host:/ssh:user@host://usr/foo")
+                 '("/ssh:user@host:/ssh:user@host://usr/" . "foo"))))
+
 (defun tramp-test-all (&optional interactive)
   "Run all tests for \\[tramp].
 If INTERACTIVE is non-nil, the tests are run interactively."