Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

How to get completion category? #94

Closed
okamsn opened this issue May 18, 2020 · 15 comments
Closed

How to get completion category? #94

okamsn opened this issue May 18, 2020 · 15 comments

Comments

@okamsn
Copy link
Contributor

okamsn commented May 18, 2020

Hello,

User @oantolin has created the packages embark, which allows using commands similar to Ivy or Helm actions. Different commands are made available, depending on what is being completed (file, buffer, etc.).

To do this, they use the following expression to detect what kind of completion is happening:

(completion-metadata-get
  (completion-metadata (minibuffer-contents)
                       minibuffer-completion-table
                       minibuffer-completion-predicate)
   'category)

In Selectrum, this returns that buffer names are being completed, even in commands like find-file or amx. I do not have this problem with normal Emacs completion.

Why does this happen, and is there a way to get the actual completion category?

Thank you.

@clemera
Copy link
Collaborator

clemera commented May 19, 2020

@raxod502 We could allow passing any completing-read collection to selectrum-read and then bind those variables there? Any needed normalization could then be moved into selectrum-read. There are many packages and commands which will assume those variables are bound and we need the metadata queried from the completion table internally for other purposes, too as discussed in #15 and #82.

@clemera
Copy link
Collaborator

clemera commented May 20, 2020

After thinking about it the above wouldn't work because functions like selectrum-read-file-name need to pass the dynamic candidate function as candidates and would additionally need a way to pass the original completion table.

Let-binding minibuffer-completion-table and minibuffer-completion-predicate around selectrum-read calls from functions which integrate with the internal API like read-file-name-function, read-buffer-function and completing-read-function would be better. But in this case minibuffer-completion-table would be bound for all recursive minibuffers which use selectrum-read, too.

With the above considerations I think we should simply allow passing the variables as args to selectrum-read. I have opened a PR for that, see #95.

@raxod502
Copy link
Member

What's wrong with just let-binding those variables? If you did something recursively, wouldn't they be let-bound recursively to the correct values?

@clemera
Copy link
Collaborator

clemera commented May 25, 2020

If you did something recursively, wouldn't they be let-bound recursively to the correct values?

Only for the cases where they are let bound. This can be done for the selectrum functions which integrate with the completion API but not for custom commands written by users or for example selectrum-read-library-name. One could let bind them back to nil but that would have to be done every time one uses selectrum-read directly.

@raxod502
Copy link
Member

Okay... so I guess then your PR seems fine, but it should have logic in selectrum-read to translate that keyword argument into a let-binding, right?

@clemera
Copy link
Collaborator

clemera commented May 25, 2020

Aren't minibuffer-completion-table and minibuffer-completion-predicate already correctly bound for the function body of selectrum-read when passed like that?

@raxod502
Copy link
Member

Using a named variable as a function argument creates a lexical binding, not a dynamic one (thank goodness). You need to invoke let explicitly to create dynamic bindings.

@clemera
Copy link
Collaborator

clemera commented May 25, 2020

Yes but I believe it should be a lexical one sorry if I expressed that confusingly.

@clemera
Copy link
Collaborator

clemera commented May 25, 2020

Wait a moment I have to check something.

@clemera
Copy link
Collaborator

clemera commented May 25, 2020

Sorry I was confused they should be dynamically bound but I think they are because minibuffer-completion-table and minibuffer-completion-predicate are special vars.

@clemera
Copy link
Collaborator

clemera commented May 25, 2020

;;; selectrum.el --- Easily select item from list -*- lexical-binding: t -*-


(defvar special-var nil)

(cl-defun check (&key special-var)
  (check2))

(defun check2 ()
  (message "is see %s" special-var))

(check :special-var "this")

@raxod502
Copy link
Member

Actually, I did that test too. But I did it without &key, silly me. I guess cl-defun turns &key into a let, whereas regular defun doesn't do any of that. So, what you have works. I'll take another look.

@clemera
Copy link
Collaborator

clemera commented May 25, 2020

Interesting I did not thought about that first and just assumed special vars are always dynamically bound even when passed as function arguments. I agree that it's better that they are not for regular defun. Ideally that would be the case for cl-defun keyargs, too then.

@okamsn
Copy link
Contributor Author

okamsn commented Jul 13, 2020

This seems to be fixed now, so I'm closing this issue.

@okamsn okamsn closed this as completed Jul 13, 2020
@clemera
Copy link
Collaborator

clemera commented Jul 13, 2020

I will also soon add instructions to the wiki how to setup embark for selectrum.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants