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

Better handle completion when the display text is not in the final result #1908

Merged
merged 3 commits into from
Aug 22, 2020

Conversation

333fred
Copy link
Contributor

@333fred 333fred commented Aug 21, 2020

Override and partial method completion can have scenarios where the DisplayText of the completion item does not appear in the final text, such as when types need to be imported or when nullability differs between original declaration and generation declaration sites. We now handle these by parsing the change if we fail to find the insertion text, and using the last method in the change to find the correct location in the new text. Fixes #1907.

…sult

Override and partial method completion can have scenarios where the DisplayText of the completion item does not appear in the final text, such as when types need to be imported or when nullability differs between original declaration and generation declaration sites. We now handle these by parsing the change if we fail to find the insertion text, and using the last method in the change to find the correct location in the new text. Fixes OmniSharp#1907.
Copy link
Member

@filipw filipw left a comment

Choose a reason for hiding this comment

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

LGTM
thanks for the helpful comments too!

Copy link
Member

@bjorkstromm bjorkstromm left a comment

Choose a reason for hiding this comment

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

LGTM!

@@ -23,7 +23,7 @@ public class CompletionItem
/// <summary>
/// Tags for this completion item
/// </summary>
public ImmutableArray<CompletionItemTag>? Tags { get; set; }
public IReadOnlyList<CompletionItemTag>? Tags { get; set; }
Copy link
Member

Choose a reason for hiding this comment

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

Normally, I'd be hesitant introducing breaking changes in Abstractions (since we ship NuGet packages of this). But this is a quite new API, so it's cool 🙂

@filipw filipw merged commit d5f7a77 into OmniSharp:master Aug 22, 2020
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.

Override completion can mess up based on context
3 participants