-
-
Notifications
You must be signed in to change notification settings - Fork 338
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
counsel-company now automatically selects its candidate if there is only one choice #2849
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks.
counsel-company
now automatically selects its candidate if there is only one choice
Is that unconditional, or is there a user option that controls whether this happens?
:action #'ivy-completion-in-region-action | ||
:caller 'counsel-company)))) | ||
(cond | ||
((= (length company-candidates) 1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: If company-candidates
is a list, it's better to say:
(and company-candidates
(null (cdr company-candidates)))
to avoid traversing the entire list. In Emacs 28, you can alternatively say:
(length= company-candidates 1)
Not sure if it's worth writing a compatibility shim for this.
) | ||
(t (ivy-read "Candidate: " company-candidates | ||
:action #'ivy-completion-in-region-action | ||
:caller 'counsel-company)))))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: there's a trailing paren and the indentation seems off. Suggest to simplify:
(if (cdr company-candidates)
(ivy-read "Candidate: " company-candidates
:action #'ivy-completion-in-region-action
:caller 'counsel-company)
(ivy-completion-in-region-action (car company-candidates)))
@basil-conto Thanks. There is no user-configurable option, but it's addable. I think my PR is stale if #2835 is to be merged, so they should keep your points in mind if they want to also add this feature. PS: I don't like =if= as it is asymmetric and needs =progn= for its direct clause. |
Not in this case ;). |
No description provided.