Subject: bug#60505: 29.0.60; Fido Mode and Tramp Completion

From: Michael Albinus <[email-masked]>
Date: Wed, 18 Jan 2023 13:30:38 +0100

Gregory Heytings <[email-masked]> writes:

> Hi Michael,

Hi Gregory,

>>> what do you think of that patch?  It would be regrettable to leave
>>> that bug unfixed in Emacs 29.
>>
>> Yes, it would be desirable to fix this. But it isn't the end of the
>> world if this doesn't happen, the problem is already evident in
>> Emacs 28, so we don't have a regression.
>>
>
> It's correct that the same problem is present in Emacs 28, but it is
> not in Emacs 27, so it's a regression.  As I tried to explain in
> bug#50387, the problem is that once Tramp is loaded, Tramp methods are
> returned when completions are requested for a root directory ("/").
> This problem is even worse since commit d5c6bf9625: prior to that
> commit only the 'scp' and 'scpx' methods were returned (in emacs -Q),
> now all methods are returned.  And this confuses the flex and
> substring completion mechanisms.

Yes. But this is not a Tramp fault. Completion styles like flex (and
substring, didn't test) do ignore Tramp file name syntax. They simply
think in terms of file name parts, separated by "/". This is not
appropriate for remote file names.

Tramp knows only file-name-completion and file-name-all-completions. This
is what it is called with; Tramp doesn't know anything about completion
styles. It would be an error to bring this into Tramp, based on these
two functions which are not designed to give the file name handler more
information about completion styles. This is our disagreement.

>>>> By the way, to limit the scope of the potential collateral
>>>> damages, it is also possible to use that condition only when
>>>> completion-styles contain 'substring' or 'flex'.
>>>>
>>>> What do you think of the attached patch?
>>
>> And what do we want to do if there are more completion-styles like
>> this? Extend Tramp then?
>
> Not extend Tramp, but add them, if necessary, to the list of
> completion-styles that are handled specially by Tramp at that
> place. Unless of course a better fix has been implemented in the
> meantime.

And this is exactly my fear: Tramp would depend on unrelated features,
and Tramp must follow. And you can't even guarantee that Tramp knows
all completion styles in question. Any user in the wild can specify her
own completion style.

> Well, that looks like a separate problem, with a non-default syntax,
> which (given the number of hits of "tramp-change-syntax" on Google or
> on Github) is apparently hardly ever used.

Then write a bug report, requesting to remove this Tramp syntax :-)

Seriously, this is the Tramp syntax used by XEmacs years ago. We've
promised the XEmacs converts to keep this syntax, in order to make their
life in GNU Emeacs less alienated.

> Nonetheless, I tried your recipe with the patch applied, and the good
> news is that with it, under Fido, Tramp behaves the same way it
> behaves under the default completions: the available methods are
> displayed after "/ [ s TAB", the same nonsense is displayed after "/ [
> ssh/ TAB", and the possible host names are displayed after typing a
> letter followed by TAB.

So yes, we shall at least hunt the bug displaying nonsense.

>> Instead, Tramp shall apply a patch which is not related there, which
>> adds further dependencies, which is not working for all use cases,
>> and which is good for making more trouble in the future. My opinion.
>
> I don't understand what you mean here, sorry.

Simply that it isn't Tramp's duty to fix the bug we're discussing. It
must be fixed by the flex (and possibly also substring) completion
styles.

I spent another couple of hours debugging completion in
minibuffer.el. The appended patch shows a possible fix for the flex
completion style. I don't claim this shall go into master; it is just a
rawhammer approach due to my limited knowledge about completion
styles. But it might give you an idea how a solution could look like.

Best regards, Michael.

diff --git a/lisp/minibuffer.el b/lisp/minibuffer.el
index 21d4607e7cf..a002143108f 100644
--- a/lisp/minibuffer.el
+++ b/lisp/minibuffer.el
@@ -3013,8 +3013,11 @@ completion-file-name-table
                                        (substring string 1)
                                        pred action))
        ((eq (car-safe action) 'boundaries)
-        (let ((start (length (file-name-directory string)))
-              (end (string-search "/" (cdr action))))
+        (let ((start (length
+                      (if (string-match-p "\\`/[-[:alnum:]]+:[^:]*\\'" string)
+                          string
+                        (file-name-directory string))))
+              (end (string-search "/\\:" (cdr action))))
           `(boundaries
             ;; if `string' is "C:" in w32, (file-name-directory string)
             ;; returns "C:/", so `start' is 3 rather than 2.
@@ -4205,6 +4208,9 @@ completion-flex--make-flex-pattern
 (defun completion-flex-try-completion (string table pred point)
   "Try to flex-complete STRING in TABLE given PRED and POINT."
   (unless (and completion-flex-nospace (string-search " " string))
+    (if (and (string-match-p "\\`/[-[:alnum:]]+:[^:]*\\'" string)
+             (eq table 'completion-file-name-table))
+        (completion-basic-try-completion string table pred point)
     (pcase-let ((`(,all ,pattern ,prefix ,suffix ,_carbounds)
                  (completion-substring--all-completions
                   string table pred point
@@ -4217,7 +4223,7 @@ completion-flex-try-completion
       ;; contain the substring "config".  FIXME: this still won't
       ;; augment "foo" to "froo" when matching "frodo" and
       ;; "farfromsober".
-      (completion-pcm--merge-try pattern all prefix suffix))))
+      (completion-pcm--merge-try pattern all prefix suffix)))))

 (defun completion-flex-all-completions (string table pred point)
   "Get flex-completions of STRING in TABLE, given PRED and POINT."