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 Razor completion bugs #6441

Merged
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
1 change: 1 addition & 0 deletions l10n/bundle.l10n.json
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@
"Unexpected error when attaching to C# preview window.": "Unexpected error when attaching to C# preview window.",
"Razor C# copied to clipboard": "Razor C# copied to clipboard",
"Copy C#": "Copy C#",
"{0} Keyword": "{0} Keyword",
"Unexpected completion trigger kind: {0}": "Unexpected completion trigger kind: {0}",
"1 reference": "1 reference",
"{0} references": "{0} references",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,8 @@ export class ProvisionalCompletionOrchestrator {
htmlPosition,
provisionalPosition,
completionContext,
projection.languageKind
projection.languageKind,
newDocument
);

// We track when we add provisional dots to avoid doing unnecessary work on commonly invoked events.
Expand Down
45 changes: 43 additions & 2 deletions src/razor/src/completion/razorCompletionItemProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,8 @@ export class RazorCompletionItemProvider extends RazorLanguageFeatureBase implem
hostDocumentPosition: vscode.Position,
projectedPosition: vscode.Position,
context: vscode.CompletionContext,
language: LanguageKind
language: LanguageKind,
razorDocument: vscode.TextDocument
) {
if (projectedUri) {
// "@" is not a valid trigger character for C# / HTML and therefore we need to translate
Expand Down Expand Up @@ -86,6 +87,11 @@ export class RazorCompletionItemProvider extends RazorLanguageFeatureBase implem
// In the code behind it's represented as __o = DateTime.
const completionCharacterOffset = projectedPosition.character - hostDocumentPosition.character;
for (const completionItem of completionItems) {
// vscode.CompletionItemKind is off by one compared to the LSP CompletionItemKind.
allisonchou marked this conversation as resolved.
Show resolved Hide resolved
if (completionItem.kind !== undefined) {
completionItem.kind = completionItem.kind - 1;
}

const doc = completionItem.documentation as vscode.MarkdownString;
if (doc && doc.value) {
// Without this, the documentation doesn't get rendered in the editor.
Expand Down Expand Up @@ -166,6 +172,8 @@ export class RazorCompletionItemProvider extends RazorLanguageFeatureBase implem
}
}

this.addUsingKeyword(language, razorDocument, hostDocumentPosition, completionItems);

const isIncomplete = completions instanceof Array ? false : completions ? completions.isIncomplete : false;
return new vscode.CompletionList(completionItems, isIncomplete);
}
Expand Down Expand Up @@ -221,7 +229,8 @@ export class RazorCompletionItemProvider extends RazorLanguageFeatureBase implem
position,
projection.position,
context,
projection.languageKind
projection.languageKind,
document
);

return completionList;
Expand Down Expand Up @@ -278,6 +287,38 @@ export class RazorCompletionItemProvider extends RazorLanguageFeatureBase implem

return item;
}

private static addUsingKeyword(
language: LanguageKind,
razorDocument: vscode.TextDocument,
hostDocumentPosition: vscode.Position,
completionItems: vscode.CompletionItem[]
) {
// This is an ugly hack, but it's needed to get the "using" keyword to show up in the completion list.
// The reason it doesn't show up is because the C# generated document puts the position of the cursor
// at '__o = [||]', which isn't a valid location for a using statement.
if (language == LanguageKind.CSharp) {
const line = razorDocument.lineAt(hostDocumentPosition.line);
const lineText = line.text.substring(0, hostDocumentPosition.character);
if (
lineText.endsWith('@') ||
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I know this check looks terrible - honestly it makes me cringe too 😅 If anyone has suggestions for an alternative, feel free to suggest

Copy link
Contributor

Choose a reason for hiding this comment

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

This works but... ugh...

What's the benefit of having a using snippet? Can we just make a server fix for Razor to add it's own completion for @ directives? I think we already do have some... (like @co)

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What's the benefit of having a using snippet? Can we just make a server fix for Razor to add it's own completion for @ directives? I think we already do have some...

I'm guessing a using snippet might be beneficial in the case where the user is already in a C# context and would want to insert something like using () { }?

I've filed dotnet/razor#9330 to track a (still hacky) server-side fix that we can use in both VS and VS Code. In the interest of getting a temporary fix in I'm planning on merging this version for now, but will follow up soon on the server-side fix.

lineText.endsWith(
'@u' ||
lineText.endsWith('@us') ||
lineText.endsWith('@usi') ||
lineText.endsWith('@usin') ||
lineText.endsWith('@using')
Comment on lines +304 to +310
Copy link
Member

Choose a reason for hiding this comment

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

You may have come up with the best way. If the user has already finished typing @using, does the item still need to be added?

Suggested change
lineText.endsWith('@') ||
lineText.endsWith(
'@u' ||
lineText.endsWith('@us') ||
lineText.endsWith('@usi') ||
lineText.endsWith('@usin') ||
lineText.endsWith('@using')
lineText.endsWith('@') ||
lineText.endsWith('@u') ||
lineText.endsWith('@us') ||
lineText.endsWith('@usi') ||
lineText.endsWith('@usin') ||
lineText.endsWith('@using')

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I considered this initially but wanted to match C#'s behavior. They seem to provide the completion item even if the user has finished typing out the whole word:
image

)
) {
const usingItem = new vscode.CompletionItem('using', vscode.CompletionItemKind.Keyword);

// Matching Roslyn's documentation behavior
(<CompletionItem>usingItem).documentation = vscode.l10n.t('{0} Keyword', 'using');

completionItems.push(usingItem);
}
}
}
}

function getTriggerKind(triggerKind: vscode.CompletionTriggerKind): CompletionTriggerKind {
Expand Down