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

counsel-company now automatically selects its candidate if there is only one choice #2849

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

NightMachinery
Copy link

No description provided.

Copy link
Collaborator

@basil-conto basil-conto left a 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)
Copy link
Collaborator

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.

Comment on lines +385 to +388
)
(t (ivy-read "Candidate: " company-candidates
:action #'ivy-completion-in-region-action
:caller 'counsel-company))))))
Copy link
Collaborator

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

@NightMachinery
Copy link
Author

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

@basil-conto
Copy link
Collaborator

PS: I don't like =if= as it is asymmetric and needs =progn= for its direct clause.

Not in this case ;).

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

Successfully merging this pull request may close these issues.

2 participants