-
Couldn't load subscription status.
- Fork 260
Add command to generate Fig spec for VS Code terminal IntelliSense #5955
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
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.
This is awesome!
| // figGeneratorDefinitions returns the TypeScript code defining all Fig generators and interfaces used by them. | ||
| // The generator names in this code must match the constants defined above. | ||
| func figGeneratorDefinitions() string { | ||
| return `interface AzdEnvListItem { |
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.
nit: Instead of inlining this TypeScript stuff could we save this as an embedded resource and return it from there? It would be easier to catch TS syntax issues/errors can get better IntelliSense during updates?
Azure Dev CLI Install InstructionsInstall scriptsMacOS/Linux
bash: pwsh: WindowsPowerShell install MSI install Standalone Binary
MSI
Documentationlearn.microsoft.com documentationtitle: Azure Developer CLI reference
|
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.
Overall looks good! As a VSCode user myself, I am excited and happy with these changes.
Two categories of questions came into my mind as I reviewed this:
- I had some workflow questions in the doc around how we should handle alpha features and extension-related commands.
- I had some overall thoughts around the additional completion metadata we're adding to the new figspec package. I could see us creating separate issues to clean up the custom stuff we're doing in figspec over time.
| strings.Contains(flag.Value.Type(), "Array") | ||
|
|
||
| isRequired := false | ||
| if annotations := flag.Annotations["cobra_annotation_bash_completion_one_required_flag"]; len(annotations) > 0 { |
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.
nit: would be nice to reuse cobra.BashCompOneRequiredFlag, and explain this more nicely in a comment:
// Handle flags that are marked as `cmd.MarkFlagRequired`| } | ||
|
|
||
| // Flags that should only appear at root level, not duplicated on subcommands | ||
| var persistentOnlyFlags = []string{ |
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 remove this duplication that is currently maintained in both azd and figspec, and reuse root.PersistentFlags().
If we simply reused root.PersistentFlags(), I believe this will still lack two flags that are special to azd: --help and --docs, which are under-the-covers generated per-command. I think we should make a quick refactor for this.
Inside cmd/cmd_help.go, L97, we have these constants already defined:
// force the following flags as global flags for display purposes when displaying help.
forceGlobalFlagNames := map[string]struct{}{
"help": {},
"docs": {},
}It makes sense to refactor this out as a constant value NonPersistentGlobalFlags. Combining the names of root.PersistentFlags() and NonPersistentGlobalFlags should get us back to this list.
I think this is easy enough for us to clean up, and would feel strongly about removing a source of duplication.
| } | ||
| }, | ||
| listInstalledExtensions: { | ||
| script: ['azd', 'ext', 'list', '--installed', '--output', 'json'], |
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.
as a V2 improvement, I would love if we could move the script definition here back into core azd completions. If we did this, we would keep the postProcess around because that is still useful
| switch path { | ||
| case "azd auth login": | ||
| if flagName == "federated-credential-provider" { | ||
| return []string{"github", "azure-pipelines", "oidc"} |
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 may be worth creating an issue to move these completions into core azd, and avoid the ownership here in figspec. What are your thoughts?
The most native way is when to pull out the flag completion funcs defined in cobra as below:
cmd.RegisterFlagCompletionFunc("mode", func(
cmd *cobra.Command, args []string, toComplete string,
) ([]string, cobra.ShellCompDirective) {
return []string{"dev", "staging", "prod"}, cobra.ShellCompDirectiveNoFileComp
})| } | ||
| case "azd extension install": | ||
| return []Arg{ | ||
| {Name: "extension-id"}, |
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 the "right fix" here to rename the current arg extension-name in azd extension install <extension-name> to extension-id?
| } | ||
| case "azd show": | ||
| return []Arg{ | ||
| {Name: "resource-name|resource-id", IsOptional: 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.
I don't mind if we want to update azd show to azd show [resource-name|resource-id]
| var subcommands []Subcommand | ||
|
|
||
| for _, sub := range cmd.Commands() { | ||
| if sub.Hidden || sub.Name() == "help" { |
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.
nit: this doesn't respect sb.includeHidden as well, perhaps it should
|
|
||
| ## Usage | ||
|
|
||
| ### Generating the spec |
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.
This isn't covering a very important question around dev setup that I have in the back of my mind:
- alpha features: Should we enable all, or disable all
alphafeatures while generating? This impacts the command structure generator. I'd vote for enabling. I wouldn't be too concerned about these commands showing up. - extensions: Should we remove all extensions while generating? Or perhaps should the generation be excluding extension commands?
Resolves #5656
Related: microsoft/vscode#272348
This PR adds a new command
azd completion figto automatically generate a Fig autocomplete spec for integration with VS Code's terminal IntelliSense. The Fig spec for azd used by VS Code IntelliSense lives in the vscode repo atextensions/terminal-suggest/src/completions/azd.ts.The command works by traversing the Cobra command tree to extract all commands, flags, and arguments and converts this information to TypeScript, following the spec format. Hidden commands and flags are omitted from the spec, but there is a
--include-hiddenflag (also hidden) that can be passed.Dynamic suggestions
The azd spec leverages Fig generators to dynamically provide context-aware suggestions for certain commands.
For example, when the user types
azd init -t, VS Code in the background executes and parsesazd template list --output jsonto provide dynamic suggestions:Other commands with dynamic suggestions include:
azd env selectazd env get-valueazd extension installOpen to thoughts but I think this is a sufficient enough initial spec - there's plenty of opportunity to extend the spec further and we can track them as separate issues.
Implementation/architecture
See documentation in
cli/azd/docs/fig-spec.mdTesting
This PR adds a new
figspec_test.gosnapshot test. The test creates a snapshot of the Fig spec atcli/azd/cmd/testdata/TestFigSpec.ts. Similar to theTestUsagesnapshot tests, whenever the command structure changes (such as when adding a new command or flag), the Fig spec snapshot needs to be updated as well or the test would fail: