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

VS Code DSCv3 completion provider #448

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

anmenaga
Copy link
Collaborator

@anmenaga anmenaga commented Jun 5, 2024

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 by dsc resource list * -a Microsoft.DSC/PowerShell.

DSC-tab-completion.mp4

@anmenaga anmenaga requested a review from SteveL-MSFT June 5, 2024 23:26
Copy link
Member

@andyleejordan andyleejordan left a 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"
Copy link
Member

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.

Copy link
Member

Choose a reason for hiding this comment

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

Say "workspaceContains:**/*.dsc.yaml"

Copy link
Member

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"
}
}
Copy link
Member

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.

Copy link
Member

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
}
Copy link
Member

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`
Copy link
Member

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')
Copy link
Member

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:')) {
Copy link
Member

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;
Copy link
Member

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) {
Copy link
Member

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 😅

Copy link
Member

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?

Copy link
Member

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);
Copy link
Member

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 + ': ';
Copy link
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants