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

[completion] Fix custom completion style and make it default for fuzzy matching #3746

Merged
merged 1 commit into from
Oct 20, 2024

Conversation

alexander-yakushev
Copy link
Member

@alexander-yakushev alexander-yakushev commented Oct 9, 2024

Back to completion woes.

Some time ago we moved the official way to enable fuzzy Compliment-driven completion from a custom completion style to flex. This worked correctly until company-mode/company-mode#1493 – cider completion now no longer suggests candidates for class shortnames like e.g. Persistent. Nothing has been broken in Company, vice versa, Company has became more like corfu and complete-at-point, so it was already broken in those clients.

I tried to fix it for flex but did not succeed. It looks like owning a custom completion style is still the proper way to go, so I'm resurrecting it with this PR and undeprecating the previous way to enable "fuzzy" completion. The current partial compatibility with flex remains, so if folks prefer to use that, they still can.

I've also fixed our custom completion style so that it is compatible with Emacs machinery – something that users complained about before.

@alexander-yakushev alexander-yakushev force-pushed the undeprecate-custom-completion-style branch 4 times, most recently from 83b3175 to ed80b77 Compare October 12, 2024 07:02
@alexander-yakushev alexander-yakushev force-pushed the undeprecate-custom-completion-style branch from ed80b77 to a70c209 Compare October 12, 2024 07:07
@bbatsov
Copy link
Member

bbatsov commented Oct 19, 2024

Seems we constantly manage to both fix something and break something on the completion side of things. Let me think about this for a bit.

@alexander-yakushev
Copy link
Member Author

alexander-yakushev commented Oct 19, 2024

Sure. But what I understood from @dgutov words is that a custom completion style is an OK solution for many packages, not something to shun from.

@bbatsov
Copy link
Member

bbatsov commented Oct 19, 2024

I'm mostly worried about this part:

Please go ahead and ping me if you have further questions or problems adapting the style implementation. This is still a breaking change in company-mode's behavior, and I'd like to see all major completion providers onboard before tagging the next release.

I'm not sure if this will work fine with the current stable version of company, so I wanted to investigate. But if you're sure about this I guess we can proceed.

@alexander-yakushev
Copy link
Member Author

It should be fine. The changes in Company introduced problems for the current flex-based solution but didn't touch the old custom completion-style solution. Besides, it's opt-in anyway, so it wouldn't impact users that don't have it enabled in the first place.


The CIDER completion at point function supports most completion styles,
including `partial-completion`, `orderless` and `flex`. It also supports a
custom completion style that is confusingly named `cider` too. Activating it
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps we should name it fuzzy-cider, cider-fuzzy, cider-flex or something like this, so it's less confusing?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there is value in retaining the current name for compatibility reasons. Besides, I always had a dare to make it a default someday, so I'm not sure if emphasizing fuzzy would be necessary. It's just the way of completion that CIDER provides and that's it. But in the end, I don't have a strong opinion about the name of the completion style.

@dgutov
Copy link
Contributor

dgutov commented Oct 19, 2024

@bbatsov

I'm not sure if this will work fine with the current stable version of company, so I wanted to investigate.

The new release came out a month ago, so this won't be a problem. I indeed hoped to get more backends onboard first, but that only had partial success. OT2H, the problem had been present with all (?) other UIs except company-mode, so it can't be too bad.

@alexander-yakushev

a custom completion style is an OK solution for many packages, not something to shun from

I agree this is more complicated than we'd want to, but for now it's what's recommended for CAPF. Also see eglot's and lsp-mode's styles.

@bbatsov bbatsov merged commit ced9f25 into master Oct 20, 2024
39 checks passed
@bbatsov bbatsov deleted the undeprecate-custom-completion-style branch October 20, 2024 06:06
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.

3 participants