From 82b20b79e5d36b163afd27211c57b7c84c8e92d6 Mon Sep 17 00:00:00 2001 From: Fredric Silberberg Date: Wed, 19 Aug 2020 20:07:48 -0700 Subject: [PATCH] Ensure unimported things are sorted after imported things Prepend a '0' or '1' for already-imported and unimported things, respectively. This will ensure that things that are already imported will appear first in the list of completions, which is more likely to be what the user actually wants. --- .../Services/Completion/CompletionService.cs | 17 +++++++++------- .../CompletionFacts.cs | 20 ++++++++++++++++++- 2 files changed, 29 insertions(+), 8 deletions(-) diff --git a/src/OmniSharp.Roslyn.CSharp/Services/Completion/CompletionService.cs b/src/OmniSharp.Roslyn.CSharp/Services/Completion/CompletionService.cs index 37f2a3de2e..28c7c1ed48 100644 --- a/src/OmniSharp.Roslyn.CSharp/Services/Completion/CompletionService.cs +++ b/src/OmniSharp.Roslyn.CSharp/Services/Completion/CompletionService.cs @@ -156,16 +156,15 @@ public async Task Handle(CompletionRequest request) // that completion provider is still creating the cache. We'll mark this completion list as not completed, and the // editor will ask again when the user types more. By then, hopefully the cache will have populated and we can mark // the completion as done. - bool isIncomplete = expandedItemsAvailable && - _workspace.Options.GetOption(CompletionItemExtensions.ShowItemsFromUnimportedNamespaces, LanguageNames.CSharp) == true; + bool seenUnimportedCompletions = false; + bool expectingImportedItems = expandedItemsAvailable && _workspace.Options.GetOption(CompletionItemExtensions.ShowItemsFromUnimportedNamespaces, LanguageNames.CSharp) == true; for (int i = 0; i < completions.Items.Length; i++) { var completion = completions.Items[i]; - var commitCharacters = buildCommitCharacters(completions, completion.Rules.CommitCharacterRules, triggerCharactersBuilder); - var insertTextFormat = InsertTextFormat.PlainText; ImmutableArray? additionalTextEdits = null; + char sortTextPrepend = '0'; if (!completion.TryGetInsertionText(out string insertText)) { @@ -251,7 +250,8 @@ public async Task Handle(CompletionRequest request) // This is technically slightly incorrect: extension method completion can provide // partial results. However, this should only affect the first completion session or // two and isn't a big problem in practice. - isIncomplete = false; + seenUnimportedCompletions = true; + sortTextPrepend = '1'; goto default; default: @@ -260,13 +260,16 @@ public async Task Handle(CompletionRequest request) } } + var commitCharacters = buildCommitCharacters(completions, completion.Rules.CommitCharacterRules, triggerCharactersBuilder); + completionsBuilder.Add(new CompletionItem { Label = completion.DisplayTextPrefix + completion.DisplayText + completion.DisplayTextSuffix, InsertText = insertText, InsertTextFormat = insertTextFormat, AdditionalTextEdits = additionalTextEdits, - SortText = completion.SortText, + // Ensure that unimported items are sorted after things already imported. + SortText = expectingImportedItems ? sortTextPrepend + completion.SortText : completion.SortText, FilterText = completion.FilterText, Kind = getCompletionItemKind(completion.Tags), Detail = completion.InlineDescription, @@ -278,7 +281,7 @@ public async Task Handle(CompletionRequest request) return new CompletionResponse { - IsIncomplete = isIncomplete, + IsIncomplete = !seenUnimportedCompletions && expectingImportedItems, Items = completionsBuilder.MoveToImmutable() }; diff --git a/tests/OmniSharp.Roslyn.CSharp.Tests/CompletionFacts.cs b/tests/OmniSharp.Roslyn.CSharp.Tests/CompletionFacts.cs index 5f029867bb..53f0f9ab7e 100644 --- a/tests/OmniSharp.Roslyn.CSharp.Tests/CompletionFacts.cs +++ b/tests/OmniSharp.Roslyn.CSharp.Tests/CompletionFacts.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Generic; +using System.Collections.Immutable; using System.Linq; using System.Threading; using System.Threading.Tasks; @@ -177,7 +178,12 @@ public Class1() using var host = GetImportCompletionHost(); var completions = await FindCompletionsWithImportedAsync(filename, input, host); - Assert.True(completions.Items.First(c => c.InsertText == "guid").Data < completions.Items.First(c => c.InsertText == "Guid").Data); + CompletionItem localCompletion = completions.Items.First(c => c.InsertText == "guid"); + CompletionItem typeCompletion = completions.Items.First(c => c.InsertText == "Guid"); + Assert.True(localCompletion.Data < typeCompletion.Data); + Assert.StartsWith("0", localCompletion.SortText); + Assert.StartsWith("1", typeCompletion.SortText); + VerifySortOrders(completions.Items); } [Theory] @@ -209,6 +215,7 @@ public static void Test(this object o) using var host = GetImportCompletionHost(); var completions = await FindCompletionsWithImportedAsync(filename, input, host); Assert.Contains("Test", completions.Items.Select(c => c.InsertText)); + VerifySortOrders(completions.Items); } [Theory] @@ -249,6 +256,7 @@ public static void Test(this object o) Assert.Equal(0, additionalEdit.StartColumn); Assert.Equal(6, additionalEdit.EndLine); Assert.Equal(13, additionalEdit.EndColumn); + VerifySortOrders(completions.Items); } [Theory] @@ -289,6 +297,7 @@ public static void Test(this object o) Assert.Equal(0, additionalEdit.StartColumn); Assert.Equal(6, additionalEdit.EndLine); Assert.Equal(19, additionalEdit.EndColumn); + VerifySortOrders(completions.Items); } [Theory] @@ -335,6 +344,7 @@ public class C3 Assert.Equal(6, additionalEdit.StartColumn); Assert.Equal(8, additionalEdit.EndLine); Assert.Equal(11, additionalEdit.EndColumn); + VerifySortOrders(completions.Items); } [Theory] @@ -1252,6 +1262,14 @@ private OmniSharpTestHost GetImportCompletionHost() private static string NormalizeNewlines(string str) => str.Replace("\r\n", Environment.NewLine); + + private static void VerifySortOrders(ImmutableArray items) + { + Assert.All(items, c => + { + Assert.True(c.SortText.StartsWith("0") || c.SortText.StartsWith("1")); + }); + } } internal static class CompletionResponseExtensions