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

# Add partial accept kind to inline completion handle #202668

Merged
merged 28 commits into from
Mar 4, 2024

Conversation

marrej
Copy link
Contributor

@marrej marrej commented Jan 17, 2024

1st part of the partial accept handler enrichment

This first proposal enables the metadata to be attached in the core to the partialAcceptHandler to return the Kind of partial acceptance that was triggered.

This allows for deeper understanding of user interactions, as well as filtering out specific triggers when doing post analysis (e.g. Suggest).

#195369

@marrej marrej marked this pull request as draft January 17, 2024 14:34
@marrej marrej marked this pull request as ready for review January 17, 2024 14:47
@marrej marrej marked this pull request as draft January 17, 2024 15:01
@hediet
Copy link
Member

hediet commented Jan 17, 2024

[eslint ] [16:52:03] /mnt/vss/_work/1/s/src/vs/workbench/api/common/extHostTypeConverters.ts: line 2406, col 4, Warning - Missing semicolon. (@typescript-eslint/semi)

@marrej marrej marked this pull request as ready for review January 17, 2024 19:20
@marrej
Copy link
Contributor Author

marrej commented Jan 17, 2024

[eslint ] [16:52:03] /mnt/vss/_work/1/s/src/vs/workbench/api/common/extHostTypeConverters.ts: line 2406, col 4, Warning - Missing semicolon. (@typescript-eslint/semi)

Sorry for the early ping, I tried to keep the PR in draftmode but you already got assigned. The issues should be resolved now :)

@marrej
Copy link
Contributor Author

marrej commented Jan 19, 2024

Reworked the API (original proposed changes), to allow easy onboarding of new users (wouldn't cause breakage when released with extensions).

@marrej
Copy link
Contributor Author

marrej commented Jan 29, 2024

Hey @bhavyaus , Is there any additional changes that you would like me to add to the PR?

Also if I may be so bold to ask, why did you add the PartialAcceptInfo directly to the proposed API. Is it because the proposed APIs can break current extension implementations and are experimental only?

@bhavyaus
Copy link
Contributor

bhavyaus commented Jan 29, 2024

Hey @bhavyaus , Is there any additional changes that you would like me to add to the PR?

Apologies. I was performing endgame testing. Ignore my previous comment.

Hey @bhavyaus , Is there any additional changes that you would like me to add to the PR?

Also if I may be so bold to ask, why did you add the PartialAcceptInfo directly to the proposed API. Is it because the proposed APIs can break current extension implementations and are experimental only?

@justschen , could you please help take a look.

@hediet hediet added this to the February 2024 milestone Jan 31, 2024
@hediet hediet modified the milestones: February 2024, March 2024 Feb 20, 2024
@hediet hediet enabled auto-merge (squash) February 28, 2024 10:39
# Conflicts:
#	src/vs/editor/contrib/inlineCompletions/browser/inlineCompletionsModel.ts
#	src/vs/editor/standalone/browser/standaloneLanguages.ts
#	src/vs/workbench/api/common/extHost.api.impl.ts
#	src/vs/workbench/api/common/extHostTypeConverters.ts
hediet
hediet previously approved these changes Feb 28, 2024
aeschli
aeschli previously approved these changes Feb 28, 2024
@hediet hediet dismissed stale reviews from aeschli and themself via 88f3f2e March 4, 2024 15:00
@hediet hediet merged commit 0bfc1db into microsoft:main Mar 4, 2024
6 checks passed
yiliang114 pushed a commit to yiliang114/vscode that referenced this pull request Mar 6, 2024
* # Add partial accept kind to inline completion handle

---------

Co-authored-by: Henning Dieterichs <hdieterichs@microsoft.com>
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.

None yet

5 participants