-
Notifications
You must be signed in to change notification settings - Fork 30
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
VS Code DSCv3 completion provider #448
base: main
Are you sure you want to change the base?
Conversation
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.
I like it! It's a great start.
"Other" | ||
], | ||
"activationEvents": [ | ||
"onLanguage:yaml" |
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.
We probably don't want it to activate for all YAML files so we'll want to figure out how to scope it to DSCv3 resources.
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.
Say "workspaceContains:**/*.dsc.yaml"
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.
Actually I think we'd want to define *.dsc.yaml
as the dscyaml language identifier for VS Code by modifying the following example:
"languages": [{
"id": "dscyaml",
"extensions": [ ".dsc.yaml", ".dsc.yml" ],
"aliases": [ "DSCv3", "DSCYAML" ] // etc.
}]
and then this would be "onLanguage:dscyaml"
"eslint": "^8.26.0", | ||
"typescript": "^5.4.2" | ||
} | ||
} |
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.
From my construction of https://github.com/microsoft/vscode-azurearcenabledmachines we can review and pull in a few more settings before getting this published as a preview.
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.
E.g. publisher is actually an identity and probably ms-vscode
, not "Microsoft," plus there's an aikey, update the engine, repo/homepage/icon etc.
@@ -0,0 +1,3 @@ | |||
{ | |||
"editor.insertSpaces": true | |||
} |
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.
Can remove this and just set a personal preference. The default indentation detection should just take care of it.
or | ||
|
||
in your terminal, run: | ||
`code --install-extension dscv3-completion-provider-0.0.1.vsix` |
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.
I'm going to suggest that this belongs in a separate repo from DSC in order to make building and releasing easier...are there any good reasons not to do that?
} | ||
|
||
let ps_cache_file_path: string = ''; | ||
if (process.platform==='win32') |
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.
Let's setup ESLint 😆
provideCompletionItems(document: vscode.TextDocument, position: vscode.Position) { | ||
|
||
const linePrefix = document.lineAt(position).text.slice(0, position.character); | ||
if (!linePrefix.endsWith('type:')) { |
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.
I'm unsure how VS Code handles filtering provided completions based on partial input (which we're discarding here)...something to look into. Like, are we the provider supposed to filter it or not?
|
||
const linePrefix = document.lineAt(position).text.slice(0, position.character); | ||
if (!linePrefix.endsWith('type:')) { | ||
return undefined; |
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.
Is there more completion we want to do in the future, or is after "type:" really the only thing we want to / can provide completions for?
|
||
const dataArray = JSON.parse(fs.readFileSync(ps_cache_file_path, 'utf-8')); | ||
var completions:vscode.CompletionItem[] = []; | ||
dataArray.ResourceCache.forEach(function (resouce: any) { |
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.
Just noting resouce
dropped an r
😅
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.
It's just an optimization but since (to my eye) everything in the lambda is...I don't know, is the word stateless? Not quite idempotent? But like the closure doesn't mutate anything, so we could fire all these completion builders off at once with await Promise.all(dataArray.ResourceCache.map(async (resource: any)...
right?
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.
Ok so technically you're pushing onto completions
from outside the closure but that was just an implementation choice, since this could also just return the completions from each async lambda and join them together as the final set of results. Is there any particular sorting we're wanting to have happen?
var completions:vscode.CompletionItem[] = []; | ||
dataArray.ResourceCache.forEach(function (resouce: any) { | ||
|
||
const all_props_completion = new vscode.CompletionItem(resouce.Type); |
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.
Should we specify a kind for the completion items? (One of these: https://vscode-api.js.org/enums/vscode.CompletionItemKind.html)
all_props_comp_text += '\nproperties:'; | ||
key_props_comp_text += '\nproperties:'; | ||
resouce.DscResourceInfo.Properties.forEach(function (prop: any) { | ||
all_props_comp_text += '\n ' + prop.Name + ': '; |
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.
If you want, it's probably a lot more readable to write this snippet as multiple strings ("which will be joined as multiple lines upon insertion") and is there any insertions or variables we might want to provide inside the completion snippet? https://code.visualstudio.com/docs/editor/userdefinedsnippets#_create-your-own-snippets
PR Summary
This is a prototype DSCv3 VS Code extension that provides completions for classic PS-class-based DSC resources when authoring
*.dsc.yaml
configuration files.Completions are displayed when
type:
is typed in a*.dsc.yaml
files.This requires latest dsc build from
main
branch and relies on PS Adapter cache, which can be generated bydsc resource list * -a Microsoft.DSC/PowerShell
.DSC-tab-completion.mp4