-
Notifications
You must be signed in to change notification settings - Fork 248
feat: implement auto-installation workflow for extensions #5753
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
- Add auto-installation support when users run unknown commands that match extension namespaces - Prompt users to install matching extensions with clear descriptions - Automatically execute the original command after successful installation - Maintain existing behavior for commands without matching extensions - Improves discoverability and reduces friction for extension adoption Fixes Azure#5752
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 a great start! Added some feedback. Reach out if you have any questions.
…x the args for auto-install with the args for the extension command
…s and multi-namespace extensions
Azure Dev CLI Install InstructionsInstall scriptsMacOS/Linux
bash:
pwsh:
WindowsPowerShell install
MSI install
Standalone Binary
MSI
Documentationlearn.microsoft.com documentationtitle: Azure Developer CLI reference
|
for i, arg := range args { | ||
// Skip this argument if it's marked as a flag value from previous iteration | ||
if skipNext { | ||
skipNext = false | ||
continue | ||
} | ||
|
||
// If it doesn't start with '-', it's a potential command | ||
if !strings.HasPrefix(arg, "-") { | ||
return arg, unknownFlags | ||
} | ||
|
||
// Check if this is a known flag that takes a value | ||
if flagsWithValues[arg] { | ||
// This flag takes a value, so skip the next argument | ||
skipNext = true | ||
continue | ||
} | ||
|
||
// Handle flags with '=' syntax like --output=json | ||
if strings.Contains(arg, "=") { | ||
parts := strings.SplitN(arg, "=", 2) | ||
if flagsWithValues[parts[0]] { | ||
// This is a known flag=value format, no need to skip next | ||
continue | ||
} | ||
// Unknown flag with equals - record it | ||
unknownFlags = append(unknownFlags, parts[0]) | ||
continue | ||
} | ||
|
||
// This is an unknown flag - record it | ||
unknownFlags = append(unknownFlags, arg) | ||
|
||
// Conservative heuristic: if the next argument doesn't start with '-' | ||
// and there are more args after it, assume the unknown flag takes a value | ||
if i+1 < len(args) && i+2 < len(args) { | ||
nextArg := args[i+1] | ||
argAfterNext := args[i+2] | ||
if !strings.HasPrefix(nextArg, "-") && !strings.HasPrefix(argAfterNext, "-") { | ||
// Pattern: --unknown value command | ||
// Skip the value, let command be found next | ||
skipNext = 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 am curious about what pushed us dowwn this path. I would be concerned about the custom flag parsing logic here not matching cobra.. Is this something you may consider as well?
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 haven't looked that far, but my gut is telling me that we could simply invoke the command, and check for "unknown command" which is a public-facing documented thing here.
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.
Well, this is expected to be different than what Cobra does, and at this point we are looking at flags before the cobra command is executed.
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 could simply invoke the command, and check for "unknown command"
That was my first implementation. But before running auto-install, folks will always see an error like:
vivazqu@vib33:~/workspace/build/debug/custom-lang$ azd myExtension
Error: unknown command "myExtension" for "azd"
Did you mean this?
extension
Run 'azd --help' for usage.
That was really annoying to see and forced me to re-think in a better UX where we make it cooler 🎼
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.
Does this work? Override Stdout and Stderr for cobra, buffer it and decide to print it out after the error.
@vhvb1989 you rock for trying that out as the first implementation :)
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.
Override Stdout and Stderr for cobra, buffer it and decide to print it out after the error.
That means that if you run azd up
, you won't see any output until the end of executing cobra. We definitely don't want that 🏃♀️➡️
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, thanks for keeping me honest.. I'll come back to this real quick..
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.
Second quick scan: Does calling command.Find(args)
work?
// Find the target command given the args and command tree
// Meant to be run on the highest node. Only searches down.
func (c *Command) Find(args []string) (*Command, []string, error) {
From a quick look, this is used in cobra's path for figuring out the command invocation before printing out error and executing.
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.
Feels like we are doing more than we should be regarding any sort of flag/arg parsing.
Lets see if this is really needed and simplify the implementation.
// the flag's value type to determine if it takes an argument: | ||
// - Bool flags don't take values | ||
// - String, Int, StringSlice, etc. flags do take values | ||
func extractFlagsWithValues(cmd *cobra.Command) map[string]bool { |
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 do we need custom flag parsing? azd
flags should be handled by standard azd
core. Extension flags should be handled by extension itself...
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 to handle running something like azd --cwd path demo
where the demo
is the extension.
We don't want azd to think --cwd
or path
is the command we are trying to run XD
// This function properly handles flags that take values (like --output json) to avoid | ||
// incorrectly identifying flag values as commands. | ||
// Returns the command and any unknown flags encountered before the command. | ||
func findFirstNonFlagArg(args []string, flagsWithValues map[string]bool) (command string, unknownFlags []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.
Still not sure we need any of this??
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.
Not for the happy path, but this is mostly for starting with flags
} | ||
|
||
// ExecuteWithAutoInstall executes the command and handles auto-installation of extensions for unknown commands. | ||
func ExecuteWithAutoInstall(ctx context.Context, rootContainer *ioc.NestedContainer) error { |
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 think we are doing to much custom work for flag/arg passing here... I would have expected to see the following workflow.
- Invoke rootCmd
- Check error
- If command not found then
- Find matching extension
- Install matching extension
- Bind extension
- Re-execute root cmd (show pass now)
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.
That flow was the initial one... It is good for the happy path azd extension
but it was not enough as a general solution.
What I didn't like (the most) of trying to run first was that azd would start by printing the error "command not found". It didn't look or feel right.
So it evolved into:
- Create rootCmd
- Check if extensions is enabled - skip and run w/o auto-install if not
- Try to get the command invocation after removing flags - fail if there is ambiguity due to flags ordering
- Check if the command invocation is built-in or known already - skip and run w/o auto-install if so
- Find the extension(s) that matches and install - skip and run w/o auto-install if no matches
- Rebuild rootCmd and execute 🤝
Overview
This PR implements auto-installation of extensions when users attempt to run commands that match available extension namespaces but are not currently installed.
Problem
Users who try to run extension commands without having the extension installed receive unhelpful error messages like:
This creates friction in the user experience as users need to manually discover and install extensions.
Solution
New Approach: Proactive Extension Detection
Instead of running the command and capturing errors, the implementation now:
Key Implementation Details
findFirstNonFlagArg()
: Extracts the actual command from arguments, ignoring flagscheckForMatchingExtension()
: Queries the extension registry to find namespace matchestryAutoInstallExtension()
: Handles user prompting and extension installationExecuteWithAutoInstall()
: Main orchestration function that integrates with existing command executionUser Experience Improvements
Before:
After:
Changes Made
cmd/auto_install.go
: New file containing auto-installation logicmain.go
: Modified to useExecuteWithAutoInstall
instead of direct command executionTesting
azd demo
,azd x
)Fixes #5752