feat: add stack and component autocompletion for commands #820
Conversation
Add autocompletion functions for valid arguments and flags
📝 WalkthroughWalkthroughThis pull request introduces several enhancements across multiple command files, primarily focusing on adding autocompletion functionality for various flags related to components and stacks. New functions are implemented to provide dynamic suggestions based on user input, improving the command-line experience. Additionally, existing functions are modified to streamline error handling and output formatting, ensuring a more efficient retrieval of components and stacks. Changes
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (6)
pkg/list/list_stacks.go (2)
19-23: Consider removing error printing from utility functionsAvoid printing error messages within
FilterAndListStacks. Let the caller handle logging by returning a wrapped error.Suggested change:
- u.PrintMessageInColor(fmt.Sprintf("Error initializing CLI config: %v", err), color.New(color.FgRed)) - return nil, err + return nil, fmt.Errorf("error initializing CLI config: %w", err)
25-29: Avoid printing errors inside utility functionsSimilarly, remove the error printing when describing stacks. Return the error for the caller to handle.
Suggested change:
- u.PrintMessageInColor(fmt.Sprintf("Error describing stacks: %v", err), color.New(color.FgRed)) - return nil, err + return nil, fmt.Errorf("error describing stacks: %w", err)cmd/vendor_pull.go (1)
52-65: Consider consolidating duplicate error handling logic.Both completion functions share identical error handling patterns. Consider extracting this into a helper function to promote DRY principles.
+ func handleCompletionError(err error) ([]string, cobra.ShellCompDirective) { + if err != nil { + u.LogErrorAndExit(schema.CliConfiguration{}, err) + } + return nil, cobra.ShellCompDirectiveNoFileComp + }cmd/validate_component.go (1)
71-74: Consider more graceful error handling.The current implementation exits the program on error during autocompletion. Consider a more graceful approach that returns an empty list instead of terminating the program.
- if err != nil { - u.LogErrorAndExit(schema.CliConfiguration{}, err) - } + if err != nil { + u.LogError(err) + return []string{}, cobra.ShellCompDirectiveNoFileComp + }cmd/docs.go (2)
30-41: Consider enhancing the error handling and performanceYour ValidArgsFunction stands strong, but we can make it even mightier!
- Consider making the error message more specific:
componentList, err := l.FilterAndListComponents(toComplete) if err != nil { - u.LogErrorAndExit(schema.CliConfiguration{}, err) + u.LogErrorAndExit(schema.CliConfiguration{}, fmt.Errorf("failed to list components for autocompletion: %w", err)) }
- For optimal performance in battle, consider implementing caching for the component list if it's frequently accessed.
Line range hint
42-156: Strategic architectural improvements to considerBrave warrior, your implementation is solid! For even greater glory, consider these tactical improvements:
Extract the browser opening logic into a utility function like
openBrowser(url string) error. This would make the code more reusable and easier to test.If markdown rendering is used in other commands, consider creating a shared configuration in a separate package:
// pkg/markdown/renderer.go func NewAtmosRenderer(screenWidth int) (*glamour.TermRenderer, error) { return glamour.NewTermRenderer( glamour.WithColorProfile(lipgloss.ColorProfile()), glamour.WithAutoStyle(), glamour.WithPreservedNewLines(), glamour.WithWordWrap(screenWidth), ) }These changes would make your code more maintainable and battle-ready! 🗡️
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (9)
cmd/describe_component.go(3 hunks)cmd/describe_dependents.go(2 hunks)cmd/docs.go(2 hunks)cmd/list_components.go(1 hunks)cmd/list_stacks.go(1 hunks)cmd/validate_component.go(3 hunks)cmd/vendor_pull.go(2 hunks)pkg/list/list_components.go(3 hunks)pkg/list/list_stacks.go(2 hunks)
🔇 Additional comments (6)
pkg/list/list_components.go (2)
41-44: Remove error printing from utility function
As before, avoid printing errors within FilterAndListComponents. Return the error instead.
48-50: Avoid printing errors inside utility functions
Consistent with prior suggestions, remove the error printing here.
cmd/vendor_pull.go (1)
37-50: LGTM! Stack autocompletion implementation looks solid.
The implementation follows Cobra's best practices for flag completion. The args validation and error handling are appropriate.
cmd/describe_component.go (1)
53-66: Verify the argument validation requirement.
The stack completion function requires exactly one argument (len(args) != 1), while vendor_pull.go checks for zero arguments. This difference in validation logic seems intentional since a component name is required before selecting a stack, but it's worth verifying.
✅ Verification successful
Stack argument validation is correctly implemented
The validation logic in describe_component.go requiring exactly one argument is correct and consistent with the command's purpose. Here's what the codebase reveals:
- Commands that operate on a specific component (
describe_component.go,validate_component.go) require exactly one argument (len(args) != 1) - Commands that list or pull components (
list_components.go,vendor_pull.go,describe_dependents.go) require zero arguments (len(args) != 0)
This pattern makes perfect sense as commands operating on specific components need the component name before stack selection, while listing commands don't.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check argument validation consistency across commands
# Look for other stack completion implementations to verify the pattern
rg -A 5 "RegisterFlagCompletionFunc.*stack" cmd/
Length of output: 2219
cmd/docs.go (2)
16-16: LGTM! Clean import addition.
The new import for the list package is well-organized and properly aliased. Stand strong, warrior! 💪
25-29: Well-structured command definition, warrior!
The command definition is clear, concise, and well-documented. The example and usage instructions provide excellent guidance for our fellow warriors.
| ValidArgsFunction: func(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) { | ||
| if len(args) != 0 { | ||
| return nil, cobra.ShellCompDirectiveNoFileComp | ||
| } | ||
|
|
||
| componentList, err := l.FilterAndListComponents(toComplete) | ||
| if err != nil { | ||
| u.LogErrorAndExit(schema.CliConfiguration{}, err) | ||
| } | ||
|
|
||
| return componentList, cobra.ShellCompDirectiveNoFileComp | ||
| }, |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Consider creating shared autocompletion utilities.
The autocompletion implementations are identical across multiple command files. Consider creating shared utility functions to:
- Reduce code duplication
- Ensure consistent validation logic
- Centralize error handling
Create a new file cmd/completion_utils.go:
package cmd
import (
"github.com/spf13/cobra"
l "github.com/cloudposse/atmos/pkg/list"
"github.com/cloudposse/atmos/pkg/schema"
u "github.com/cloudposse/atmos/pkg/utils"
)
func ComponentCompletion(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
if len(args) != 0 {
return nil, cobra.ShellCompDirectiveNoFileComp
}
componentList, err := l.FilterAndListComponents(toComplete)
if err != nil {
u.LogErrorAndExit(schema.CliConfiguration{}, err)
}
return componentList, cobra.ShellCompDirectiveNoFileComp
}
func StackCompletionWithComponent(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
if len(args) != 1 {
return nil, cobra.ShellCompDirectiveNoFileComp
}
stacksList, err := l.FilterAndListStacks(toComplete)
if err != nil {
u.LogErrorAndExit(schema.CliConfiguration{}, err)
}
return stacksList, cobra.ShellCompDirectiveNoFileComp
}Also applies to: 65-78
|
@RoseSecurity thanks for this! Excited for it. Should this work? 
|
Cobra can do autocompletion only for the configured Cobra commands. |
|
@aknysh cobra supports completion functions for handling flags. Wouldn't that work? https://github.com/spf13/cobra/blob/main/site/content/completions/_index.md#completions-for-flags https://github.com/spf13/cobra/blob/main/completions.go See flag completions. |
|
Interestingly, this was working for me locally yesterday, but today, it doesn't produce the same result. My goal is to restructure the Going to keep working at this! |
|
Thanks, @RoseSecurity! This will be a huge QoL improvement. Out of curiosity, will you take a stab at |
|
Closing this until the improved |
what
pkg/listfunctions to include configuration initialization so that the outputted component and stack lists could be easily used by Cobra valid argument functions and flag completion functionsValidArgsFunctionandRegisterFlagCompletionFuncfor stacks and components in the following commands:atmos describe componentatmos describe dependentsatmos docsatmos list componentsatmos list stacksatmos validate componentatmos vendor pullwhy
testing
make buildafter each command autocompletion addition and testedreferences
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Chores