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

Component and stack completion support #992

Merged
merged 20 commits into from
Feb 12, 2025

Conversation

samtholiya
Copy link
Collaborator

@samtholiya samtholiya commented Jan 30, 2025

what

  • We are adding tab completion for components and stack

why

  • Providing completions for components and stack will improve the accuracy and speed of the user while executing commands that require components and stack.

references

Summary by CodeRabbit

  • New Features
     • Enhanced CLI commands with dynamic auto-completion for component and stack names, offering smart suggestions during execution.

  • Documentation
     • Updated user guides with clear instructions on setting up tab completions for Bash and Zsh.

  • Refactor
     • Improved the output formatting for listing commands to deliver clearer, more structured results.

  • Tests
     • Added new test cases to validate the auto-completion functionality and output consistency.

@mergify mergify bot added the triage Needs triage label Jan 30, 2025
@samtholiya samtholiya changed the title Initial component and stack completion support Component and stack completion support Jan 30, 2025
@samtholiya samtholiya marked this pull request as ready for review February 4, 2025 23:01
@samtholiya samtholiya requested a review from a team as a code owner February 4, 2025 23:01
Copy link
Contributor

coderabbitai bot commented Feb 4, 2025

📝 Walkthrough

Walkthrough

These changes enhance the Atmos CLI by adding dynamic tab completion for both component and stack arguments across many commands. New functions—such as ComponentsArgCompletion, stackFlagCompletion, and AddStackCompletion—have been introduced or integrated into the initialization of key commands. The modifications update command definitions to include a new ValidArgsFunction property and apply auto-completion setup via AddStackCompletion. In addition, list commands have been refactored to return slices instead of strings, and new YAML configuration files, tests, snapshots, and documentation have been added to validate and guide the tab completion functionality.

Changes

File(s) Change Summary
cmd/{aws_eks_update_kubeconfig.go, describe_component.go, describe_dependents.go, helmfile_generate_varfile.go, pro_lock.go, pro_unlock.go, terraform_generate_backend.go, terraform_generate_varfile.go, validate_component.go, vendor_pull.go, terraform_commands.go} Added ValidArgsFunction set to ComponentsArgCompletion and invoked AddStackCompletion (or registered flag completion) to enable dynamic component and stack suggestions.
cmd/{describe_affected.go, describe_stacks.go, helmfile.go, terraform.go, vendor_diff.go, workflow.go} Invoked AddStackCompletion to integrate stack completion functionality into command initialization.
cmd/{list_components.go, list_stacks.go} Refactored inline logic into separate functions (listComponents and listStacks) with updated error handling and integrated AddStackCompletion for completion support.
pkg/list/{list_components.go, list_components_test.go, list_stacks.go, list_stacks_test.go} Updated return types from a single string to a slice of strings and adjusted error/output handling for listing components and stacks.
cmd/cmd_utils.go Introduced new functions: stackFlagCompletion, AddStackCompletion, and ComponentsArgCompletion to provide dynamic shell completion capabilities.
tests/{fixtures/scenarios/completions/atmos.yaml, fixtures/scenarios/completions/stacks/catalog/station.yaml, fixtures/scenarios/completions/stacks/deploy/dev.yaml, snapshots/*, test-cases/tab-completions.yaml} Added YAML configuration files, new test cases, and updated snapshots aimed at validating component and stack completion functionality.
{website/docs/cli/commands/completion.mdx, cmd/root.go} Updated documentation with detailed tab completion setup instructions and modified the logging configuration in setupLogger.

Sequence Diagram(s)

sequenceDiagram
    participant U as User
    participant CLI as Atmos CLI
    participant CF as Completion Functions
    
    U->>CLI: Begin typing a command (e.g., "atmos terraform plan")
    CLI->>CF: Invoke ComponentsArgCompletion for component suggestions
    CF-->>CLI: Return list of valid component names
    CLI-->>U: Display component suggestions

    U->>CLI: Append "--stack" flag
    CLI->>CF: Invoke AddStackCompletion for stack suggestions
    CF-->>CLI: Return list of valid stack names
    CLI-->>U: Display stack suggestions
Loading

Assessment against linked issues

Objective Addressed Explanation
Implement tab completion support for atmos commands (components and stacks) [#45, #992]

Possibly related PRs

  • Glamorous implementation #853: The changes in the main PR, which enhance command-line argument completion for the awsEksCmdUpdateKubeconfigCmd command, are related to the modifications in the retrieved PR that also involve the AddStackCompletion function, indicating a shared focus on improving command usability through completion features.
  • feat: create atmos list command for listing stacks and components #797: The changes in the main PR, which enhance argument completion for the awsEksCmdUpdateKubeconfigCmd command, are related to the modifications in the retrieved PR that also introduce argument completion features for the list command and its subcommands, specifically through the use of AddStackCompletion and ComponentsArgCompletion. Both PRs involve similar functionalities aimed at improving user experience through command-line argument suggestions.

Suggested reviewers

  • aknysh
  • Gowiem

📜 Recent review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f0302af and 28efab3.

📒 Files selected for processing (1)
  • website/docs/cli/commands/completion.mdx (2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
  • website/docs/cli/commands/completion.mdx
⏰ Context from checks skipped due to timeout of 90000ms (5)
  • GitHub Check: Build (macos-latest, macos)
  • GitHub Check: Build (windows-latest, windows)
  • GitHub Check: website-deploy-preview
  • GitHub Check: Analyze (go)
  • GitHub Check: Summary

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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR. (Beta)
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary or @auto-summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai or @auto-title anywhere in the PR title to generate the title automatically.

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🧹 Nitpick comments (9)
cmd/pro_lock.go (1)

17-17: Command completion enhancement is well-positioned!

The addition of component and stack completion mirrors the changes in pro_unlock.go, maintaining consistency across related commands.

Consider adding a comment in the command's Long description to highlight the tab completion support:

-	Long:               `This command calls the atmos pro API and locks a stack`,
+	Long:               `This command calls the atmos pro API and locks a stack. Supports tab completion for components and stacks.`,

Also applies to: 32-32

pkg/list/list_stacks_test.go (1)

40-40: Consider moving the hardcoded component to a constant.

The hardcoded string "eks-blue/cluster" would be better defined as a constant at the package level for maintainability and reusability.

+const testEksComponent = "eks-blue/cluster"
+
 func TestListStacksWithComponent(t *testing.T) {
     // ...
-    output, err := FilterAndListStacks(stacksMap, "eks-blue/cluster")
+    output, err := FilterAndListStacks(stacksMap, testEksComponent)
cmd/terraform.go (1)

73-73: Fix typo in function name.

The function name has a typo: "Compltion" should be "Completion".

-AddStackCompltion(terraformCmd)
+AddStackCompletion(terraformCmd)
cmd/validate_component.go (1)

42-42: Fix typo in function name.

The function name has a typo: "Compltion" should be "Completion".

-AddStackCompltion(validateComponentCmd)
+AddStackCompletion(validateComponentCmd)
cmd/describe_stacks.go (1)

39-39: Fix typo in function name.

The function name has a typo: "Compltion" should be "Completion".

-AddStackCompltion(describeStacksCmd)
+AddStackCompletion(describeStacksCmd)
cmd/aws_eks_update_kubeconfig.go (1)

48-48: Fix typo in function name.

The function name has a typo: "Compltion" should be "Completion".

-AddStackCompltion(awsEksCmdUpdateKubeconfigCmd)
+AddStackCompletion(awsEksCmdUpdateKubeconfigCmd)
cmd/describe_affected.go (1)

44-44: LGTM! Consider fixing the typo in the function name.

The addition of stack completion functionality is well-placed. However, there's a typo in the function name (AddStackCompltion should be AddStackCompletion).

-AddStackCompltion(describeAffectedCmd)
+AddStackCompletion(describeAffectedCmd)
tests/test-cases/tab-completions.yaml (1)

1-17: Component Completion Test Case is Well-Defined

This test case is straightforward and clearly checks for the expected completion output ("station") when using the component flag. One minor nitpick: the argument at line 12 ("\"\""), while likely intentional to represent an empty string, could be double-checked to ensure it matches the intended behavior in your CLI's parsing logic.

tests/fixtures/scenarios/completions/atmos.yaml (1)

10-10: Remove Trailing Whitespace

There is a trailing space on line 10 as flagged by YAMLlint. It's a minor formatting issue, but cleaning it up will improve overall code quality.

🧰 Tools
🪛 YAMLlint (1.35.1)

[error] 10-10: trailing spaces

(trailing-spaces)

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 300ca0f and 5612f60.

📒 Files selected for processing (32)
  • cmd/aws_eks_update_kubeconfig.go (1 hunks)
  • cmd/cmd_utils.go (1 hunks)
  • cmd/describe_affected.go (1 hunks)
  • cmd/describe_component.go (1 hunks)
  • cmd/describe_dependents.go (2 hunks)
  • cmd/describe_stacks.go (1 hunks)
  • cmd/helmfile.go (1 hunks)
  • cmd/helmfile_generate_varfile.go (2 hunks)
  • cmd/list_components.go (2 hunks)
  • cmd/list_stacks.go (3 hunks)
  • cmd/pro_lock.go (2 hunks)
  • cmd/pro_unlock.go (2 hunks)
  • cmd/terraform.go (1 hunks)
  • cmd/terraform_commands.go (1 hunks)
  • cmd/terraform_generate_backend.go (2 hunks)
  • cmd/terraform_generate_varfile.go (2 hunks)
  • cmd/validate_component.go (2 hunks)
  • cmd/vendor_diff.go (1 hunks)
  • cmd/vendor_pull.go (1 hunks)
  • cmd/workflow.go (1 hunks)
  • pkg/list/list_components.go (2 hunks)
  • pkg/list/list_components_test.go (1 hunks)
  • pkg/list/list_stacks.go (2 hunks)
  • pkg/list/list_stacks_test.go (1 hunks)
  • tests/fixtures/scenarios/completions/atmos.yaml (1 hunks)
  • tests/fixtures/scenarios/completions/stacks/catalog/station.yaml (1 hunks)
  • tests/fixtures/scenarios/completions/stacks/deploy/dev.yaml (1 hunks)
  • tests/snapshots/TestCLICommands_completion_for_component_should_be_suggested.stderr.golden (1 hunks)
  • tests/snapshots/TestCLICommands_completion_for_component_should_be_suggested.stdout.golden (1 hunks)
  • tests/snapshots/TestCLICommands_completion_for_stacks_should_be_suggested.stderr.golden (1 hunks)
  • tests/snapshots/TestCLICommands_completion_for_stacks_should_be_suggested.stdout.golden (1 hunks)
  • tests/test-cases/tab-completions.yaml (1 hunks)
✅ Files skipped from review due to trivial changes (6)
  • tests/snapshots/TestCLICommands_completion_for_stacks_should_be_suggested.stderr.golden
  • tests/snapshots/TestCLICommands_completion_for_component_should_be_suggested.stdout.golden
  • tests/snapshots/TestCLICommands_completion_for_stacks_should_be_suggested.stdout.golden
  • tests/snapshots/TestCLICommands_completion_for_component_should_be_suggested.stderr.golden
  • tests/fixtures/scenarios/completions/stacks/deploy/dev.yaml
  • tests/fixtures/scenarios/completions/stacks/catalog/station.yaml
🧰 Additional context used
📓 Learnings (1)
cmd/validate_component.go (1)
Learnt from: samtholiya
PR: cloudposse/atmos#914
File: cmd/validate_stacks.go:20-23
Timestamp: 2025-01-09T22:27:25.538Z
Learning: The validate commands in Atmos can have different help handling implementations. Specifically, validate_component.go and validate_stacks.go are designed to handle help requests differently, with validate_stacks.go including positional argument checks while validate_component.go does not.
🪛 golangci-lint (1.62.2)
cmd/vendor_pull.go

31-31: Error return value of vendorPullCmd.RegisterFlagCompletionFunc is not checked

(errcheck)

cmd/cmd_utils.go

618-618: Error return value of cmd.RegisterFlagCompletionFunc is not checked

(errcheck)

🪛 YAMLlint (1.35.1)
tests/fixtures/scenarios/completions/atmos.yaml

[error] 10-10: trailing spaces

(trailing-spaces)

⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: Summary
🔇 Additional comments (27)
pkg/list/list_stacks.go (1)

10-10: Return type change looks solid, warrior!

The shift from string to []string return type is a battle-worthy improvement that better serves the command completion use case. The nil return for empty results follows proper Go idioms.

Let's verify the impact of this change:

Also applies to: 33-33, 36-36, 42-42

✅ Verification successful

🏁 Script executed:

#!/bin/bash
# Find all direct callers of FilterAndListStacks to ensure they handle the new return type
rg --type go "FilterAndListStacks\(" -A 2

Length of output: 838


Return type update verified – all direct callers are handling the new []string return type correctly.

  • Verified in cmd/list_stacks.go where the return values are directly assigned.
  • Confirmed in pkg/list/list_stacks_test.go with proper handling via YAML conversion.
cmd/pro_unlock.go (1)

17-17: Command completion enhancement looks battle-ready!

The addition of component and stack completion will make this command more efficient to use in the heat of battle.

Also applies to: 32-32

cmd/helmfile.go (1)

23-23: Stack completion addition is tactically sound!

This enhancement will provide valuable assistance during helmfile operations.

cmd/terraform_generate_backend.go (1)

17-17: Well done! Tab completion enhancements look solid.

The addition of ValidArgsFunction and AddStackCompltion provides a great user experience improvement for component and stack completion.

Also applies to: 33-33

cmd/helmfile_generate_varfile.go (1)

16-16: Excellent consistency in tab completion implementation!

The changes mirror the pattern used in other commands, maintaining a consistent user experience across the CLI.

Also applies to: 32-32

cmd/terraform_generate_varfile.go (1)

16-16: Great work maintaining consistency!

The tab completion enhancements follow the same robust pattern as other commands.

Also applies to: 32-32

pkg/list/list_stacks_test.go (1)

48-49: Great improvement in test assertions!

The specific stack name assertions make the test expectations clearer and more maintainable.

cmd/vendor_diff.go (2)

18-18: Documentation needs to be added before implementing the command.

The TODO comment indicates missing documentation at https://atmos.tools/cli/commands/vendor. This should be addressed before implementing the command to ensure proper usage guidance.

Would you like me to help draft the documentation for this command?


33-33: LGTM! Stack completion added.

The addition of stack completion enhances the command's usability, aligning with the PR's objective to improve CLI interaction.

cmd/describe_dependents.go (1)

17-17: LGTM! Component and stack completion added.

The changes enhance the command's usability by adding:

  1. Component name suggestions via ValidArgsFunction
  2. Stack name completion via AddStackCompltion

This implementation aligns perfectly with the PR's objective to improve CLI interaction.

Also applies to: 33-33

pkg/list/list_components_test.go (1)

48-53: LGTM! Test updated to validate component list.

The test has been properly updated to:

  1. Use the new FilterAndListComponents function
  2. Validate the structure and content of the returned component list
pkg/list/list_components.go (1)

31-31: LGTM! Function signature and error handling improved.

The changes enhance the function by:

  1. Returning a more appropriate []string type for component lists
  2. Providing more descriptive error messages
  3. Properly handling empty results with []string{}

This refactoring improves both the API and error handling.

Also applies to: 39-39, 43-43, 61-63

cmd/vendor_pull.go (1)

31-33: Great addition of completion support!

Adding tab completion for both components and stacks will significantly improve the user experience.

🧰 Tools
🪛 golangci-lint (1.62.2)

31-31: Error return value of vendorPullCmd.RegisterFlagCompletionFunc is not checked

(errcheck)

cmd/list_stacks.go (1)

45-59: Well-structured refactoring!

The extraction of stack listing logic into a separate function improves code organization and maintainability. The error messages are clear and descriptive.

cmd/list_components.go (1)

45-66: Clean and consistent refactoring!

The extraction of component listing logic follows the same pattern as the stack listing, maintaining consistency across the codebase. Error handling is thorough and messages are descriptive.

cmd/describe_component.go (1)

25-25: Excellent enhancement of command usability!

The addition of tab completion for both components and stacks will make the command more user-friendly and reduce errors.

Also applies to: 31-31

cmd/validate_component.go (1)

22-22: LGTM! Component completion added.

The addition of ComponentsArgCompletion will improve user experience by providing tab completion for component names.

cmd/aws_eks_update_kubeconfig.go (1)

41-41: LGTM! Component completion added.

The addition of ComponentsArgCompletion will improve user experience by providing tab completion for component names.

cmd/workflow.go (1)

191-191: LGTM! Same typo as mentioned before.

The addition of stack completion functionality is well-placed, but contains the same typo in the function name.

cmd/terraform_commands.go (1)

267-267: LGTM! Good addition of component completion.

The addition of component completion functionality is well-placed and enhances the user experience for terraform commands.

cmd/cmd_utils.go (2)

609-615: LGTM! Well-structured stack completion function.

The function properly handles errors and returns appropriate shell completion directives.


621-643: LGTM! Well-implemented component completion.

The function handles both component completion and flag completion with proper error handling and flag name parsing.

tests/test-cases/tab-completions.yaml (1)

18-34: Stacks Completion Test Case is Structured Properly

The test for stack completions is clear and mirrors the structure of the component test. It expects "dev" in the output, which appears consistent with your configuration. Everything looks clear here—great job!

tests/fixtures/scenarios/completions/atmos.yaml (4)

1-2: Base Path Declaration is Clear

The declaration of base_path in line 1 is concise and sets up the deployment context correctly.


3-9: Components Section is Organized

The components section for Terraform is well-structured with clear fields such as base_path, apply_auto_approve, deploy_run_init, etc. This configuration should readily support dynamic completion for component names.


11-17: Stacks Section is Configured Effectively

The stacks configuration is defined with base_path, inclusions, exclusions, and a naming pattern. This should integrate smoothly with the new stack completion functionality.


19-21: Logs Section is Set Up Appropriately

The logs section correctly configures the logging level and file output. The use of /dev/stderr is a common practice for CLI-based applications.

pkg/list/list_components_test.go Outdated Show resolved Hide resolved
cmd/vendor_pull.go Show resolved Hide resolved
cmd/cmd_utils.go Outdated Show resolved Hide resolved
@osterman
Copy link
Member

osterman commented Feb 5, 2025

This works well for me!

But please update docs on completion. Let's add some notes for configuring zsh.

Zsh Completion Setup

To enable tab completion for Atmos in Zsh, add the following to your ~/.zshrc:

# Initialize Zsh completion system
autoload -Uz compinit && compinit

# Enable Atmos CLI completion
source <(atmos completion zsh)

# Improve completion behavior
zstyle ':completion:*' menu select      # Enable menu selection
zstyle ':completion:*' force-list always # Force vertical menu listing

# Ensure the Tab key triggers autocompletion
bindkey '\t' expand-or-complete

After saving the file, apply the changes by running:

source ~/.zshrc

Now, pressing <Tab> after atmos will display available subcommands.

If completions do not work, try regenerating the completion cache:

rm -f ~/.zcompdump; compinit

@osterman osterman added minor New features that do not break anything and removed triage Needs triage labels Feb 5, 2025
@aknysh
Copy link
Member

aknysh commented Feb 6, 2025

@samtholiya please resolve the conflicts and address the comments

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (1)
cmd/cmd_utils.go (1)

621-643: Simplify flag name sanitization logic.

The flag name sanitization logic can be simplified to reduce code duplication and improve readability.

Consider this refactoring:

 func ComponentsArgCompletion(cmd *cobra.Command, args []string, toComplete string) ([]string, cobra.ShellCompDirective) {
     if len(args) == 0 {
         output, err := listComponents(cmd)
         if err != nil {
             return nil, cobra.ShellCompDirectiveNoFileComp
         }
         return output, cobra.ShellCompDirectiveNoFileComp
     }
     if len(args) > 0 {
         flagName := args[len(args)-1]
-        if strings.HasPrefix(flagName, "--") {
-            flagName = strings.ReplaceAll(flagName, "--", "")
-        }
-        if strings.HasPrefix(toComplete, "--") {
-            flagName = strings.ReplaceAll(toComplete, "--", "")
-        }
-        flagName = strings.ReplaceAll(flagName, "=", "")
+        // Use toComplete if it starts with "--", otherwise use the last arg
+        if strings.HasPrefix(toComplete, "--") {
+            flagName = toComplete
+        }
+        // Clean flag name by removing "--" prefix and "=" suffix
+        flagName = strings.TrimPrefix(flagName, "--")
+        flagName = strings.TrimSuffix(flagName, "=")
         if option, ok := cmd.GetFlagCompletionFunc(flagName); ok {
             return option(cmd, args, toComplete)
         }
     }
     return nil, cobra.ShellCompDirectiveNoFileComp
 }
📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 5612f60 and 9d6a070.

📒 Files selected for processing (3)
  • cmd/cmd_utils.go (1 hunks)
  • cmd/terraform.go (1 hunks)
  • cmd/terraform_commands.go (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
  • cmd/terraform_commands.go
  • cmd/terraform.go
🧰 Additional context used
🪛 golangci-lint (1.62.2)
cmd/cmd_utils.go

618-618: Error return value of cmd.RegisterFlagCompletionFunc is not checked

(errcheck)

⏰ Context from checks skipped due to timeout of 90000ms (3)
  • GitHub Check: Build (macos-latest, macos)
  • GitHub Check: Build (windows-latest, windows)
  • GitHub Check: Summary
🔇 Additional comments (2)
cmd/cmd_utils.go (2)

609-615: LGTM! Well-structured completion function.

The function properly handles errors and returns appropriate shell completion directives.


617-619: Previously identified issues with typo and error handling.

🧰 Tools
🪛 golangci-lint (1.62.2)

618-618: Error return value of cmd.RegisterFlagCompletionFunc is not checked

(errcheck)

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (2)
pkg/list/list_components_test.go (1)

50-52: Consider extracting the expected components list to a constant.

The hardcoded component list in the assertion makes the test brittle and harder to maintain. Consider extracting it to a constant or test data file.

+const expectedComponents = []string{
+    "infra/vpc",
+    "mixin/test-1",
+    "mixin/test-2",
+    "test/test-component",
+    "test/test-component-override",
+    "test/test-component-override-2",
+    "test/test-component-override-3",
+    "top-level-component1",
+    "vpc",
+    "vpc/new",
+}

 func TestListComponentsWithStack(t *testing.T) {
     // ... existing code ...
-    assert.ObjectsAreEqualValues([]string{"infra/vpc", "mixin/test-1", "mixin/test-2", "test/test-component", "test/test-component-override", "test/test-component-override-2", "test/test-component-override-3", "top-level-component1", "vpc", "vpc/new"}, output)
+    assert.ObjectsAreEqualValues(expectedComponents, output)
website/docs/cli/commands/completion.mdx (1)

65-86: Minor Corrections in Bash Completion Setup Instructions
Please fix the typo “bellow” to “below” for clarity. Also, the code block in step 4 is labeled as zsh while the instructions are for Bash—consider changing it to bash (or a generic shell marker) to keep the language consistent.

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9d6a070 and eec991e.

📒 Files selected for processing (2)
  • pkg/list/list_components_test.go (1 hunks)
  • website/docs/cli/commands/completion.mdx (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
  • GitHub Check: Build (macos-latest, macos)
  • GitHub Check: Build (windows-latest, windows)
  • GitHub Check: Summary
🔇 Additional comments (2)
pkg/list/list_components_test.go (1)

48-49: LGTM! Function signature change improves clarity.

The transition from FilterAndListStacks to FilterAndListComponents with reordered parameters makes the code more intuitive. The filter parameter (testStack) being first follows common filtering function patterns.

website/docs/cli/commands/completion.mdx (1)

31-64: Great job on the Zsh Completion Setup section!
This new block clearly details how users can enable Atmos CLI tab completion in Zsh with easy-to-follow, step-by-step instructions—including troubleshooting tips. Everything is well-organized and informative.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (5)
website/docs/cli/commands/completion/atmos-completion-zsh.mdx (1)

12-25: Configuration Code Block Review
The code snippet for initializing the Zsh completions includes all the necessary commands: initializing the completion system, sourcing the Atmos CLI completion output, adjusting the completion behavior with zstyle, and binding the Tab key. This well-structured snippet should make it simple for users to set up their environment.

Consider adding a brief inline comment explaining what each command does for novice users, although the current comments are sufficient for most.

website/docs/cli/commands/completion/atmos-completion-bash.mdx (2)

9-12: Typographical Error in Instructional Text

The sentence "follow the steps given bellow" contains a typo. It should read "follow the steps given below" for clarity.

Suggested diff:

-To enable tab completion for Atmos in `bash`, follow the steps given bellow
+To enable tab completion for Atmos in `bash`, follow the steps given below

31-33: Final Instruction is Concise and Informative

The final line clearly informs users that pressing <Tab> after typing atmos will show available subcommands. Additionally, considering recent feedback on enhancing shell completions, you might consider adding a cross-reference or note about Zsh configuration either in this file or in an associated Zsh documentation file if it isn’t redundant with existing docs.

website/docs/cli/commands/completion/usage.mdx (2)

31-40: Clear and Concise Examples Section

The new "Examples" section (lines 32–39) cleanly demonstrates how to generate completion scripts for Bash, Zsh, Fish, and PowerShell. This addition is clear and adds tangible value to users. However, based on community feedback (e.g., detailed Zsh configuration instructions) consider adding a brief note or link to a more detailed guide on configuring Zsh (such as initializing compinit and related settings) so that users who require this extra step can find guidance easily.


41-52: Comprehensive Script Loading Instructions

The section outlining how to load the Bash completion script (lines 41–52) is well written—it provides both the redirection-to-file and process substitution methods. As with the examples section, you might also want to include similar loading instructions for Zsh if there are additional steps (for instance, updating the ~/.zshrc file) needed to get completions working reliably in that shell.

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between eec991e and e5cb294.

📒 Files selected for processing (4)
  • website/docs/cli/commands/completion/_category_.json (1 hunks)
  • website/docs/cli/commands/completion/atmos-completion-bash.mdx (1 hunks)
  • website/docs/cli/commands/completion/atmos-completion-zsh.mdx (1 hunks)
  • website/docs/cli/commands/completion/usage.mdx (1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (4)
  • GitHub Check: Build (windows-latest, windows)
  • GitHub Check: Analyze (go)
  • GitHub Check: website-deploy-preview
  • GitHub Check: Summary
🔇 Additional comments (9)
website/docs/cli/commands/completion/atmos-completion-zsh.mdx (3)

8-11: Zsh Completion Setup Section Clarity
The "## Zsh Completion Setup" section introduces the configuration steps clearly. The instructions to add the configuration to ~/.zshrc are well presented and easy to follow.


27-31: Reloading Configuration Instructions
The instructions to apply changes by running source ~/.zshrc are clear and concise. This provides users with the necessary next step after updating their configuration.


35-39: Troubleshooting Guidance for Completion Cache
The troubleshooting section is helpful for users experiencing issues with completions. The recommendation to remove the existing .zcompdump and rerun compinit is a practical solution.

website/docs/cli/commands/completion/_category_.json (2)

1-11: JSON Structure is Solid.

The JSON object is correctly formatted and includes all the necessary properties (label, position, className, collapsible, collapsed, and link) to define the "completion" command category in the documentation. This clear structure should serve well for integrating the new tab completion documentation.


7-9: Verify Documentation Link.

Double-check that the "id" value "usage" within the link object accurately references the intended documentation page. This is especially important if later we add further details on Zsh configuration as mentioned in the PR objectives.

website/docs/cli/commands/completion/atmos-completion-bash.mdx (4)

1-7: Front Matter Metadata is Clear and Correct

The YAML header section is well-organized and clearly describes the document. This metadata will help users quickly identify the purpose of the file.


13-19: Bash Script Instructions Look Correct

The code block that captures the autocompletion script and sets the executable permission is concise and accurate. The use of the Bash redirection operator (&>) is appropriate for this context.


21-24: Sourcing Command in .bashrc is Clear

The instructions for adding the source command to .bashrc are clear and correctly formatted. This will ensure that the autocompletion script is loaded when a new Bash session is started.


26-29: Reloading Bash Configuration is Correct

The guidance to source .bashrc after making changes is spot on. This step ensures that users immediately benefit from the updated completion script.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (2)
website/docs/cli/commands/completion.mdx (2)

44-48: Bash Code Block Language Consistency
In the Bash Completion Setup tab, the code block for applying changes is currently marked as “zsh” even though it’s meant for Bash instructions. Switching the language annotation to “bash” will improve clarity for users.

Apply this diff:

-```zsh
+```bash
 source ~/.bashrc
-```
-```
+```

90-92: Refine Warning Note for Clarity
The warning note contains some repetitive phrasing that could be streamlined. Consider rephrasing to eliminate redundant wording and enhance readability. For example, try:

-Note: The Atmos command completion script is generated based on the currently active workspace. If you create the completion script outside of any workspace, it will not include custom commands from any specific workspace. If you later switch to a workspace with custom commands, the completion script will not be able to suggest them. Additionally, if you create the script in a workspace with custom commands and then switch to another workspace, it might incorrectly suggest those custom commands. To ensure accurate suggestions, it's recommended to generate or regenerate the completion script from within the appropriate workspace.
+Note: The Atmos command completion script is generated based on your active workspace. If you generate the script without an active workspace, it won’t include custom commands specific to any workspace. Similarly, switching workspaces after generating the script may lead to incorrect suggestions. To ensure accuracy, generate or regenerate the script from within the appropriate workspace.
🧰 Tools
🪛 LanguageTool

[style] ~91-~91: This phrase is redundant. Consider using “outside”.
Context: ...ce. If you create the completion script outside of any workspace, it will not include cust...

(OUTSIDE_OF)


[style] ~91-~91: This word has been used in one of the immediately preceding sentences. Using a synonym could make your text more interesting to read, unless the repetition is intentional.
Context: ...another workspace, it might incorrectly suggest those custom commands. To ensure accura...

(EN_REPEATEDWORDS_SUGGEST)

📜 Review details

Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e15575c and f0302af.

📒 Files selected for processing (1)
  • website/docs/cli/commands/completion.mdx (2 hunks)
🧰 Additional context used
🪛 LanguageTool
website/docs/cli/commands/completion.mdx

[style] ~91-~91: This phrase is redundant. Consider using “outside”.
Context: ...ce. If you create the completion script outside of any workspace, it will not include cust...

(OUTSIDE_OF)


[style] ~91-~91: This word has been used in one of the immediately preceding sentences. Using a synonym could make your text more interesting to read, unless the repetition is intentional.
Context: ...another workspace, it might incorrectly suggest those custom commands. To ensure accura...

(EN_REPEATEDWORDS_SUGGEST)

⏰ Context from checks skipped due to timeout of 90000ms (5)
  • GitHub Check: Build (windows-latest, windows)
  • GitHub Check: Analyze (javascript-typescript)
  • GitHub Check: website-deploy-preview
  • GitHub Check: Analyze (go)
  • GitHub Check: Summary
🔇 Additional comments (2)
website/docs/cli/commands/completion.mdx (2)

8-9: Docusaurus Tabs Integration
The new imports for Tabs and TabItem are exactly what’s needed to enable the structured, tabbed interface for the shell-specific instructions. This aligns well with previous feedback.


53-87: Zsh Completion Setup Looks Great
The instructions for Zsh—including initialization, setting up the completion system, and troubleshooting—are clear, detailed, and helpful. Kudos for covering the necessary steps for users to configure their environments properly.

@samtholiya samtholiya force-pushed the feature/dev-1780-shell-tab-completion branch from f0302af to c9edd24 Compare February 9, 2025 16:43
coderabbitai[bot]
coderabbitai bot previously approved these changes Feb 9, 2025
Copy link
Member

@aknysh aknysh left a comment

Choose a reason for hiding this comment

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

thanks @samtholiya

@aknysh aknysh merged commit d6fc8fe into main Feb 12, 2025
45 checks passed
@aknysh aknysh deleted the feature/dev-1780-shell-tab-completion branch February 12, 2025 16:56
Copy link

These changes were released in v1.161.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
minor New features that do not break anything
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Shell / Tab completion for atmos stacks and components
3 participants