Orderless matches all TRAMP methods

[mavit]

I type:

C-x C-f C-a C-k /ssh: C-a C-k /zzz TAB

I see:

Click on a completion to select it.
In this buffer, type RET to select the completion near point.

45 possible completions:
-:             adb:           afp:           dav:           davs:
doas:          docker:        fcp:           ftp:           gdrive:
krlogin:       ksu:           kubernetes:    mtp:           nc:
nextcloud:     plink-ksh:     plink:         plinkx:        podman:
pscp:          psftp:         rclone:        rcp:           remcp:
remsh:         rsh:           rsync:         scp-ksh:       scp:
scpx:          sftp:          sg:            smb:           ssh-ksh:
ssh:           sshfs:         sshx:          su:            sudo-csw-ksh:
sudo-csw:      sudo-ksh:      sudo:          sudoedit:      telnet:

It’s not clear to me why any of these match /zzz. I'm not sure if this is a bug or some misunderstanding on my part.

[oantolin]

It does seem like a bug but not in orderless. Try this in emacs -Q without even loading orderless

(require 'tramp)

(let ((completion-regexp-list '("zzz")))
  (all-completions "/" #'read-file-name-internal nil 1))

According to the docstring of all-completions, completion tables which are functions (like read-file-name-internal) are supposed to handle completion-regexp-list themselves, returning only completions that match all of those regexps.

[oantolin]

It seems to be specifically completion-file-name-table that should handle completion-regexp-list and fails to do so.

[minad]

On 11/2/23 01:02, Omar Antolín Camarena wrote:

It seems to be specifically |completion-file-name-table| that should handle |completion-regexp-list| and fails to do so.

Isn't this the same or a similar problem as described in https://github.com/minad/vertico#tramp-hostname-and-username-completion? The completion table should be fixed for Tramp in the upcoming Emacs 30 (or maybe a newer Tramp version on ELPA). If not, this problem should be reported via M-x report-emacs-bug.

[oantolin]

It does sound like it could be the same problem. Do you have an easy way to test this updated tramp, @minad? If so, maybe you could run this code:

(require 'tramp)

(let ((completion-regexp-list '("zzz")))
  (all-completions "/" #'read-file-name-internal nil 1))

[oantolin]

At any rate, since I can reliably reproduce this issue without Orderless, I'm closing this issue.

[mavit]

It does seem like a bug but not in orderless. Try this in emacs -Q without even loading orderless

(require 'tramp)

(let ((completion-regexp-list '("zzz")))
  (all-completions "/" #'read-file-name-internal nil 1))

In Emacs 29.1.90, the above returns nil, but I still experience the bug if I:

(package-initialize)
(require 'orderless)
(setq completion-styles '(orderless basic))

The issue goes away again if I then:

(setq completion-styles '(basic))

[minad]

The issue is probably also present for

(setq completion-styles '(substring))

[oantolin]

Could you please test with substring instead of orderless on that build of Emacs, @mavit?

[mavit]

The issue does not occur with substring.

[oantolin]

Interesting! That seems very odd, I can't imagine what changed that makes substring work but not orderless. This one will be hard for me to debug since I'd have to install one of these more recent Emacs builds (I'm using 29.1).

[minad]

I've looked into this. The problem is that the the Tramp completion table does not perform proper filtering. The completion style substring performs (inefficient) double filtering, such that the problem is hidden. The consequence of this is also that substring is two timers slower than Orderless. The problem can be shown using the following simple substring completion style.

(setq completion-styles '(test)
      completion-category-defaults nil
      completion-category-overrides nil)

(defun test-all-completions (string table pred point)
  (let* ((limit (car (completion-boundaries string table pred "")))
     (completion-regexp-list (list (regexp-quote (substring string limit)))))
    (all-completions (substring string 0 limit) table pred)))

(defun test-try-completion (string table pred point)
  (pcase (length (test-all-completions string table pred point))
    (1 t)
    (0 nil)
    (_ (cons string (length string)))))

(add-to-list 'completion-styles-alist
             '(test
               test-try-completion
               test-all-completions
               "Test completion style."))

[minad]

For background, the function try-completion and all-completion doc strings specify that function completion tables must handle the completion-regexp-list, see https://www.gnu.org/software/emacs/manual/html_node/elisp/Basic-Completion.html.

To be acceptable, a possible completion must also match all the regexps
in completion-regexp-list (unless COLLECTION is a function, in
which case that function should itself handle completion-regexp-list).

...

This function returns a list of all possible completions of string. 
The arguments to this function are the same as those of try-completion,
and it uses completion-regexp-list in the same way that try-completion does.

[mavit]

Thanks for your help. I have reported this as a TRAMP bug: https://lists.gnu.org/archive/html/tramp-devel/2023-11/msg00000.html

[oantolin]

So, I'm a little confused. Here on Emacs 29.1 the bug is present with all completion styles:

(require 'tramp)

(cl-loop for style in '(basic substring partial-completion flex orderless)
         collect
         (let* ((completion-styles (list style))
                (completion-regexp-list '("zzz"))
                (all (completion-all-completions
                      "/" #'read-file-name-internal nil 1)))
           (setcdr (last all) nil)
           (cons style (length all))))

If you run that all styles return the same 38 matches. So are you saying @mavit that 29.1.90 added the redundant double filtering to all the builtin styles (making all of them twice as slow, probably) instead of fixing the TRAMP completion table?

[oantolin]

That is so weird. Even if the TRAMP table is fixed, I'd still consider the double-filtering a performance bug that should be fixed as well.

[oantolin]

Oh, I think there is something else I don't quite understand: if in emacs -Q I do (require 'tramp) and then (setq completion-styles '(flex)) and type C-x C-f /zzz TAB I don't actually get the matches, even on 29.1. Who is removing the spurious matches there? The default completion UI?

[minad]

On 11/16/23 14:10, Omar Antolín Camarena wrote:

That is so weird. Even if the TRAMP table is fixed, I'd still consider the double-filtering a performance bug that should be fixed as well.

I think the double filtering is actually a side effect of the highlighting (without the new completion-lazy-hilit=t). Anyway, the Tramp completion table should be fixed such that it respects completion-regexp-list. This is trivial to do. All that is needed is to use complete-with-action in the completion table function.

[minad]

On 11/16/23 14:10, Omar Antolín Camarena wrote:

So, I'm a little confused. Here on Emacs 29.1 the bug is present with all completion styles:

(require 'tramp)

(cl-loop for style in '(basic substring partial-completion flex orderless) collect (let ((completion-styles (list style)) (completion-regexp-list '("zzz"))) (cons style (length (all-completions "/" #'read-file-name-internal nil 1)))))

Your mistake here is that you use all-completions (querying the table directly) instead of completion-all-completions I think.

[oantolin]

Your mistake here is that you use all-completions (querying the table directly) instead of completion-all-completions I think.

Yes, that certainly was a mistake, but I updated the code as soon as I noticed, with the same results. Here it is for reference:

(require 'tramp)

(cl-loop for style in '(basic substring partial-completion flex orderless)
         collect
         (let* ((completion-styles (list style))
                (completion-regexp-list '("zzz"))
                (all (completion-all-completions
                      "/" #'read-file-name-internal nil 1)))
           (setcdr (last all) nil)
           (cons style (length all))))

[minad]

On my Emacs 29.1.90 (compiled a few days ago) I get the following result:

((basic . 0) (substring . 0) (partial-completion . 0) (flex . 0) (orderless . 39))

My test code (slightly modified from yours from above):

(package-initialize)
(require 'orderless)
(require 'tramp)

(cl-loop for style in '(basic substring partial-completion flex orderless)
         collect
         (let* ((completion-styles (list style))
        (minibuffer-completing-file-name t) ;; !!!
                (all (completion-all-completions
                      "/zzz" #'read-file-name-internal nil 4)))
       (when all (setcdr (last all) nil))
           (cons style (length all))))

[oantolin]

Huh, with that code, I get the same answer as you but on Emacs 29.1. (Well, almost the same: there seem to be only 38 TRAMP methods here, not 39; but I do get 0 for all the other styles). And I get the same answer whether or not I have (minibuffer-completing-file-name t).

[minad]

(Well, almost the same: there seem to be only 38 TRAMP methods here, not 39; but I do get 0 for all the other styles).

Iirc new Tramp methods have been added (containers like docker).

And I get the same answer whether or not I have (minibuffer-completing-file-name t).

Makes a difference for me. So Tramp seems to check this variable (which is a bad idea, better use the completion category). Without that I get 0 even for Orderless. :shrug:

[minad]

@mavit It seems Michael Albinus already fixed the problem in Tramp, see https://github.com/emacs-mirror/emacs/commit/7b0e07c41ae92d4cb139b1c47ce9debc37cfffcb.