Implement: atmos list values #1036
Conversation
📝 WalkthroughWalkthroughThis update introduces two new CLI commands— Changes
Possibly related PRs
Suggested labels
Suggested reviewers
✨ Finishing Touches
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: 2
🧹 Nitpick comments (11)
pkg/list/list_workflows.go (1)
202-230: Simplify the default case logic.The default case handles both empty format and "table" format with duplicated string joining logic. This can be simplified to reduce code duplication.
Consider this refactor:
default: + var output strings.Builder + output.WriteString(strings.Join(header, delimiter) + utils.GetLineEnding()) + for _, row := range rows { + output.WriteString(strings.Join(row, delimiter) + utils.GetLineEnding()) + } + // If format is empty or "table", use table format if format == "" && term.IsTTYSupportForStdout() { // Create a styled table for TTY t := table.New(). // ... existing table configuration ... Headers(header...). Rows(rows...) return t.String() + utils.GetLineEnding(), nil } - // Default to simple tabular format for non-TTY or when format is explicitly "table" - var output strings.Builder - output.WriteString(strings.Join(header, delimiter) + utils.GetLineEnding()) - for _, row := range rows { - output.WriteString(strings.Join(row, delimiter) + utils.GetLineEnding()) - } return output.String(), nilpkg/list/list_values.go (3)
23-31: Consider removing the unused function.The static analysis tool confirms that
getMapKeysis unused. If there's no future usage planned, removing it will reduce clutter.-// getMapKeys returns a sorted slice of map keys -func getMapKeys(m map[string]interface{}) []string { - keys := make([]string, 0, len(m)) - for k := range m { - keys = append(keys, k) - } - sort.Strings(keys) - return keys -}🧰 Tools
🪛 golangci-lint (1.62.2)
24-24: func
getMapKeysis unused(unused)
34-324: Refactor the large function.
FilterAndListValueshandles multiple steps: filtering stacks, applying queries, formatting output, etc. Splitting it into smaller helper functions would improve readability and maintainability.
265-272: Handle CSV field quoting.Concatenating fields with a plain delimiter can corrupt CSV data if the field itself contains the delimiter or newline. Consider quoting fields for robust CSV output.
pkg/list/list_values_test.go (1)
56-148: Expand test coverage.You might test nested or array-based query paths to confirm filtering works for complex data structures. It would further validate the code’s resilience.
cmd/list_values.go (2)
35-63: Avoid repeated flag error-handling.Error-handling for each flag is repeated. Consolidating this into a small helper would reduce duplication and simplify reading.
95-116: Evaluate independent command structure.
listVarsCmdsets a single flag before reusinglistValuesCmd.Run. This works but might complicate future expansions. Consider a dedicated or unified approach for better clarity.website/docs/cli/commands/list/list-values.mdx (4)
31-42: Consider enhancing flag documentation with example values.While the flag descriptions are clear, adding example values would make them more user-friendly.
<dt><code>--query string</code></dt> - <dd>JMESPath query to filter values (e.g., ".vars" to show only variables)</dd> + <dd>JMESPath query to filter values (e.g., ".vars" to show only variables, ".config.vpc" to show VPC configuration)</dd> <dt><code>--format string</code></dt> - <dd>Output format: `table`, `json`, `csv`, `tsv` (default "`table`")</dd> + <dd>Output format: `table`, `json`, `csv`, `tsv` (default "`table`"). Example: --format=json</dd>
58-58: Replace TODO placeholder with actual JMESPath query examples.The placeholder needs to be replaced with practical JMESPath query examples to help users understand how to filter values effectively.
Would you like me to help generate some practical JMESPath query examples? Here's a suggestion:
-TODO: define more outputs +# Show only networking configuration +atmos list values vpc --query ".config.networking" + +# Filter for specific environment variables +atmos list values vpc --query ".vars | [?contains(name, 'ENV')]" + +# Show components with specific tags +atmos list values vpc --query ".metadata.tags"
86-86: Add example command output to enhance documentation.Replace the TODO with actual command output to help users understand what to expect.
Would you like me to help generate an example output? Here's a suggestion:
-TODO: define example output +Component: vpc + +┌─────────────┬──────────────┬──────────────┬──────────────┐ +│ Key │ dev-ue1 │ staging-ue1 │ prod-ue1 │ +├─────────────┼──────────────┼──────────────┼──────────────┤ +│ cidr_block │ 10.0.0.0/16 │ 10.1.0.0/16 │ 10.2.0.0/16 │ +│ enable_flow │ true │ true │ true │ +│ max_azs │ 3 │ 3 │ 3 │ +└─────────────┴──────────────┴──────────────┴──────────────┘
91-92: Enhance typography in related commands section.Replace hyphens with em dashes for better readability.
-[atmos list components](/cli/commands/list/component) - List available components -[atmos describe component](/cli/commands/describe/component) - Show detailed information about a component +[atmos list components](/cli/commands/list/component) — List available components +[atmos describe component](/cli/commands/describe/component) — Show detailed information about a component🧰 Tools
🪛 LanguageTool
[typographical] ~91-~91: To join two clauses or introduce examples, consider using an em dash.
Context: ...omponents](/cli/commands/list/component) - List available components - [atmos descr...(DASH_RULE)
[typographical] ~92-~92: To join two clauses or introduce examples, consider using an em dash.
Context: ...onent](/cli/commands/describe/component) - Show detailed information about a compon...(DASH_RULE)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
cmd/list_values.go(1 hunks)pkg/list/list_values.go(1 hunks)pkg/list/list_values_test.go(1 hunks)pkg/list/list_workflows.go(1 hunks)pkg/list/list_workflows_test.go(7 hunks)website/docs/cli/commands/list/list-values.mdx(1 hunks)
🧰 Additional context used
🪛 LanguageTool
website/docs/cli/commands/list/list-values.mdx
[typographical] ~91-~91: To join two clauses or introduce examples, consider using an em dash.
Context: ...omponents](/cli/commands/list/component) - List available components - [atmos descr...
(DASH_RULE)
[typographical] ~92-~92: To join two clauses or introduce examples, consider using an em dash.
Context: ...onent](/cli/commands/describe/component) - Show detailed information about a compon...
(DASH_RULE)
🪛 golangci-lint (1.62.2)
pkg/list/list_values.go
24-24: func getMapKeys is unused
(unused)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Summary
🔇 Additional comments (3)
pkg/list/list_workflows.go (1)
25-25: LGTM! New format constants follow the established pattern.The addition of
FormatYAMLandFormatTSVconstants maintains consistency with the existing format naming convention.Also applies to: 27-27
pkg/list/list_workflows_test.go (1)
76-76: LGTM! File permission notation updated to modern format.The change to use the
0oprefix for octal file permissions is more explicit and follows modern Go conventions.Also applies to: 89-89, 100-100, 118-118, 136-136, 310-310
website/docs/cli/commands/list/list-values.mdx (1)
1-28: Strong work on the command description!The command description effectively communicates the purpose and value proposition of the
atmos list valuescommand. The tabular view explanation is particularly clear and helpful.
Co-authored-by: Erik Osterman (CEO @ Cloud Posse) <[email protected]>
cmd/list_values.go
Outdated
| // Initialize logger from CLI config | ||
| configAndStacksInfo := schema.ConfigAndStacksInfo{} | ||
| atmosConfig, err := config.InitCliConfig(configAndStacksInfo, true) | ||
| if err != nil { | ||
| fmt.Fprintf(os.Stderr, "Error initializing CLI config: %v\n", err) | ||
| return | ||
| } | ||
|
|
||
| logger, err := l.NewLoggerFromCliConfig(atmosConfig) | ||
| if err != nil { | ||
| fmt.Fprintf(os.Stderr, "Error initializing logger: %v\n", err) | ||
| return | ||
| } | ||
|
|
There was a problem hiding this comment.
I don't believe we need to do this everywhere. And if we did, then we should have a function for it, so we don't duplicate this logic.
There was a problem hiding this comment.
config.InitCliConfig(configAndStacksInfo, true) is used to start config.
however we indeed don't need logger, err := l.NewLoggerFromCliConfig(atmosConfig)
cmd/list_values.go
Outdated
| Use: "values [component]", | ||
| Short: "List component values across stacks", | ||
| Long: "List values for a component across all stacks where it is used", | ||
| Example: "atmos list values vpc\n" + |
There was a problem hiding this comment.
Move to markdown file and embed with properly annotated examples.
There was a problem hiding this comment.
so are we going to stop using it even for cobra cli only?
There was a problem hiding this comment.
Please look at how we format this in the other files and stick to convention. Cobra handles usage, flags, etc.
https://github.com/cloudposse/atmos/blob/main/cmd/markdown/atmos_workflow_usage.md
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (2)
cmd/list_metadata.go (1)
33-36: 🛠️ Refactor suggestionUse semantic logging consistently.
Replace fmt.Fprintf with semantic logging to maintain consistency with the rest of the codebase.
- fmt.Fprintf(os.Stderr, "Error initializing CLI config: %v\n", err) + log.Error("failed to initialize CLI config", "error", err)cmd/list_values.go (1)
21-29: 🛠️ Refactor suggestionMove examples to markdown files.
Based on the project's conventions, examples should be moved to markdown files and properly embedded.
Please move the examples to separate markdown files and use the embedding pattern as shown in other commands. Reference: #959
Also applies to: 116-121
🧹 Nitpick comments (4)
cmd/list_metadata.go (1)
71-73: Enhance format comparison with constants.Consider extracting the format comparison into a constant for better maintainability.
+const ( + defaultDelimiterByFormat = map[string]string{ + list.FormatCSV: list.DefaultCSVDelimiter, + list.FormatTSV: list.DefaultTSVDelimiter, + } +) - if formatFlag == list.FormatCSV && delimiterFlag == list.DefaultTSVDelimiter { - delimiterFlag = list.DefaultCSVDelimiter + if defaultDelimiter, ok := defaultDelimiterByFormat[formatFlag]; ok && delimiterFlag == list.DefaultTSVDelimiter { + delimiterFlag = defaultDelimitercmd/list_settings.go (3)
16-26: Enhance command examples with output descriptions.While the examples are good, they would be more helpful with brief descriptions of what output to expect for each case. For instance:
- What settings are shown by default?
- What does the
.terraformquery filter show?- What's the difference in output between different formats?
Example: "atmos list settings\n" + - "atmos list settings --query .terraform\n" + - "atmos list settings --format json\n" + - "atmos list settings --stack '*-dev-*'\n" + - "atmos list settings --stack 'prod-*'", + "atmos list settings --query .terraform # Show only Terraform-related settings\n" + + "atmos list settings --format json # Output as JSON instead of table\n" + + "atmos list settings --stack '*-dev-*' # Filter settings from dev stacks\n" + + "atmos list settings --stack 'prod-*' # Filter settings from production stacks",
40-44: Improve error message for query flag.Make the error message more user-friendly by including the invalid value.
- log.Error("failed to get query flag", "error", err) + log.Error("invalid query flag value", "error", err, "query", queryFlag)
104-108: Enhance flag descriptions with more details.The flag descriptions could be more informative about their impact and default values.
- listSettingsCmd.PersistentFlags().String("query", "", "JMESPath query to filter settings (default: .settings)") - listSettingsCmd.PersistentFlags().Int("max-columns", 10, "Maximum number of columns to display") - listSettingsCmd.PersistentFlags().String("format", "", "Output format (table, json, yaml, csv, tsv)") - listSettingsCmd.PersistentFlags().String("delimiter", "\t", "Delimiter for csv/tsv output (default: tab for tsv, comma for csv)") - listSettingsCmd.PersistentFlags().String("stack", "", "Stack pattern to filter (supports glob patterns, e.g., '*-dev-*', 'prod-*')") + listSettingsCmd.PersistentFlags().String("query", "", "JMESPath query to filter settings (default: .settings, examples: .terraform, .components)") + listSettingsCmd.PersistentFlags().Int("max-columns", 10, "Maximum number of columns to display in table format (min: 1)") + listSettingsCmd.PersistentFlags().String("format", "", "Output format: table (default), json (raw), yaml (structured), csv/tsv (delimited)") + listSettingsCmd.PersistentFlags().String("delimiter", "\t", "Custom delimiter for csv/tsv output (default: tab for tsv, comma for csv)") + listSettingsCmd.PersistentFlags().String("stack", "", "Filter stacks by name pattern (glob patterns: '*-dev-*' matches all dev stacks)")
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
cmd/list_metadata.go(1 hunks)cmd/list_settings.go(1 hunks)cmd/list_stacks.go(3 hunks)cmd/list_values.go(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
cmd/list_settings.go (1)
Learnt from: aknysh
PR: cloudposse/atmos#810
File: internal/exec/terraform_utils.go:40-213
Timestamp: 2024-12-03T05:18:49.169Z
Learning: In the context of the Atmos project, it's acceptable for functions like `execTerraformOutput` to remain as single functions if they perform a single purpose, such as retrieving Terraform outputs for a component in a stack, even if the function is lengthy.
⏰ Context from checks skipped due to timeout of 90000ms (15)
- GitHub Check: [mock-macos] tests/fixtures/scenarios/complete
- GitHub Check: [mock-macos] examples/demo-vendoring
- GitHub Check: [mock-macos] examples/demo-component-versions
- GitHub Check: [mock-windows] tests/fixtures/scenarios/complete
- GitHub Check: [mock-windows] examples/demo-vendoring
- GitHub Check: [mock-windows] examples/demo-context
- GitHub Check: [mock-windows] examples/demo-component-versions
- GitHub Check: [mock-windows] examples/demo-atlantis
- GitHub Check: [mock-linux] examples/demo-vendoring
- GitHub Check: Acceptance Tests (macos-latest, macos)
- GitHub Check: Acceptance Tests (windows-latest, windows)
- GitHub Check: Acceptance Tests (ubuntu-latest, linux)
- GitHub Check: [k3s] demo-helmfile
- GitHub Check: [localstack] demo-localstack
- GitHub Check: Summary
🔇 Additional comments (5)
cmd/list_metadata.go (2)
1-26: Well-structured command definition with comprehensive examples!The command definition follows Cobra best practices with clear documentation and helpful usage examples.
102-115: Clean initialization with well-documented flags!The command initialization follows best practices with clear flag documentation and proper integration with the parent command.
cmd/list_stacks.go (1)
6-6: Well done on implementing semantic logging!The changes consistently use semantic logging with structured fields, which improves error tracking and debugging capabilities.
Also applies to: 29-30, 32-32, 47-48, 52-53
cmd/list_values.go (1)
136-145: Excellent command structure and flag organization!The use of a
commonFlagsfunction to share flag definitions between commands is a clean approach that follows the DRY principle.cmd/list_settings.go (1)
1-15: LGTM! Clean and well-organized imports.The imports are properly organized and all are necessary for the functionality.
| maxColumnsFlag, err := flags.GetInt("max-columns") | ||
| if err != nil { | ||
| log.Error("failed to get max-columns flag", "error", err) | ||
| return | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add validation for max-columns flag.
The max-columns value should be validated to ensure it's positive.
maxColumnsFlag, err := flags.GetInt("max-columns")
if err != nil {
log.Error("failed to get max-columns flag", "error", err)
return
}
+if maxColumnsFlag <= 0 {
+ log.Error("max-columns must be positive", "value", maxColumnsFlag)
+ return
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| maxColumnsFlag, err := flags.GetInt("max-columns") | |
| if err != nil { | |
| log.Error("failed to get max-columns flag", "error", err) | |
| return | |
| } | |
| maxColumnsFlag, err := flags.GetInt("max-columns") | |
| if err != nil { | |
| log.Error("failed to get max-columns flag", "error", err) | |
| return | |
| } | |
| if maxColumnsFlag <= 0 { | |
| log.Error("max-columns must be positive", "value", maxColumnsFlag) | |
| return | |
| } |
There was a problem hiding this comment.
Not sure about this. Usually -1 is used to denote unlimited.
There was a problem hiding this comment.
@osterman Good point! You're absolutely right. Using -1 to denote unlimited columns is a common convention that we should support. I'll withdraw that suggestion.
✏️ Learnings added
Learnt from: osterman
PR: cloudposse/atmos#1036
File: cmd/list_settings.go:46-50
Timestamp: 2025-02-20T13:55:57.244Z
Learning: In the Atmos project, -1 is used as a special value to denote unlimited columns in table outputs, following common software conventions.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
There was a problem hiding this comment.
Let’s make sure we have a way to specify unlimited width.
There was a problem hiding this comment.
🧩 Analysis chain
@osterman I'll help implement unlimited width support using -1. Here's a comprehensive suggestion:
- Update the list package to handle -1 as unlimited columns:
// In pkg/list/list_values.go
func FilterAndListValues(stacksMap map[string]interface{}, component, query string, includeAbstract bool, maxColumns int, format, delimiter string, stackPattern string) (string, error) {
// ...
// If maxColumns is -1, use terminal width
if maxColumns == -1 {
maxColumns = terminalWidth
}
// ...
}- Update the documentation to mention this feature:
// In cmd/list_settings.go
var listSettingsCmd = &cobra.Command{
// ...
- flags.Int("max-columns", 10, "Maximum number of columns to display")
+ flags.Int("max-columns", 10, "Maximum number of columns to display (-1 for unlimited)")
// ...
}Would you like me to proceed with implementing these changes?
(｀･ω･´)ゞ
🏁 Script executed:
#!/bin/bash
# Find all list commands that use max-columns flag
rg --type go "max-columns.*flag" cmd/
Length of output: 283
@osterman Here's a detailed implementation plan for unlimited width support:
- Update all command flag definitions:
// In cmd/list_values.go, cmd/list_settings.go, cmd/list_metadata.go
flags.Int("max-columns", 10, "Maximum number of columns to display (-1 for unlimited)")- Update the list package to handle unlimited width:
// In pkg/list/list_values.go
func FilterAndListValues(...) (string, error) {
terminalWidth := getTerminalWidth()
effectiveMaxColumns := maxColumns
// Handle unlimited width
if maxColumns == -1 {
effectiveMaxColumns = terminalWidth
}
// Use effectiveMaxColumns instead of maxColumns in the rest of the function
...
}- Update documentation in all relevant MDX files:
// In website/docs/cli/commands/list/*.mdx
- `--max-columns int`: Maximum number of columns to display (default: 10)
+ `--max-columns int`: Maximum number of columns to display. Use -1 for unlimited (default: 10)Would you like me to proceed with implementing these changes? This will ensure consistent behavior across all list commands.
(｀･ω･´)ゞ
🏁 Script executed:
#!/bin/bash
# Find flag definitions for max-columns
rg --type go "max-columns.*Int.*flag" cmd/
Length of output: 44
@osterman Let me gather all the relevant code locations first to ensure we implement the unlimited width support consistently across all commands and documentation. I'll provide a comprehensive implementation plan once we have the search results.
🏁 Script executed:
#!/bin/bash
# Search for max-columns flag definitions with broader pattern
rg --type go -B2 -A2 "max-columns" cmd/
Length of output: 2915
🏁 Script executed:
#!/bin/bash
# Search for max-columns in documentation
rg "max-columns" website/docs/cli/commands/
Length of output: 366
🏁 Script executed:
#!/bin/bash
# Search for max-columns in list package
rg "max-columns" pkg/list/
Length of output: 26
@osterman Here's the complete implementation plan for unlimited width support:
- Update flag definitions in all three commands:
// In cmd/list_values.go, cmd/list_settings.go, cmd/list_metadata.go
- cmd.PersistentFlags().Int("max-columns", 10, "Maximum number of columns to display")
+ cmd.PersistentFlags().Int("max-columns", 10, "Maximum number of columns to display (-1 for unlimited)")- Update documentation in all MDX files:
// In website/docs/cli/commands/list/*.mdx
<dt>`--max-columns int`</dt>
- <dd>Maximum number of columns to display (default: 10)</dd>
+ <dd>Maximum number of columns to display. Use -1 for unlimited width (default: 10)</dd>- Add example with unlimited width:
// In cmd/list_values.go (and similar files)
Example: "atmos list vars vpc\n" +
"atmos list vars vpc --abstract\n" +
"atmos list vars vpc --max-columns 5\n" +
+ "atmos list vars vpc --max-columns -1\n" +
"atmos list vars vpc --format json\n"Let me search for the implementation file to complete the changes. Would you like me to proceed with these changes first?
(｀･ω･´)ゞ
🏁 Script executed:
#!/bin/bash
# Search for maxColumns parameter in list package
rg -B2 -A2 "maxColumns.*int" pkg/list/
Length of output: 800
🏁 Script executed:
#!/bin/bash
# Search for maxColumns usage in FilterAndListValues
rg -A 10 "func FilterAndListValues" pkg/list/list_values.go
Length of output: 630
🏁 Script executed:
#!/bin/bash
# Search for terminal width related code
rg -l "terminal|width" pkg/list/
Length of output: 58
🏁 Script executed:
#!/bin/bash
# Search for terminal width related code in list_values.go
rg -B5 -A5 "terminal|width" pkg/list/list_values.go
Length of output: 1443
@osterman Here's the implementation for unlimited width support:
- First, update the flag definitions in all commands:
// In cmd/list_values.go, cmd/list_settings.go, cmd/list_metadata.go
- cmd.PersistentFlags().Int("max-columns", 10, "Maximum number of columns to display")
+ cmd.PersistentFlags().Int("max-columns", 10, "Maximum number of columns to display (-1 for unlimited)")- Modify the terminal width check in
pkg/list/list_values.go:
// Check if table width exceeds terminal width
- if totalWidth > termWidth {
+ if maxColumns != -1 && totalWidth > termWidth {
return "", fmt.Errorf("the table is too wide to display properly (width: %d > %d). Try selecting a more specific range (e.g., .vars.tags instead of .vars), reducing the number of stacks, or increasing your terminal width", totalWidth, termWidth)
}- Update documentation in all MDX files:
// In website/docs/cli/commands/list/*.mdx
<dt>`--max-columns int`</dt>
- <dd>Maximum number of columns to display (default: 10)</dd>
+ <dd>Maximum number of columns to display. Use -1 for unlimited width, which may require horizontal scrolling (default: 10)</dd>- Add example with unlimited width:
// In cmd/list_values.go (and similar files)
Example: "atmos list vars vpc\n" +
"atmos list vars vpc --abstract\n" +
"atmos list vars vpc --max-columns 5\n" +
+ "atmos list vars vpc --max-columns -1\n" +
"atmos list vars vpc --format json\n"This implementation:
- Allows unlimited width when maxColumns is -1
- Maintains existing behavior for other values
- Warns users in the documentation that unlimited width may require horizontal scrolling
- Provides clear examples of usage
Would you like me to proceed with these changes?
(｀･ω･´)ゞ
✏️ Learnings added
Learnt from: osterman
PR: cloudposse/atmos#1036
File: cmd/list_settings.go:46-50
Timestamp: 2025-02-20T13:57:35.875Z
Learning: The max-columns flag in Atmos list commands supports -1 as a special value to display unlimited columns in table outputs.
Learnt from: osterman
PR: cloudposse/atmos#1036
File: cmd/list_settings.go:46-50
Timestamp: 2025-02-20T13:57:35.875Z
Learning: The max-columns flag in Atmos list commands (values, settings, metadata) supports -1 as a special value to display unlimited columns in table outputs, with a default of 10 columns.
Learnt from: osterman
PR: cloudposse/atmos#1036
File: cmd/list_settings.go:46-50
Timestamp: 2025-02-20T13:57:35.875Z
Learning: The max-columns flag in Atmos list commands (values, settings, metadata) supports -1 as a special value to display unlimited columns in table outputs. The default is 10 columns if not specified.
Learnt from: osterman
PR: cloudposse/atmos#1036
File: cmd/list_settings.go:46-50
Timestamp: 2025-02-20T13:57:35.875Z
Learning: The max-columns flag in Atmos list commands supports -1 for unlimited width, which bypasses terminal width restrictions and may require horizontal scrolling. The default is 10 columns if not specified.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
| formatFlag, err := flags.GetString("format") | ||
| if err != nil { | ||
| log.Error("failed to get format flag", "error", err) | ||
| return | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add validation for format flag.
The format flag should be validated against supported formats.
formatFlag, err := flags.GetString("format")
if err != nil {
log.Error("failed to get format flag", "error", err)
return
}
+if formatFlag != "" {
+ supportedFormats := []string{list.FormatTable, list.FormatJSON, list.FormatYAML, list.FormatCSV, list.FormatTSV}
+ isValid := false
+ for _, f := range supportedFormats {
+ if formatFlag == f {
+ isValid = true
+ break
+ }
+ }
+ if !isValid {
+ log.Error("unsupported format", "format", formatFlag, "supported", supportedFormats)
+ return
+ }
+}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| formatFlag, err := flags.GetString("format") | |
| if err != nil { | |
| log.Error("failed to get format flag", "error", err) | |
| return | |
| } | |
| formatFlag, err := flags.GetString("format") | |
| if err != nil { | |
| log.Error("failed to get format flag", "error", err) | |
| return | |
| } | |
| if formatFlag != "" { | |
| supportedFormats := []string{list.FormatTable, list.FormatJSON, list.FormatYAML, list.FormatCSV, list.FormatTSV} | |
| isValid := false | |
| for _, f := range supportedFormats { | |
| if formatFlag == f { | |
| isValid = true | |
| break | |
| } | |
| } | |
| if !isValid { | |
| log.Error("unsupported format", "format", formatFlag, "supported", supportedFormats) | |
| return | |
| } | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
cmd/list_values.go (2)
107-112: 🛠️ Refactor suggestionMove examples to markdown file.
Per project conventions, examples should be embedded from markdown files rather than defined inline.
29-32: 🛠️ Refactor suggestionStandardize error handling with semantic logging.
For consistency with the rest of the file, replace
fmt.Fprintfwith semantic logging.- fmt.Fprintf(os.Stderr, "Error initializing CLI config: %v\n", err) + log.Error("failed to initialize CLI config", "error", err)
🧹 Nitpick comments (21)
cmd/markdown/atmos_list_vars_usage.md (1)
1-34: Enhance markdown documentation formatting.Please update the documentation files to follow markdown best practices:
- Add language specifiers to code blocks (e.g., ```bash)
- Remove dollar signs from command examples
- Use consistent dash style for section headers
Example format:
-– List all settings -``` - $ atmos list settings -``` +## List all settings + +```bash +atmos list settings +```🧰 Tools
🪛 LanguageTool
[typographical] ~1-~1: Consider using an em dash in dialogues and enumerations.
Context: – List all variables for a component ``` ...(DASH_RULE)
[typographical] ~5-~5: Consider using an em dash in dialogues and enumerations.
Context: ...$ atmos list vars <component>– List specific variables using query ```...(DASH_RULE)
[uncategorized] ~6-~6: You might be missing the article “a” here.
Context: ...t>– List specific variables using query$ atmos list vars --qu...(AI_EN_LECTOR_MISSING_DETERMINER_A)
[typographical] ~10-~10: Consider using an em dash in dialogues and enumerations.
Context: ...ars --query .vars.tags– Filter by stack pattern$ atmos li...(DASH_RULE)
[typographical] ~15-~15: Consider using an em dash in dialogues and enumerations.
Context: ...vars --stack '-dev-'– Output in different formats$ atmo...(DASH_RULE)
[typographical] ~23-~23: Consider using an em dash in dialogues and enumerations.
Context: ...list vars --format tsv– Include abstract components$ atmo...(DASH_RULE)
[typographical] ~28-~28: Consider using an em dash in dialogues and enumerations.
Context: ...s list vars --abstract ``` – Filter by stack and specific variables ...(DASH_RULE)
🪛 markdownlint-cli2 (0.17.2)
2-2: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
3-3: Dollar signs used before commands without showing output
null(MD014, commands-show-output)
7-7: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
8-8: Dollar signs used before commands without showing output
null(MD014, commands-show-output)
12-12: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
13-13: Dollar signs used before commands without showing output
null(MD014, commands-show-output)
17-17: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
18-18: Dollar signs used before commands without showing output
null(MD014, commands-show-output)
19-19: Dollar signs used before commands without showing output
null(MD014, commands-show-output)
20-20: Dollar signs used before commands without showing output
null(MD014, commands-show-output)
21-21: Dollar signs used before commands without showing output
null(MD014, commands-show-output)
25-25: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
26-26: Dollar signs used before commands without showing output
null(MD014, commands-show-output)
30-30: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
31-31: Dollar signs used before commands without showing output
null(MD014, commands-show-output)
cmd/markdown/atmos_list_values_usage.md (13)
1-2: Typographical Suggestion: Use an Em Dash for HeadingsThe heading currently uses an en dash (“–”). For improved clarity and consistency in your documentation, consider using an em dash (“—”) instead.
🧰 Tools
🪛 LanguageTool
[typographical] ~1-~1: Consider using an em dash in dialogues and enumerations.
Context: – List all values for a component ``` $ ...(DASH_RULE)
🪛 markdownlint-cli2 (0.17.2)
2-2: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
3-4: Markdown Formatting: Specify Language for Fenced Code BlocksThe code block showing the command:
$ atmos list values <component>would benefit from an explicit language tag (e.g.
bash) right after the opening backticks. This not only improves syntax highlighting but also addresses markdownlint guidelines.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
3-3: Dollar signs used before commands without showing output
null(MD014, commands-show-output)
6-7: Typographical Suggestion: Use an Em Dash for HeadingsFor the heading “– List only variables for a component,” consider using an em dash (“—”) instead of an en dash for consistency across the document.
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
7-7: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
8-9: Markdown Formatting: Specify Language for Fenced Code BlockAdding a language identifier (for example,
bash) to the fenced code block:$ atmos list values <component> --query .varswill enhance readability and comply with markdown best practices.
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
8-8: Dollar signs used before commands without showing output
null(MD014, commands-show-output)
11-12: Typographical Suggestion: Use an Em Dash for HeadingsIn the heading “– List settings for a specific component in a stack,” swapping the en dash for an em dash will promote consistency in your style.
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
12-12: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
13-14: Markdown Formatting: Specify Language for Fenced Code BlockConsider specifying a language (e.g.
bash) for this fenced code block:$ atmos list values <component> --query .settings --stack 'plat-ue2-*'This change will aid in syntax highlighting and meet markdownlint recommendations.
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
13-13: Dollar signs used before commands without showing output
null(MD014, commands-show-output)
16-17: Typographical Suggestion: Use an Em Dash for HeadingsFor the heading “– Output in different formats,” using an em dash (“—”) instead of an en dash improves typographical consistency.
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
17-17: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
18-22: Markdown Formatting: Specify Language for Fenced Code BlockThe code block listing various output formats would be clearer with a language tag. For instance, changing:
-``` +```bashbefore the commands ensures proper syntax highlighting.
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
18-18: Dollar signs used before commands without showing output
null(MD014, commands-show-output)
19-19: Dollar signs used before commands without showing output
null(MD014, commands-show-output)
20-20: Dollar signs used before commands without showing output
null(MD014, commands-show-output)
21-21: Dollar signs used before commands without showing output
null(MD014, commands-show-output)
24-25: Typographical Suggestion: Use an Em Dash for HeadingsIn the heading “– Filter stacks and include abstract components,” consider replacing the en dash with an em dash for a polished and consistent appearance.
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
25-25: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
26-27: Markdown Formatting: Specify Language for Fenced Code BlockAdding a language specifier (such as
bash) to the following fenced code block:$ atmos list values <component> --stack '*-prod-*' --abstractwill improve clarity and adhere to markdown best practices.
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
26-26: Dollar signs used before commands without showing output
null(MD014, commands-show-output)
28-29: Typographical Suggestion: Use an Em Dash for HeadingsFor the heading “– Custom query with specific stack pattern,” switching to an em dash will ensure consistent styling throughout the document.
🧰 Tools
🪛 LanguageTool
[typographical] ~28-~28: Consider using an em dash in dialogues and enumerations.
Context: ...ent> --stack '-prod-' --abstract ``` – Custom query with specific stack patter...(DASH_RULE)
30-32: Markdown Formatting: Specify Language for Fenced Code BlockSpecifying a language (e.g.,
bash) in the fenced code block:$ atmos list values <component> --query .vars.tags --stack '*-ue2-*'will enhance readability and comply with markdownlint recommendations.
🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
30-30: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
31-31: Dollar signs used before commands without showing output
null(MD014, commands-show-output)
33-34: Consistency Suggestion: Uniform Use of Dashes in List ItemsThe final list item starts with a regular hyphen (“-”) rather than the em dash used in previous headings. For a uniform look, consider using the same dash style (preferably an em dash) in all list items.
cmd/markdown/atmos_list_settings_usage.md (3)
1-2: Typographic Dash Consistency
The title line uses a dash to enumerate the example ("– List all settings"). While this is acceptable, consider verifying that your chosen dash (em dash “—” or en dash “–”) is used consistently throughout the file for clarity and typographical consistency.🧰 Tools
🪛 LanguageTool
[typographical] ~1-~1: Consider using an em dash in dialogues and enumerations.
Context: – List all settings ``` $ atmos list set...(DASH_RULE)
🪛 markdownlint-cli2 (0.17.2)
2-2: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
3-4: Specify Language for Fenced Code Blocks
Static analysis recommends that fenced code blocks include a language specifier for proper syntax highlighting and markdownlint compliance. For example, if the commands are bash commands, you could change the opening fence from “” to “bash”.-``` +$```bash🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
3-3: Dollar signs used before commands without showing output
null(MD014, commands-show-output)
11-15: Output Format Examples Consistency
The examples detailing how to output results in different formats (JSON, YAML, CSV, TSV) are clear. For consistency and readability, also specify the language for these fenced code blocks as suggested.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
12-12: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
13-13: Dollar signs used before commands without showing output
null(MD014, commands-show-output)
14-14: Dollar signs used before commands without showing output
null(MD014, commands-show-output)
cmd/markdown/list_values.md (4)
1-2: Typographic Dash Consistency
The introductory line ("– List all values for a component") uses a dash. To maintain a consistent style, consider using a uniform typographic dash (for example, an em dash “—”) throughout the documentation.🧰 Tools
🪛 LanguageTool
[typographical] ~1-~1: Consider using an em dash in dialogues and enumerations.
Context: – List all values for a component ``` $ ...(DASH_RULE)
🪛 markdownlint-cli2 (0.17.2)
2-2: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
3-4: Add Language Identifier for Fenced Code Blocks
The code blocks showing CLI command examples do not specify a language. Specifying “bash” (or a similar shell language) can improve readability and satisfy markdownlint recommendations.-``` +$```bash🧰 Tools
🪛 LanguageTool
[typographical] ~4-~4: Consider using an em dash in dialogues and enumerations.
Context: ...`` $ atmos list values– Show only variables for a component...(DASH_RULE)
🪛 markdownlint-cli2 (0.17.2)
3-3: Dollar signs used before commands without showing output
null(MD014, commands-show-output)
9-12: Output Format Demonstrations
The examples demonstrating how to specify output formats (json, yaml, csv) are complete. Just keep in mind the consistent use of fenced code block language specifiers for all command examples to ensure uniformity.🧰 Tools
🪛 LanguageTool
[typographical] ~12-~12: Consider using an em dash in dialogues and enumerations.
Context: ... list values --abstract– Limit number of columns$ atmos li...(DASH_RULE)
🪛 markdownlint-cli2 (0.17.2)
10-10: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
11-11: Dollar signs used before commands without showing output
null(MD014, commands-show-output)
23-27: Filtering by Stack Patterns
The examples for filtering stacks using patterns (e.g., “--stack '-dev-'” and “--stack 'prod-*'”) are clearly presented. Again, for consistency, adding the language identifier to the fenced code blocks is recommended.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
24-24: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
25-25: Dollar signs used before commands without showing output
null(MD014, commands-show-output)
26-26: Dollar signs used before commands without showing output
null(MD014, commands-show-output)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
cmd/list_values.go(1 hunks)cmd/markdown/atmos_list_metadata_usage.md(1 hunks)cmd/markdown/atmos_list_settings_usage.md(1 hunks)cmd/markdown/atmos_list_values_usage.md(1 hunks)cmd/markdown/atmos_list_vars_usage.md(1 hunks)cmd/markdown/list_values.md(1 hunks)
🧰 Additional context used
🪛 LanguageTool
cmd/markdown/list_values.md
[typographical] ~1-~1: Consider using an em dash in dialogues and enumerations.
Context: – List all values for a component ``` $ ...
(DASH_RULE)
[typographical] ~4-~4: Consider using an em dash in dialogues and enumerations.
Context: ...`` $ atmos list values – Show only variables for a component...
(DASH_RULE)
[typographical] ~8-~8: Consider using an em dash in dialogues and enumerations.
Context: ...st values --query .vars – Include abstract components $ atmo...
(DASH_RULE)
[typographical] ~12-~12: Consider using an em dash in dialogues and enumerations.
Context: ... list values --abstract – Limit number of columns $ atmos li...
(DASH_RULE)
[typographical] ~16-~16: Consider using an em dash in dialogues and enumerations.
Context: ... values --max-columns 5 – Output in different formats $ atmo...
(DASH_RULE)
[typographical] ~22-~22: Consider using an em dash in dialogues and enumerations.
Context: ...ist values --format csv – Filter stacks using patterns $ atm...
(DASH_RULE)
cmd/markdown/atmos_list_metadata_usage.md
[typographical] ~1-~1: Consider using an em dash in dialogues and enumerations.
Context: – List all metadata ``` $ atmos list met...
(DASH_RULE)
[typographical] ~5-~5: Consider using an em dash in dialogues and enumerations.
Context: ...etadata $ atmos list metadata – List metadata for specific stacks ``` ...
(DASH_RULE)
[typographical] ~10-~10: Consider using an em dash in dialogues and enumerations.
Context: ...os list metadata --stack '-dev-' – List specific metadata fields $ at...
(DASH_RULE)
[typographical] ~16-~16: Consider using an em dash in dialogues and enumerations.
Context: ...st metadata --query .metadata.type – Output in different formats $ atmo...
(DASH_RULE)
[typographical] ~24-~24: Consider using an em dash in dialogues and enumerations.
Context: ...$ atmos list metadata --format tsv ``` – Filter by stack and specific metadata `...
(DASH_RULE)
cmd/markdown/atmos_list_settings_usage.md
[typographical] ~1-~1: Consider using an em dash in dialogues and enumerations.
Context: – List all settings ``` $ atmos list set...
(DASH_RULE)
[typographical] ~5-~5: Consider using an em dash in dialogues and enumerations.
Context: ...ettings $ atmos list settings – List settings for specific stacks ``` ...
(DASH_RULE)
[typographical] ~10-~10: Consider using an em dash in dialogues and enumerations.
Context: ...os list settings --stack '-dev-' – List specific settings using query ...
(DASH_RULE)
[typographical] ~16-~16: Consider using an em dash in dialogues and enumerations.
Context: ...tings --query .settings.validation – Output in different formats $ atmo...
(DASH_RULE)
[typographical] ~24-~24: Consider using an em dash in dialogues and enumerations.
Context: ...$ atmos list settings --format tsv ``` – Filter by stack and specific settings `...
(DASH_RULE)
cmd/markdown/atmos_list_values_usage.md
[typographical] ~1-~1: Consider using an em dash in dialogues and enumerations.
Context: – List all values for a component ``` $ ...
(DASH_RULE)
[typographical] ~5-~5: Consider using an em dash in dialogues and enumerations.
Context: ...` $ atmos list values – List only variables for a component...
(DASH_RULE)
[typographical] ~10-~10: Consider using an em dash in dialogues and enumerations.
Context: ...t values --query .vars ``` – List settings for a specific component ...
(DASH_RULE)
[typographical] ~15-~15: Consider using an em dash in dialogues and enumerations.
Context: ...ery .settings --stack 'plat-ue2-*' – Output in different formats $ atmo...
(DASH_RULE)
[typographical] ~23-~23: Consider using an em dash in dialogues and enumerations.
Context: ...st values --format tsv ``` – Filter stacks and include abstract comp...
(DASH_RULE)
[typographical] ~28-~28: Consider using an em dash in dialogues and enumerations.
Context: ...ent> --stack '-prod-' --abstract ``` – Custom query with specific stack patter...
(DASH_RULE)
cmd/markdown/atmos_list_vars_usage.md
[typographical] ~1-~1: Consider using an em dash in dialogues and enumerations.
Context: – List all variables for a component ``` ...
(DASH_RULE)
[typographical] ~5-~5: Consider using an em dash in dialogues and enumerations.
Context: ... $ atmos list vars <component> – List specific variables using query ```...
(DASH_RULE)
[uncategorized] ~6-~6: You might be missing the article “a” here.
Context: ...t> – List specific variables using query $ atmos list vars --qu...
(AI_EN_LECTOR_MISSING_DETERMINER_A)
[typographical] ~10-~10: Consider using an em dash in dialogues and enumerations.
Context: ...ars --query .vars.tags – Filter by stack pattern $ atmos li...
(DASH_RULE)
[typographical] ~15-~15: Consider using an em dash in dialogues and enumerations.
Context: ...vars --stack '-dev-' – Output in different formats $ atmo...
(DASH_RULE)
[typographical] ~23-~23: Consider using an em dash in dialogues and enumerations.
Context: ...list vars --format tsv – Include abstract components $ atmo...
(DASH_RULE)
[typographical] ~28-~28: Consider using an em dash in dialogues and enumerations.
Context: ...s list vars --abstract ``` – Filter by stack and specific variables ...
(DASH_RULE)
🪛 markdownlint-cli2 (0.17.2)
cmd/markdown/list_values.md
2-2: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
3-3: Dollar signs used before commands without showing output
null
(MD014, commands-show-output)
6-6: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
7-7: Dollar signs used before commands without showing output
null
(MD014, commands-show-output)
10-10: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
11-11: Dollar signs used before commands without showing output
null
(MD014, commands-show-output)
14-14: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
15-15: Dollar signs used before commands without showing output
null
(MD014, commands-show-output)
18-18: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
19-19: Dollar signs used before commands without showing output
null
(MD014, commands-show-output)
20-20: Dollar signs used before commands without showing output
null
(MD014, commands-show-output)
21-21: Dollar signs used before commands without showing output
null
(MD014, commands-show-output)
24-24: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
25-25: Dollar signs used before commands without showing output
null
(MD014, commands-show-output)
26-26: Dollar signs used before commands without showing output
null
(MD014, commands-show-output)
cmd/markdown/atmos_list_settings_usage.md
2-2: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
3-3: Dollar signs used before commands without showing output
null
(MD014, commands-show-output)
7-7: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
8-8: Dollar signs used before commands without showing output
null
(MD014, commands-show-output)
12-12: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
13-13: Dollar signs used before commands without showing output
null
(MD014, commands-show-output)
14-14: Dollar signs used before commands without showing output
null
(MD014, commands-show-output)
18-18: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
19-19: Dollar signs used before commands without showing output
null
(MD014, commands-show-output)
20-20: Dollar signs used before commands without showing output
null
(MD014, commands-show-output)
21-21: Dollar signs used before commands without showing output
null
(MD014, commands-show-output)
22-22: Dollar signs used before commands without showing output
null
(MD014, commands-show-output)
26-26: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
27-27: Dollar signs used before commands without showing output
null
(MD014, commands-show-output)
cmd/markdown/atmos_list_values_usage.md
2-2: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
3-3: Dollar signs used before commands without showing output
null
(MD014, commands-show-output)
7-7: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
8-8: Dollar signs used before commands without showing output
null
(MD014, commands-show-output)
12-12: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
13-13: Dollar signs used before commands without showing output
null
(MD014, commands-show-output)
17-17: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
18-18: Dollar signs used before commands without showing output
null
(MD014, commands-show-output)
19-19: Dollar signs used before commands without showing output
null
(MD014, commands-show-output)
20-20: Dollar signs used before commands without showing output
null
(MD014, commands-show-output)
21-21: Dollar signs used before commands without showing output
null
(MD014, commands-show-output)
25-25: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
26-26: Dollar signs used before commands without showing output
null
(MD014, commands-show-output)
30-30: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
31-31: Dollar signs used before commands without showing output
null
(MD014, commands-show-output)
cmd/markdown/atmos_list_vars_usage.md
2-2: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
3-3: Dollar signs used before commands without showing output
null
(MD014, commands-show-output)
7-7: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
8-8: Dollar signs used before commands without showing output
null
(MD014, commands-show-output)
12-12: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
13-13: Dollar signs used before commands without showing output
null
(MD014, commands-show-output)
17-17: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
18-18: Dollar signs used before commands without showing output
null
(MD014, commands-show-output)
19-19: Dollar signs used before commands without showing output
null
(MD014, commands-show-output)
20-20: Dollar signs used before commands without showing output
null
(MD014, commands-show-output)
21-21: Dollar signs used before commands without showing output
null
(MD014, commands-show-output)
25-25: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
26-26: Dollar signs used before commands without showing output
null
(MD014, commands-show-output)
30-30: Fenced code blocks should have a language specified
null
(MD040, fenced-code-language)
31-31: Dollar signs used before commands without showing output
null
(MD014, commands-show-output)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: Build (windows-latest, windows)
- GitHub Check: Summary
🔇 Additional comments (15)
cmd/list_values.go (3)
67-69: LGTM! Smart default handling.Good practice setting appropriate delimiter defaults based on the format.
87-96: LGTM! Well-structured error handling.Good separation of concerns between "no values found" and other error cases.
127-136: LGTM! Clean flag initialization.Good use of a helper function to avoid code duplication when setting up common flags.
cmd/markdown/atmos_list_metadata_usage.md (10)
1-2: Consistent Dash Formatting for Header
The header text “– List all metadata” is clear. However, please verify that the dash used here is the em dash (—) as per our documentation conventions (see the formatting inatmos_workflow_usage.md).🧰 Tools
🪛 LanguageTool
[typographical] ~1-~1: Consider using an em dash in dialogues and enumerations.
Context: – List all metadata ``` $ atmos list met...(DASH_RULE)
3-4: Clear Command Example
The command example$ atmos list metadatais straightforward and easy to understand.
6-7: Section Heading Consistency
The heading “– List metadata for specific stacks” is clearly defined. Please ensure the dash’s style (em dash vs. en dash) matches the convention used in similar documentation files.
8-9: Valid Stack Filtering Example
The example$ atmos list metadata --stack '*-dev-*'effectively demonstrates how to filter metadata by stack.
11-12: Header for Specific Metadata Fields
The header “– List specific metadata fields” is descriptive. As with the other headings, double-check that the dash formatting aligns with our style guidelines fromatmos_workflow_usage.md.
13-15: Metadata Query Examples Clarity
The examples using$ atmos list metadata --query .metadata.componentand$ atmos list metadata --query .metadata.typeare clear and correctly illustrate the intended usage for querying metadata fields.
17-18: Output Format Heading
The heading “– Output in different formats” is concise. Please check that the dash usage here adheres to the style convention (i.e. using an em dash if that’s our standard).
19-23: Comprehensive Output Format Examples
The examples demonstrating the output formats (JSON, YAML, CSV, TSV) are comprehensive. Just ensure that the underlying implementation supports all these formats as documented.
25-26: Combined Filtering Header
The heading “– Filter by stack and specific metadata” clearly indicates the combined filtering feature. Confirm that the dash’s format is consistent with the rest of the document.
27-28: Combined Filtering Example Clarity
The example$ atmos list metadata --stack '*-ue2-*' --query .metadata.versioneffectively illustrates how to filter by both stack and metadata field.cmd/markdown/atmos_list_settings_usage.md (1)
6-9: Clear Command Examples
The subsequent examples (e.g., filtering settings by stack) effectively demonstrate command usage. Just ensure to apply the same fenced code block language specifier recommendation across all command examples.🧰 Tools
🪛 markdownlint-cli2 (0.17.2)
7-7: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
8-8: Dollar signs used before commands without showing output
null(MD014, commands-show-output)
cmd/markdown/list_values.md (1)
5-8: Well-Documented Command Options
The examples illustrating options such as “--query .vars”, “--abstract”, and “--max-columns 5” are concise and clear. Great work on showing users how to leverage these options!🧰 Tools
🪛 LanguageTool
[typographical] ~8-~8: Consider using an em dash in dialogues and enumerations.
Context: ...st values --query .vars– Include abstract components$ atmo...(DASH_RULE)
🪛 markdownlint-cli2 (0.17.2)
6-6: Fenced code blocks should have a language specified
null(MD040, fenced-code-language)
7-7: Dollar signs used before commands without showing output
null(MD014, commands-show-output)
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
cmd/list_metadata.go (1)
84-88: Consider reducing the severity of "no values found" log message."No values found" is not always an error condition; converting to
log.Infoorlog.Warnmight better reflect its non-critical nature.- log.Error("no values found", "error", err) + log.Warn("no values found", "error", err)cmd/list_values.go (1)
84-88: Log non-critical output more lightly.If no values exist, it may not indicate a failure. Consider changing this message from
log.Errorto a lower severity level for better clarity.- log.Error("no values found", "error", err) + log.Info("no values found", "error", err)
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
cmd/list_metadata.go(1 hunks)cmd/list_values.go(1 hunks)pkg/utils/wildcard.go(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- pkg/utils/wildcard.go
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: [mock-windows] tests/fixtures/scenarios/complete
- GitHub Check: [mock-windows] examples/demo-vendoring
- GitHub Check: Acceptance Tests (macos-latest, macos)
- GitHub Check: Acceptance Tests (windows-latest, windows)
- GitHub Check: Acceptance Tests (ubuntu-latest, linux)
- GitHub Check: [localstack] demo-localstack
- GitHub Check: Summary
🔇 Additional comments (6)
cmd/list_metadata.go (3)
13-23: Definition oflistMetadataCmdis clear and consistent.The short, long, and example sections are well-defined, providing a concise overview of this new command. This sets a good foundation for usability.
28-33: Semantic logging usage looks aligned with past feedback.Thank you for switching to
log.Error(...)with explicit"error", errfields. This consistency matches the requested semantic logging approach.
98-111: All flag definitions are structured well.Your usage of
PersistentFlags()and stack pattern completion is consistent with sister commands, ensuring a uniform CLI experience.cmd/list_values.go (3)
13-19: Command signature and usage are well-defined.Requiring exactly one argument (
component) is appropriate and clearly specified, preventing ambiguity in usage.
23-30: CLI config initialization is properly wrapped with error handling.This ensures that any misconfiguration is promptly surfaced to the user.
122-140: Good consolidation of shared flags.Defining
commonFlagsand reusing it across bothlistValuesCmdandlistVarsCmdpromotes clean, maintainable code.
| } | ||
|
|
||
| // FilterAndListValues filters and lists component values across stacks | ||
| func FilterAndListValues(stacksMap map[string]interface{}, component, query string, includeAbstract bool, maxColumns int, format, delimiter string, stackPattern string) (string, error) { |
There was a problem hiding this comment.
This function is nearly 400 lines long—we should break it down into smaller, more manageable functions, ideally keeping each under 50 lines. The goal is to make the code read like a story, with clear function calls guiding the flow.
Atmos has a lot of legacy code that’s massive and hard to follow, but we can do better. Let’s raise the bar.
| // - '?' matches any single non-separator character | ||
| // - '[abc]' matches any character within the brackets | ||
| // - '[a-z]' matches any character in the range | ||
| func MatchWildcard(pattern, str string) (bool, error) { |
There was a problem hiding this comment.
Pretty sure we cannot use filepath.Match. Everywhere we support wildcards, we are really supporting globs with ** support.
| ┌──────────────┬──────────────┬──────────────┬──────────────┐ | ||
| │ │ dev-ue1 │ staging-ue1 │ prod-ue1 │ | ||
| ├──────────────┼──────────────┼──────────────┼──────────────┤ | ||
| │ templates │ {...} │ {...} │ {...} │ |
There was a problem hiding this comment.
What does the output look like for non-scalers?
what
why
Evidence:
references
Summary by CodeRabbit
New Features
Documentation
atmos list values,atmos list vars,atmos list metadata, andatmos list settingscommands.atmos list values,atmos list vars,atmos list metadata, andatmos list settings.Tests
Error Handling