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

InlineCompletion PartialAccept additional info #195369

Open
marrej opened this issue Oct 11, 2023 · 5 comments · May be fixed by #241356
Open

InlineCompletion PartialAccept additional info #195369

marrej opened this issue Oct 11, 2023 · 5 comments · May be fixed by #241356
Assignees
Labels
feature-request Request for new features or functionality inline-completions
Milestone

Comments

@marrej
Copy link
Contributor

marrej commented Oct 11, 2023

TLDR; Would it be possible to extend the handlePartialAccept with metadata field, allowing to attach additional info, such as origin?

It would help understanding in the downstream completion metrics, where acceptances are comming from to better understand user behavior. The field could be later on extended with any additional information if neccessary, e.g. in the case of partial accept it might be helpful to report back also the length of the accepted token (not only the length of the full acceptance from start of insertion to cursor position ).

@hediet hediet added feature-request Request for new features or functionality inline-completions labels Dec 4, 2023
@hediet hediet added this to the Backlog milestone Dec 4, 2023
@marrej
Copy link
Contributor Author

marrej commented Dec 6, 2023

Hi @hediet is that something that you plan to do in the comming weeks? Or should I grab it and try to propose a change via PR?

@hediet
Copy link
Member

hediet commented Dec 6, 2023

Feel free to provide a PR, but I cannot guarantee you that it will get merged.
A PR might make it more likely that this issue gets resolved soon though.

@marrej
Copy link
Contributor Author

marrej commented Dec 6, 2023

That sounds good to me, thanks!
Do you preffer I add the change in to new proposed API, or extend the current ?

@hediet
Copy link
Member

hediet commented Dec 7, 2023

Please extend the current!

@marrej
Copy link
Contributor Author

marrej commented Jan 17, 2024

Going through what I think would be interesting to have in the additional metadata I settled on this:

  • replacementRange
  • partialAcceptanceKind
  • documentVersionBeforeAcceptance
  • text [very optional as it can be directly extracted using the replacement range + added chars]

The reason for that is, to be able to get a better visibility on what happened in the core without having to hack in addtional listeners for typing/cursor changes, directly from the partialAccepts. The data mentioned above would allow to easily reconstruct all edits that happened in a document, and allow for better post processing of the data.

I decided to split this in to 2 PRs:

  • 1st - proposing the structural changes going through the whole stack (proposed API - extHost - mainThread - Core) and adding the base types
  • 2nd - adding the other properties & convertors (which should be a smaller PR)

The 1st is being sent out right now, while the 2nd will be sent out after discussion, as there may be some points that we might want to clarify further.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request Request for new features or functionality inline-completions
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants