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

Sources v2 #465

Merged
merged 15 commits into from
Dec 10, 2024
Merged

Sources v2 #465

merged 15 commits into from
Dec 10, 2024

Conversation

Saghen
Copy link
Owner

@Saghen Saghen commented Dec 6, 2024

Notes

  • Enabled providers should be reworked, ideally with a simple way for per-filetype sources
  • How to handle sources returning multiple responses?
    • Do we immediately show on the first response? Would this cause weird behavior with the LSP source when there's multiple clients giving completions?
    • How do we handle async sources? (i.e. copilot)
  • Is source defined keyword regex necessary? Can they instead use enabled since it now runs before each call to get_completions? CC @stefanboca

@Saghen Saghen marked this pull request as draft December 6, 2024 21:37
@stefanboca
Copy link
Collaborator

stefanboca commented Dec 6, 2024

With regards to keyword pattern as they're used in nvim-cmp, my understanding is that sources are not disabled if a match is not found, but rather that matching the keyword pattern is used as another trigger in addition to trigger characters. So to replicate nvim-cmp's mechanism, using enabled is not enough.

Also, I believe nvim-cmp uses lua patterns, not regex.

@stefanboca
Copy link
Collaborator

I also think that nvim-cmp uses the keyword pattern to define the completion context if a match is found, for reference.

@Saghen
Copy link
Owner Author

Saghen commented Dec 9, 2024

Ahh super insightful, thanks

matching the keyword pattern is used as another trigger in addition to trigger characters

I'm thinking we could do something like should_trigger to cover this case

I also think that nvim-cmp uses the keyword pattern to define the completion context if a match is found

And we could ignore this, since sources could mark themselves as incomplete if the default context isn't fine-grained enough for them. We could also expose a helper for getting the match before the cursor for those that need this

@Saghen
Copy link
Owner Author

Saghen commented Dec 10, 2024

Skipping #313 for now, I'm thinking the existing APIs could be enough. Maybe I'd need to see a specific example where it's needed

@Saghen Saghen marked this pull request as ready for review December 10, 2024 21:02
@Saghen Saghen merged commit 7ff28da into main Dec 10, 2024
2 checks passed
@Saghen Saghen deleted the sources-v2 branch December 10, 2024 21:37
Saghen added a commit that referenced this pull request Dec 10, 2024
Large rewrite of how sources are handled, adding support for async providers/timeouts, tree based fallbacks, dynamically adding sources and some other goodies

Closes #386
Closes #219
Closes #328
Closes #331
Closes #312
Closes #454
Closes #444
Closes #372
Closes #475
Saghen added a commit that referenced this pull request Dec 11, 2024
Large rewrite of how sources are handled, adding support for async providers/timeouts, tree based fallbacks, dynamically adding sources and some other goodies

Closes #386
Closes #219
Closes #328
Closes #331
Closes #312
Closes #454
Closes #444
Closes #372
Closes #475
soifou added a commit to soifou/blink.cmp that referenced this pull request Dec 11, 2024
Regression introduced in recent refactoring (Saghen#465) where the path
provider was not returning any results.

Ensures that all successfully completed stat operations are returned.
Saghen pushed a commit that referenced this pull request Dec 11, 2024
Regression introduced in recent refactoring (#465) where the path
provider was not returning any results.

Ensures that all successfully completed stat operations are returned.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment