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.

Sign up for GitHub

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

Jump to bottom

Fix for issue #2805 by using company-finish on counsel-company. #2835

Closed
wants to merge 4 commits into from
Closed

Fix for issue #2805 by using company-finish on counsel-company. #2835

wants to merge 4 commits into from

Conversation

joefbsjr
Copy link
Contributor

Simple change that makes counsel-company work more seamlessly with company-complete. By using company-finish instead of ivy-completion-in-region-action many behaviors expected from company, like 'keep-prefix' option and post-complete hooks now work.

This change needs that company-abort be called only if ivy-exit is nil when unwinding, otherwise company-finish does not work.

@joefbsjr joefbsjr closed this Mar 28, 2021
@joefbsjr joefbsjr deleted the branch abo-abo:master March 28, 2021 15:10
@joefbsjr joefbsjr deleted the master branch March 28, 2021 15:10
@joefbsjr joefbsjr restored the master branch March 28, 2021 15:21
@joefbsjr joefbsjr deleted the master branch March 28, 2021 15:22
@joefbsjr joefbsjr restored the master branch March 28, 2021 15:24
@joefbsjr joefbsjr reopened this Mar 28, 2021
@joefbsjr
Fix company-prefix and company-common when using counsel-company.
ceda444 
The company-common is always downcase, even when the prefix might not
be. This fix allows counsel-company to use company-common even when
the prefix has uppercase letters.

It also sets company-prefix to be equal to company-common, without
this, company-finish does not know that the prefix was expanded, and
does not correctly removes the prefix from the candidate, generating
duplicated characters.
@usaoc
Copy link

usaoc commented Mar 30, 2021

Hi, a few comments here if you don't mind.

  1. I don't think we still need the variable len if we are not bothering with ivy-completion-in-region-action. We also don't need to set ivy-completion-{beg,end}.

  2. We could wrap the ivy-read part with (when company-candidates ...) so that it does nothing when there is no candidate, replicating the behavior of company-complete.

  3. I suspect that the workaround is not needed. The problem is that we are dealing with outdated company-candidates because company-complete-common inserted the common part without updating the variables. In fact, I think we could just call company-abort and then company-complete before calling ivy-read so that the variables are properly set. It also solves the problem that the text properties of the candidates are incorrect (the completed common part is not properly propertized).

@NightMachinery
Copy link

I am also touching this, and I think my changes could be integrated into this PR:

#2849

@NightMachinery NightMachinery mentioned this pull request Apr 15, 2021
Open
@joefbsjr joefbsjr closed this by deleting the head repository Oct 13, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@joefbsjr @usaoc @NightMachinery