-
Notifications
You must be signed in to change notification settings - Fork 5.4k
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
refactor(lsp): refactor completions and add tests #9789
Conversation
#[derive(Debug, Default, Clone, Deserialize)] | ||
#[serde(rename_all = "camelCase")] | ||
pub struct WorkspaceSettings { | ||
pub enable: bool, | ||
pub config: Option<String>, | ||
pub import_map: Option<String>, | ||
pub code_lens: Option<CodeLensSettings>, | ||
#[serde(default)] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These give us default values if they aren't sent by the client, this makes it a lot easier dealing with the values.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great. Tested it locally, and optional chaining completions now work correctly. 👍
} | ||
ScriptElementKind::ConstElement => CompletionItemKind::Constant, | ||
ScriptElementKind::LetElement | ||
ScriptElementKind::ConstElement |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why not use CompletionItemKind::Constant
for this? This is what sourcegraphs' tsc LSP does. Did you align this to VSC impl instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, I will double check that I didn't make a mistake though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope, that is the way it is in VSCode 🤷 :
private static convertKind(kind: string): vscode.CompletionItemKind {
switch (kind) {
case PConst.Kind.primitiveType:
case PConst.Kind.keyword:
return vscode.CompletionItemKind.Keyword;
case PConst.Kind.const:
case PConst.Kind.let:
case PConst.Kind.variable:
case PConst.Kind.localVariable:
case PConst.Kind.alias:
case PConst.Kind.parameter:
return vscode.CompletionItemKind.Variable;
case PConst.Kind.memberVariable:
case PConst.Kind.memberGetAccessor:
case PConst.Kind.memberSetAccessor:
return vscode.CompletionItemKind.Field;
case PConst.Kind.function:
case PConst.Kind.localFunction:
return vscode.CompletionItemKind.Function;
case PConst.Kind.method:
case PConst.Kind.constructSignature:
case PConst.Kind.callSignature:
case PConst.Kind.indexSignature:
return vscode.CompletionItemKind.Method;
case PConst.Kind.enum:
return vscode.CompletionItemKind.Enum;
case PConst.Kind.enumMember:
return vscode.CompletionItemKind.EnumMember;
case PConst.Kind.module:
case PConst.Kind.externalModuleName:
return vscode.CompletionItemKind.Module;
case PConst.Kind.class:
case PConst.Kind.type:
return vscode.CompletionItemKind.Class;
case PConst.Kind.interface:
return vscode.CompletionItemKind.Interface;
case PConst.Kind.warning:
return vscode.CompletionItemKind.Text;
case PConst.Kind.script:
return vscode.CompletionItemKind.File;
case PConst.Kind.directory:
return vscode.CompletionItemKind.Folder;
case PConst.Kind.string:
return vscode.CompletionItemKind.Constant;
default:
return vscode.CompletionItemKind.Property;
}
}
This refactor is to give us a more solid base to build on for import completions.
It refactors some code and adds a completion resolver, plus some tests.