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.

Sign up for GitHub

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

Jump to bottom

Ensure unimported things are sorted after imported things #1903

Merged
merged 2 commits into from
Aug 20, 2020
Merged

Ensure unimported things are sorted after imported things #1903

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
82b20b7
Ensure unimported things are sorted after imported things
333fred Aug 20, 2020
36f4c0f
Merge branch 'master' into sort-orders
filipw Aug 20, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 10 additions & 7 deletions src/OmniSharp.Roslyn.CSharp/Services/Completion/CompletionService.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,16 +157,15 @@ public async Task<CompletionResponse> 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<LinePositionSpanTextChange>? additionalTextEdits = null;
char sortTextPrepend = '0';

if (!completion.TryGetInsertionText(out string insertText))
{
Expand Down Expand Up @@ -254,7 +253,8 @@ public async Task<CompletionResponse> 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:
Expand All @@ -263,13 +263,16 @@ public async Task<CompletionResponse> 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,
Expand All @@ -281,7 +284,7 @@ public async Task<CompletionResponse> Handle(CompletionRequest request)

return new CompletionResponse
{
IsIncomplete = isIncomplete,
IsIncomplete = !seenUnimportedCompletions && expectingImportedItems,
Items = completionsBuilder.ToImmutableArray()
};

Expand Down
20 changes: 19 additions & 1 deletion tests/OmniSharp.Roslyn.CSharp.Tests/CompletionFacts.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
using System;
using System.Collections.Generic;
using System.Collections.Immutable;
using System.Linq;
using System.Threading;
using System.Threading.Tasks;
Expand Down Expand Up @@ -178,7 +179,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]
Expand Down Expand Up @@ -210,6 +216,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]
Expand Down Expand Up @@ -250,6 +257,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]
Expand Down Expand Up @@ -290,6 +298,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]
Expand Down Expand Up @@ -336,6 +345,7 @@ public class C3
Assert.Equal(6, additionalEdit.StartColumn);
Assert.Equal(8, additionalEdit.EndLine);
Assert.Equal(11, additionalEdit.EndColumn);
VerifySortOrders(completions.Items);
}

[Theory]
Expand Down Expand Up @@ -1278,6 +1288,14 @@ private OmniSharpTestHost GetImportCompletionHost()

private static string NormalizeNewlines(string str)
=> str.Replace("\r\n", Environment.NewLine);

private static void VerifySortOrders(ImmutableArray<CompletionItem> items)
{
Assert.All(items, c =>
{
Assert.True(c.SortText.StartsWith("0") || c.SortText.StartsWith("1"));
});
}
}

internal static class CompletionResponseExtensions
Expand Down