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

# Extend the partial accept handler #241356

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

marrej
Copy link
Contributor

@marrej marrej commented Feb 20, 2025

The main problem with the current partial acceptance is that it provides the full length of the original insertion, rather than just the added content to the editor. Having access to the addedLength would allow completion providers to truthfully report the change in characters.

The versionBeforeAcceptance can be used by the extensions to truthfully report the state sequence and changes

ReplacementRange allows to better track the original intended edit at the version of acceptance.

closes #195369

@isidorn isidorn assigned hediet and unassigned isidorn Feb 20, 2025
@hediet
Copy link
Member

hediet commented Feb 24, 2025

Thanks for the PR! I believe the properties shouldn't be optional though. If data is provided, it should always be provided, otherwise every consumer will wonder about the cases in which they are not available.

@marrej
Copy link
Contributor Author

marrej commented Feb 24, 2025

Fair point. Let me modify it so the data can be required. Thanks!

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.

InlineCompletion PartialAccept additional info
3 participants