Skip to content

Add CompletionItem conversion for additionalTextEdits#87648

Merged
jrieken merged 1 commit intomicrosoft:masterfrom
jzyrobert:87346-additionalTextEdits
Dec 31, 2019
Merged

Add CompletionItem conversion for additionalTextEdits#87648
jrieken merged 1 commit intomicrosoft:masterfrom
jzyrobert:87346-additionalTextEdits

Conversation

@jzyrobert
Copy link
Contributor

This PR fixes #87346
Running the sample repo,
Before:

dantup
undefined

After:
Code_-_OSS_2019-12-24_00-00-09

@kieferrm kieferrm requested a review from jrieken December 24, 2019 18:40
@jrieken jrieken added this to the January 2020 milestone Dec 30, 2019
Copy link
Member

Choose a reason for hiding this comment

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

For commands, use

fromInternal(command: modes.Command): vscode.Command | undefined {
(which you somehow need to get here)

@jzyrobert jzyrobert requested a review from jrieken December 30, 2019 22:24
Copy link
Member

Choose a reason for hiding this comment

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

Why the if-defence? Shouldn't the type always be correct?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The edits.text of type ISingleEditOperation are possibly null, while modes.TextEdit requires a non-null string.

Is casting it to the type is sufficient?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah - cast it. It's TextEdit-objects that have sourced them, so they should be well defined.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Renove the range check as well? Or keep it.

Copy link
Member

Choose a reason for hiding this comment

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

nuke it

@jzyrobert
Copy link
Contributor Author

@jrieken

@jrieken
Copy link
Member

jrieken commented Dec 31, 2019

looking good now. thanks!

@jrieken jrieken merged commit d506ebe into microsoft:master Dec 31, 2019
@github-actions github-actions bot locked and limited conversation to collaborators Mar 27, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

vscode.executeCompletionItemProvider results are missing additionalTextEdits

2 participants