Giter Site home page Giter Site logo

Comments (23)

minad avatar minad commented on August 20, 2024 1

@mavit It seems Michael Albinus already fixed the problem in Tramp, see emacs-mirror/emacs@7b0e07c.

from orderless.

oantolin avatar oantolin commented on August 20, 2024

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.

from orderless.

oantolin avatar oantolin commented on August 20, 2024

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

from orderless.

minad avatar minad commented on August 20, 2024

from orderless.

oantolin avatar oantolin commented on August 20, 2024

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))

from orderless.

oantolin avatar oantolin commented on August 20, 2024

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

from orderless.

mavit avatar mavit commented on August 20, 2024

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))

from orderless.

minad avatar minad commented on August 20, 2024

The issue is probably also present for

(setq completion-styles '(substring))

from orderless.

oantolin avatar oantolin commented on August 20, 2024

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

from orderless.

mavit avatar mavit commented on August 20, 2024

The issue does not occur with substring.

from orderless.

oantolin avatar oantolin commented on August 20, 2024

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).

from orderless.

minad avatar minad commented on August 20, 2024

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."))

from orderless.

minad avatar minad commented on August 20, 2024

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.

from orderless.

mavit avatar mavit commented on August 20, 2024

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

from orderless.

oantolin avatar oantolin commented on August 20, 2024

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?

from orderless.

oantolin avatar oantolin commented on August 20, 2024

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.

from orderless.

oantolin avatar oantolin commented on August 20, 2024

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?

from orderless.

minad avatar minad commented on August 20, 2024

from orderless.

minad avatar minad commented on August 20, 2024

from orderless.

oantolin avatar oantolin commented on August 20, 2024

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))))

from orderless.

minad avatar minad commented on August 20, 2024

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))))

from orderless.

oantolin avatar oantolin commented on August 20, 2024

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).

from orderless.

minad avatar minad commented on August 20, 2024

(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. 🤷

from orderless.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.