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

Fix completion on part of existing string #1941

Merged
merged 5 commits into from
Sep 28, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
Original file line number Diff line number Diff line change
Expand Up @@ -61,10 +61,17 @@ public class CompletionItem
public string? InsertText { get; set; }

/// <summary>
/// The format of <see cref="InsertText"/>.
/// The format of <see cref="InsertText"/>. This applies to both <see cref="InsertText"/> and
/// <see cref="TextEdit"/>.<see cref="LinePositionSpanTextChange.NewText"/>.
/// </summary>
public InsertTextFormat? InsertTextFormat { get; set; }

/// <summary>
/// An edit which is applied to a document when selecting this completion. When an edit is provided the value of
/// <see cref="InsertText"/> is ignored.
/// </summary>
public LinePositionSpanTextChange? TextEdit { get; set; }

/// <summary>
/// An optional set of characters that when pressed while this completion is active will accept it first and
/// then type that character.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,9 @@ public async Task<CompletionResponse> Handle(CompletionRequest request)
bool expectingImportedItems = expandedItemsAvailable && _workspace.Options.GetOption(CompletionItemExtensions.ShowItemsFromUnimportedNamespaces, LanguageNames.CSharp) == true;
var syntax = await document.GetSyntaxTreeAsync();

var replacingSpanStartPosition = sourceText.Lines.GetLinePosition(typedSpan.Start);
var replacingSpanEndPosition = sourceText.Lines.GetLinePosition(typedSpan.End);

for (int i = 0; i < completions.Items.Length; i++)
{
var completion = completions.Items[i];
Expand Down Expand Up @@ -233,14 +236,30 @@ public async Task<CompletionResponse> Handle(CompletionRequest request)

var change = await completionService.GetChangeAsync(document, completion);

// If the span we're using to key the completion off is the same as the replacement
// span, then we don't need to do anything special, just snippitize the text and
// exit
if (typedSpan == change.TextChange.Span)
{
// If the span we're using to key the completion off is the same as the replacement
// span, then we don't need to do anything special, just snippitize the text and
// exit
(insertText, insertTextFormat) = getAdjustedInsertTextWithPosition(change, position, newOffset: 0);
break;
}
if (change.TextChange.Span.Start > typedSpan.Start)
{
// If the span we're using to key the replacement span within the original typed span
// span, we want to prepend the missing text from the original typed text to here. The
// reason is that some lsp clients, such as vscode, use the range from the text edit as
// the selector for what filter text to use. This can lead to odd scenarios where invoking
// completion and typing `EQ` will bring up the Equals override, but then dismissing and
// reinvoking completion will have a range that just replaces the Q. Vscode will then consider
// that capital Q to be the start of the filter text, and filter out the Equals overload
// leaving the user with no completion and no explanation
Copy link
Contributor

@333fred 333fred Sep 18, 2020

Choose a reason for hiding this comment

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

Technically, vscode does have an API to deal with this inconsistency. But it's not in the lsp spec, only in the native vscode completion handler api, so we can't use it here.

Debug.Assert(change.TextChange.Span.End == typedSpan.End);
var prefix = typedText.Substring(0, change.TextChange.Span.Start - typedSpan.Start);

(insertText, insertTextFormat) = getAdjustedInsertTextWithPosition(change, position, newOffset: 0, prefix);
break;
}

int additionalEditEndOffset;
(additionalTextEdits, additionalEditEndOffset) = await GetAdditionalTextEdits(change, sourceText, (CSharpParseOptions)syntax!.Options, typedSpan, completion.DisplayText, providerName);
Expand Down Expand Up @@ -271,7 +290,14 @@ public async Task<CompletionResponse> Handle(CompletionRequest request)
completionsBuilder.Add(new CompletionItem
{
Label = completion.DisplayTextPrefix + completion.DisplayText + completion.DisplayTextSuffix,
InsertText = insertText,
TextEdit = new LinePositionSpanTextChange
{
NewText = insertText,
StartLine = replacingSpanStartPosition.Line,
StartColumn = replacingSpanStartPosition.Character,
EndLine = replacingSpanEndPosition.Line,
EndColumn = replacingSpanEndPosition.Character
},
InsertTextFormat = insertTextFormat,
AdditionalTextEdits = additionalTextEdits,
// Ensure that unimported items are sorted after things already imported.
Expand Down Expand Up @@ -359,7 +385,8 @@ static ImmutableArray<char> buildCommitCharacters(CSharpCompletionList completio
static (string, InsertTextFormat) getAdjustedInsertTextWithPosition(
CompletionChange change,
int originalPosition,
int newOffset)
int newOffset,
string? prependText = null)
{
// We often have to trim part of the given change off the front, but we
// still want to turn the resulting change into a snippet and control
Expand All @@ -375,7 +402,7 @@ static ImmutableArray<char> buildCommitCharacters(CSharpCompletionList completio
if (!(change.NewPosition is int newPosition)
|| newPosition >= (change.TextChange.Span.Start + newText.Length))
{
return (newText.Substring(newOffset), InsertTextFormat.PlainText);
return (prependText + newText.Substring(newOffset), InsertTextFormat.PlainText);
}

if (newPosition < (originalPosition + newOffset))
Expand All @@ -389,7 +416,7 @@ static ImmutableArray<char> buildCommitCharacters(CSharpCompletionList completio
// requested start to the new position, and from the new position to the end of the
// string.
int midpoint = newPosition - change.TextChange.Span.Start;
var beforeText = LspSnippetHelpers.Escape(newText.Substring(newOffset, midpoint - newOffset));
var beforeText = LspSnippetHelpers.Escape(prependText + newText.Substring(newOffset, midpoint - newOffset));
var afterText = LspSnippetHelpers.Escape(newText.Substring(midpoint));

return (beforeText + "$0" + afterText, InsertTextFormat.Snippet);
Expand Down
Loading