bug#60505: 29.0.60; Fido Mode and Tramp Completion

[Michael Albinus <michael.albinus@gmx.de>]

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

Stefan Monnier <monnier@iro.umontreal.ca> writes:

Hi Stefan,

>> I could imagine that the completion machinery offers an API that a
>> package could register its own idea of a file name syntax.
>
> The completion code relies on `file-name-directory`,
> `directory-file-name`, etc... for that.
>
> The problem as I see it goes as follows:
>
> According to `file-name-directory`, in `/ssh:myhost` the part
> `ssh:myhost` is an element of the `/` directory.  For this reason, the
> completion machinery would expect (file-name-all-completions "s" "/") to
> include "ssh:myhost" in its return list rather than only "ssh:".
>
> Now, it's impractical for Tramp to do that.  So the end result is the
> kind of bug reports we're discussing.
>
> One way to fix the problem is for Tramp to "teach" the rest of the
> system that `/ssh:` is a kind of directory, in which case the completion
> machinery would know that (file-name-all-completions "s" "/") returns
> "ssh:" and that "myhost" would be included only in the return value of
> (file-name-all-completions "" "/ssh:").

I've played with this idea, and the appended patch makes it work,
indeed. I had to adapt expand-file-name, file-exists-p,
file-name-directory and file-name-nondirectory, but the changes look
simple. It works for me now for the default completion styles (basic
partial-completion emacs22) as well as for fido-mode, with both the
default and simplified Tramp syntax.

It doesn't work (yet) for the separated Tramp syntax, but this isn't our
major problem, and could be fixed later. Could people check how it plays
in their environment?

> Another is to change Tramp's syntax so that it uses the regular "/"
> separator rather than ":".  This would get a similar result but without
> touching `file-name-directory` and friends.

No way. We have had syntax ambiguities for Tramp in the past, and I
don't want to go this painful way, again.

> We could also consider introducing a new set of (file-name) functions
>
>     completion-file-name-directory, completion-directory-file-name, ...
>
> so the completion code can use its own notion of how a file name gets
> separated into parts.

That's what I have done in the patch, adding such functions for the
tramp-completion-file-name-handler implementation. An implementation for
directory-file-name wasn't needed, but we could add if necessary.

> But introducing such a subtle distinction would likely introduce a lot
> of bugs&confusion as well.

Perhaps. So this patch isn't a candidate for Emacs 29.1. I would push it
to the master branch (when there is positive feedback), and I would also
add it to the tramp-2-6-stable branch in the Tramp git repo. By this,
the next Tramp 2.6 release on GNU ELPA would contain this change as
well, and we could get broader feedback.

What do people think?

>         Stefan

Best regards, Michael.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: Type: text/x-patch, Size: 4437 bytes --]

diff --git a/lisp/net/tramp.el b/lisp/net/tramp.el
index 21dbd40b1d2..0dc7d68983b 100644
--- a/lisp/net/tramp.el
+++ b/lisp/net/tramp.el
@@ -441,6 +441,8 @@ tramp-default-method-alist
 (defconst tramp-default-method-marker "-"
   "Marker for default method in remote file names.")

+(add-to-list 'tramp-methods `(,tramp-default-method-marker))
+
 (defcustom tramp-default-user nil
   "Default user to use for transferring files.
 It is nil by default; otherwise settings in configuration files like
@@ -1414,9 +1416,13 @@ tramp-password-prompt-not-unique
 during direct remote copying with scp.")

 (defconst tramp-completion-file-name-handler-alist
-  '((file-name-all-completions
+  '((expand-file-name . tramp-completion-handle-expand-file-name)
+    (file-exists-p . tramp-completion-handle-file-exists-p)
+    (file-name-all-completions
      . tramp-completion-handle-file-name-all-completions)
-    (file-name-completion . tramp-completion-handle-file-name-completion))
+    (file-name-completion . tramp-completion-handle-file-name-completion)
+    (file-name-directory . tramp-completion-handle-file-name-directory)
+    (file-name-nondirectory . tramp-completion-handle-file-name-nondirectory))
   "Alist of completion handler functions.
 Used for file names matching `tramp-completion-file-name-regexp'.
 Operations not mentioned here will be handled by Tramp's file
@@ -1707,7 +1713,6 @@ tramp-dissect-file-name
 		     :port port :localname localname :hop hop))
 	  ;; The method must be known.
 	  (unless (or nodefault non-essential
-		      (string-equal method tramp-default-method-marker)
 		      (assoc method tramp-methods))
 	    (tramp-user-error
 	     v "Method `%s' is not known." method))
@@ -2941,6 +2946,30 @@ tramp-connectable-p
         (and vec (process-live-p (get-process (tramp-buffer-name vec))))
 	(not non-essential))))

+(defun tramp-completion-handle-expand-file-name (filename &optional directory)
+  "Like `expand-file-name' for partial Tramp files."
+  (if (file-name-absolute-p filename)
+      filename
+    (concat (or directory default-directory "/") filename)))
+
+(defun tramp-completion-handle-file-exists-p (filename)
+  "Like `file-exists-p' for partial Tramp files."
+  ;; We need special handling only when a method is needed.  Then we
+  ;; regard all files "/method:" or "/[method/" as existent, if
+  ;; "method" is a valid Tramp method.
+  (or (and (not (string-empty-p tramp-method-regexp))
+           (string-match
+	    (rx
+	     (regexp tramp-prefix-regexp)
+	     (group (regexp tramp-method-regexp))
+	     (regexp tramp-postfix-method-regexp))
+            filename)
+           ;; Is it a valid method?
+           (member
+            filename
+            (tramp-get-completion-methods (match-string 1 filename))))
+      (tramp-run-real-handler #'file-exists-p (list filename))))
+
 ;; Method, host name and user name completion.
 ;; `tramp-completion-dissect-file-name' returns a list of
 ;; `tramp-file-name' structures.  For all of them we return possible
@@ -3176,6 +3205,27 @@ tramp-get-completion-user-host
   (unless (zerop (+ (length user) (length host)))
     (tramp-completion-make-tramp-file-name method user host nil)))

+(defun tramp-completion-handle-file-name-directory (filename)
+  "Like `file-name-directory' for partial Tramp files."
+  ;; We need special handling only when a method is needed.  Then we
+  ;; return "/method:" or "/[method/", if "method" is a valid Tramp
+  ;; method.
+  (if (and (not (string-empty-p tramp-method-regexp))
+           (string-match
+            (rx (group
+	         (regexp tramp-prefix-regexp)
+                 (group (regexp tramp-method-regexp))
+	         (regexp tramp-postfix-method-regexp)))
+            filename)
+           ;; Is it a valid method?
+           (assoc (match-string 2 filename) tramp-methods))
+      (match-string 1 filename)
+    (tramp-run-real-handler #'file-name-directory (list filename))))
+
+(defun tramp-completion-handle-file-name-nondirectory (filename)
+  "Like `file-name-nondirectory' for partial Tramp files."
+  (tramp-compat-string-replace (file-name-directory filename) "" filename))
+
 (defun tramp-parse-default-user-host (method)
   "Return a list of (user host) tuples allowed to access for METHOD.
 This function is added always in `tramp-get-completion-function'