Colorize Atmos Describe Commands when TTY attached #907
Conversation
|
Important Cloud Posse Engineering Team Review RequiredThis pull request modifies files that require Cloud Posse's review. Please be patient, and a core maintainer will review your changes. To expedite this process, reach out to us on Slack in the |
mergify
bot
added
triage
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
📝 WalkthroughWalkthroughThis pull request introduces comprehensive syntax highlighting capabilities for the Atmos CLI, enhancing terminal output readability. The changes span multiple files, including configuration schema, utility functions, and documentation. The implementation utilizes the Chroma library to provide configurable syntax highlighting for YAML and JSON outputs, with customizable options like lexer, style, and formatting. Changes
Assessment against linked issues
Possibly related PRs
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (7)
pkg/utils/yaml_utils.go (1)
44-50: Consider logging highlight errors
You are silently falling back to plain YAML on errors. Logging the error could help troubleshooting.if err != nil { + LogWarning(fmt.Sprintf("Highlight error: %v", err)) PrintMessage(y) return nil }pkg/schema/schema.go (1)
45-52: SyntaxHighlightingSettings structure
Comprehensive fields. Consider if some fields might evolve into their own sub-struct for more flexibility.pkg/utils/json_utils.go (1)
36-43: Fallback to plain JSON
Using plain text on highlight errors matches the YAML approach. Consider logging these errors.if err != nil { + LogWarning(fmt.Sprintf("Highlight error: %v", err)) PrintMessage(prettyJSON.String()) return nil }website/docs/cli/configuration/terminal.mdx (4)
23-41: Consider adding environment variable override information.While the configuration example is clear, adding information about corresponding environment variables would help users who prefer environment-based configuration.
Add environment variable information like this:
settings: terminal: syntax_highlighting: - enabled: true # Enable/disable syntax highlighting + enabled: true # Enable/disable syntax highlighting (ATMOS_TERMINAL_SYNTAX_HIGHLIGHTING_ENABLED)
63-73: Enhance style options documentation, brave documenter!The style options section could benefit from more comprehensive information about available themes and their use cases.
Consider expanding the style options section like this:
Color scheme for syntax highlighting. Available options include: <ul> - <li>`vim`</li> - <li>`monokai`</li> - <li>`github`</li> - <li>`dracula`</li> + <li>`vim` - Classic Vim-like highlighting</li> + <li>`monokai` - High contrast theme, great for dark backgrounds</li> + <li>`github` - GitHub's syntax highlighting theme</li> + <li>`dracula` - Dark theme optimized for readability</li> <li>And many other standard styles</li>
90-109: Add example outputs for better clarity.Consider showing example outputs to help users understand what to expect when using these commands.
Add example outputs like this:
# Display config in YAML format with syntax highlighting atmos describe config -f yaml + +# Example output: +# settings: +# terminal: +# syntax_highlighting: +# enabled: true
133-135: Add performance considerations for theme selection.Consider adding a note about performance implications of different themes, especially in large outputs.
Add performance note like this:
## Supported Themes -Atmos supports a wide range of themes for syntax highlighting. You can find the full list of supported themes [here](https://xyproto.github.io/splash/docs/). +Atmos supports a wide range of themes for syntax highlighting. You can find the full list of supported themes [here](https://xyproto.github.io/splash/docs/). Note that some themes may have different performance characteristics when highlighting large outputs.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
atmos.yaml(1 hunks)pkg/config/config.go(4 hunks)pkg/schema/schema.go(2 hunks)pkg/utils/highlight_utils.go(1 hunks)pkg/utils/json_utils.go(1 hunks)pkg/utils/yaml_utils.go(1 hunks)website/docs/cli/configuration/terminal.mdx(1 hunks)
🔇 Additional comments (18)
pkg/utils/yaml_utils.go (1)
34-43: Check fallback for non-AtmosConfiguration data
These lines gracefully handleAtmosConfigurationtypes. For other data types, the code proceeds without highlighting. If you want to highlight any arbitrary data, consider a separate fallback.pkg/config/config.go (4)
91-105: Default syntax highlighting config
This default configuration looks helpful. Confirm that this aligns with all project needs before proceeding.
129-139: Confirmed loading defaults
LoadingdefaultCliConfigas a base config is consistent and clear.
292-316: Selective overriding
The logic for partial initialization ofSettingsfields is neat. Good approach for layering config.
429-479: Preserving nested values
These lines smartly preserve nested settings in Viper when merging. Consider adding tests for corner cases with deeply nested fields.pkg/schema/schema.go (5)
19-19: Pointer for Settings
Switching to*AtmosSettingsis a good move for optional configuration sections.
40-42: SyntaxHighlightOptions
Straightforward struct for advanced formatting.
54-56: TerminalSettings struct
Allows expansion for future terminal-oriented features.
58-65: Unified Settings struct
Centralizing advanced options, including docs and terminal, improves clarity.
67-67: Type alias
The alias ensures prior references toAtmosSettingsremain valid. Verify reflection-based code if present.pkg/utils/highlight_utils.go (5)
18-30: DefaultHighlightSettings
A straightforward default featuring the “dracula” style. Nice for consistent theming.
32-71: GetHighlightSettings
This method robustly ensures defaults. The fallback steps are well arranged.
73-86: HighlightCode
Checks TTY presence, preventing formatting outside interactive terminals. Good approach.
88-137: HighlightCodeWithConfig
Smartly fetches config-based settings. The fallback to lexers/styles is handy.
139-160: HighlightWriter
A helpful abstraction for streaming highlights. This will keep I/O code tidy.pkg/utils/json_utils.go (1)
27-35: Atmos config type check
ExtractingAtmosConfigurationhere is consistent withyaml_utils.go. Good synergy.website/docs/cli/configuration/terminal.mdx (1)
1-21: Strong documentation structure, warrior!The documentation is well-organized with clear sections, imported components, and key points that effectively guide users through the terminal settings.
atmos.yaml (1)
318-327: Strong configuration structure, but consider adding validation!The terminal settings configuration is well-structured and aligns perfectly with the documentation. However, consider adding validation rules to ensure valid values for lexer, formatter, and style.
Let's verify the supported values:
|
💥 This pull request now has conflicts. Could you fix it @Cerebrovinny? 🙏 |
Co-authored-by: Erik Osterman (CEO @ Cloud Posse) <[email protected]>
Cerebrovinny
force-pushed
the
DEV-2876/feat-tty
branch
from
8ae2aad to
f9f91f8
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (9)
pkg/utils/json_utils.go (1)
36-41: Consider extracting common highlighting logic.The error handling and highlighting logic is duplicated between JSON and YAML utils.
Consider extracting the common highlighting logic into a shared helper function:
+func highlightWithFallback(content string, config schema.AtmosConfiguration) { + highlighted, err := HighlightCodeWithConfig(content, config) + if err != nil { + PrintMessage(content) + return nil + } + PrintMessage(highlighted) + return nil +}website/docs/cli/configuration/terminal.mdx (2)
32-40: Consider adding validation constraints.The configuration example would benefit from comments indicating any validation constraints for these settings (e.g., allowed values for lexer, formatter, style).
133-135: Consider adding theme preview information.While linking to the full list of themes is helpful, consider adding a few screenshots of popular themes to help users make an informed choice.
pkg/utils/highlight_utils.go (4)
18-30: LGTM! Consider adding documentation for the style options.The default settings are well-chosen. The Dracula style provides good contrast for terminal output.
Consider adding a code comment listing the available styles from Chroma (e.g., "monokai", "github", etc.) to help users customize their experience.
73-86: Consider adding context and buffer cleanup.While the implementation is functional, it could be improved:
- Add context.Context parameter for cancellation support
- Consider using bufpool for buffer reuse in high-throughput scenarios
Example improvement:
-func HighlightCode(code string, lexerName string, style string) (string, error) { +func HighlightCode(ctx context.Context, code string, lexerName string, style string) (string, error) { if !term.IsTerminal(int(os.Stdout.Fd())) { return code, nil } - var buf bytes.Buffer + buf := bufPool.Get().(*bytes.Buffer) + buf.Reset() + defer bufPool.Put(buf) + // Check context before expensive operation + select { + case <-ctx.Done(): + return "", ctx.Err() + default: + } err := quick.Highlight(&buf, code, lexerName, "terminal", style)
88-137: Enhance error handling and add context support.The implementation is solid but could be improved:
- Add context support for cancellation
- Wrap errors with more context
- Consider using buffer pool
Example improvement:
-func HighlightCodeWithConfig(code string, config schema.AtmosConfiguration) (string, error) { +func HighlightCodeWithConfig(ctx context.Context, code string, config schema.AtmosConfiguration) (string, error) { if !term.IsTerminal(int(os.Stdout.Fd())) { return code, nil } settings := GetHighlightSettings(config) if !settings.Enabled { return code, nil } + select { + case <-ctx.Done(): + return "", ctx.Err() + default: + } // Get lexer lexer := lexers.Get(settings.Lexer) if lexer == nil { lexer = lexers.Fallback } iterator, err := lexer.Tokenise(nil, code) if err != nil { - return code, err + return code, fmt.Errorf("failed to tokenize code with lexer %s: %w", settings.Lexer, err) }
153-160: Improve error handling in Write method.Consider these enhancements:
- Add context support
- Preserve original error from writer.Write
Example improvement:
-func (h *HighlightWriter) Write(p []byte) (n int, err error) { +func (h *HighlightWriter) Write(ctx context.Context, p []byte) (n int, err error) { - highlighted, err := HighlightCodeWithConfig(string(p), h.config) + highlighted, err := HighlightCodeWithConfig(ctx, string(p), h.config) if err != nil { return 0, err } - return h.writer.Write([]byte(highlighted)) + n, err = h.writer.Write([]byte(highlighted)) + if err != nil { + return n, fmt.Errorf("failed to write highlighted code: %w", err) + } + return n, nil }pkg/config/config.go (1)
429-479: Simplify configuration merging logic.The current implementation is verbose and hard to maintain. Consider using a recursive merge function or a configuration merging library.
Example approach using a recursive merge function:
func mergeConfigs(dst, src map[string]interface{}) { for key, srcVal := range src { if dstVal, exists := dst[key]; exists { if srcMap, isSrcMap := srcVal.(map[string]interface{}); isSrcMap { if dstMap, isDstMap := dstVal.(map[string]interface{}); isDstMap { mergeConfigs(dstMap, srcMap) continue } } } dst[key] = srcVal } }Then use it in processConfigFile:
- // Complex nested handling - if settingsMap, ok := value.(map[string]interface{}); ok { - if terminalMap, ok := settingsMap["terminal"].(map[string]interface{}); ok { - // ... many nested if statements - } - } + settings := fileViper.AllSettings() + mergeConfigs(v.AllSettings(), settings)pkg/schema/schema.go (1)
72-79: LGTM! Well-organized settings structure.The Settings struct provides a clean and logical organization of various configuration options. The use of optional fields with pointers allows for flexible configuration while maintaining clarity.
Consider adding validation tags (like
validate:"required") for any fields that should be mandatory in the future.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
atmos.yaml(1 hunks)pkg/config/config.go(4 hunks)pkg/schema/schema.go(2 hunks)pkg/utils/highlight_utils.go(1 hunks)pkg/utils/json_utils.go(1 hunks)pkg/utils/yaml_utils.go(1 hunks)website/docs/cli/configuration/terminal.mdx(1 hunks)
🧰 Additional context used
🪛 golangci-lint (1.62.2)
pkg/config/config.go
92-92: cannot use &schema.TerminalSettings{…} (value of type *schema.TerminalSettings) as schema.Terminal value in struct literal
(typecheck)
297-297: invalid operation: atmosConfig.Settings.Terminal == nil (mismatched types schema.Terminal and untyped nil)
(typecheck)
299-299: atmosConfig.Settings.Terminal.SyntaxHighlighting undefined (type schema.Terminal has no field or method SyntaxHighlighting)
(typecheck)
300-300: atmosConfig.Settings.Terminal.SyntaxHighlighting undefined (type schema.Terminal has no field or method SyntaxHighlighting)
(typecheck)
303-303: atmosConfig.Settings.Terminal.SyntaxHighlighting undefined (type schema.Terminal has no field or method SyntaxHighlighting)
(typecheck)
🪛 GitHub Check: Build (macos-latest, macos)
pkg/utils/highlight_utils.go
[failure] 40-40:
invalid operation: config.Settings.Terminal == nil (mismatched types schema.Terminal and untyped nil)
[failure] 41-41:
cannot use &schema.TerminalSettings{…} (value of type *schema.TerminalSettings) as schema.Terminal value in assignment
[failure] 46-46:
config.Settings.Terminal.SyntaxHighlighting undefined (type schema.Terminal has no field or method SyntaxHighlighting)
[failure] 47-47:
config.Settings.Terminal.SyntaxHighlighting undefined (type schema.Terminal has no field or method SyntaxHighlighting)
[failure] 51-51:
config.Settings.Terminal.SyntaxHighlighting undefined (type schema.Terminal has no field or method SyntaxHighlighting)
🪛 GitHub Check: Build (ubuntu-latest, linux)
pkg/utils/highlight_utils.go
[failure] 40-40:
invalid operation: config.Settings.Terminal == nil (mismatched types schema.Terminal and untyped nil)
[failure] 41-41:
cannot use &schema.TerminalSettings{…} (value of type *schema.TerminalSettings) as schema.Terminal value in assignment
[failure] 46-46:
config.Settings.Terminal.SyntaxHighlighting undefined (type schema.Terminal has no field or method SyntaxHighlighting)
[failure] 47-47:
config.Settings.Terminal.SyntaxHighlighting undefined (type schema.Terminal has no field or method SyntaxHighlighting)
[failure] 51-51:
config.Settings.Terminal.SyntaxHighlighting undefined (type schema.Terminal has no field or method SyntaxHighlighting)
🔇 Additional comments (11)
pkg/utils/yaml_utils.go (2)
34-42: LGTM! Robust type handling for AtmosConfiguration.The type switch pattern effectively handles both value and pointer types, ensuring consistent behavior regardless of how the configuration is passed.
44-49: Well-implemented error handling with graceful fallback.The code gracefully falls back to plain text output if highlighting fails, ensuring the user experience isn't disrupted.
pkg/utils/json_utils.go (1)
27-34: LGTM! Consistent implementation with YAML utils.The type switch pattern matches the implementation in
yaml_utils.go, maintaining consistency across the codebase.website/docs/cli/configuration/terminal.mdx (2)
102-109: LGTM! Clear explanation of piping behavior.The note about disabled syntax highlighting when piping is crucial for users to understand the behavior in different contexts.
1-135: Verify terminal detection implementation.The documentation mentions that syntax highlighting is automatically disabled when piping, but we should verify the terminal detection implementation.
✅ Verification successful
Terminal detection implementation is properly handled
The codebase has robust terminal detection implementation that correctly handles piped output scenarios. Here's the key evidence:
Terminal detection is implemented using the standard
term.IsTerminal()function to check if stdout is connected to a terminal:
pkg/utils/highlight_utils.go: Disables syntax highlighting when not connected to a terminalcmd/docs.go: Adjusts output width based on terminal detectioninternal/tui/templates/term/term_writer.go: Provides terminal-aware text wrappingColor support is properly handled:
internal/tui/utils/utils.go: Checks terminal color support before applying styling- Uses
supportscolor.Stdout().SupportsColorto detect color capabilitiesPiped output is correctly detected:
pkg/utils/highlight_utils.go: Explicitly checks!term.IsTerminal(int(os.Stdout.Fd()))and returns plain text when pipedinternal/tui/templates/term/term_writer.go: Falls back to basic writer when not connected to a terminalThe implementation aligns with the documentation's claim about automatically disabling syntax highlighting when output is piped.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify terminal detection implementation # Search for terminal detection logic in the codebase # Look for terminal detection code rg -l "os\.(Stdout|Stderr)\..*TTY" --type go # Look for specific terminal detection functions ast-grep --pattern 'func $_(){ $$$ os.Stdout.$$$ $$$ }'Length of output: 111
Script:
#!/bin/bash # Let's try different patterns to find terminal detection logic # Search for terminal-related code using broader patterns rg -i "isterm|istty|terminal" --type go -A 5 # Look for imports of terminal-related packages rg "import.*term" --type go -A 5 # Search for specific terminal detection patterns rg "os\.(Stdout|Stderr)" --type go -A 3 # Look for color/formatting related code which often involves terminal detection rg "color.*term|term.*color" --type go -A 3Length of output: 29844
pkg/utils/highlight_utils.go (2)
32-71: Well-structured defensive programming!The function properly handles all nil cases and gracefully falls back to defaults.
🧰 Tools
🪛 GitHub Check: Build (macos-latest, macos)
[failure] 40-40:
invalid operation: config.Settings.Terminal == nil (mismatched types schema.Terminal and untyped nil)
[failure] 41-41:
cannot use &schema.TerminalSettings{…} (value of type *schema.TerminalSettings) as schema.Terminal value in assignment
[failure] 46-46:
config.Settings.Terminal.SyntaxHighlighting undefined (type schema.Terminal has no field or method SyntaxHighlighting)
[failure] 47-47:
config.Settings.Terminal.SyntaxHighlighting undefined (type schema.Terminal has no field or method SyntaxHighlighting)
[failure] 51-51:
config.Settings.Terminal.SyntaxHighlighting undefined (type schema.Terminal has no field or method SyntaxHighlighting)🪛 GitHub Check: Build (ubuntu-latest, linux)
[failure] 40-40:
invalid operation: config.Settings.Terminal == nil (mismatched types schema.Terminal and untyped nil)
[failure] 41-41:
cannot use &schema.TerminalSettings{…} (value of type *schema.TerminalSettings) as schema.Terminal value in assignment
[failure] 46-46:
config.Settings.Terminal.SyntaxHighlighting undefined (type schema.Terminal has no field or method SyntaxHighlighting)
[failure] 47-47:
config.Settings.Terminal.SyntaxHighlighting undefined (type schema.Terminal has no field or method SyntaxHighlighting)
[failure] 51-51:
config.Settings.Terminal.SyntaxHighlighting undefined (type schema.Terminal has no field or method SyntaxHighlighting)
139-151: Clean implementation following Go idioms!The struct and constructor are well-designed and follow Go best practices.
atmos.yaml (1)
318-327: 🛠️ Refactor suggestionConsolidate duplicate terminal settings sections.
There are two
terminalsections in the configuration. Consider merging them to avoid confusion.Suggested structure:
settings: terminal: syntax_highlighting: enabled: true lexer: yaml formatter: terminal style: dracula pager: true options: line_numbers: true wrap: false - max_width: 120 - pager: true - timestamps: false - colors: true - unicode: true + display: + max_width: 120 + timestamps: false + colors: true + unicode: trueLikely invalid or redundant comment.
pkg/schema/schema.go (3)
54-66: LGTM! Well-structured syntax highlighting configuration.The new types provide a comprehensive and flexible configuration for syntax highlighting. The structure is clean, and the field names are descriptive and self-documenting.
68-70: LGTM! Clean terminal settings structure.The TerminalSettings struct is well-designed with a clear single responsibility for managing terminal-specific configurations.
19-19: Verify handling of nil Settings pointer.The change from value to pointer type for the Settings field is a breaking change. Ensure that all code paths handle the potential nil value appropriately.
✅ Verification successful
All code paths properly handle nil Settings field
The codebase shows proper handling of nil Settings field throughout. Here's the evidence:
In
pkg/utils/highlight_utils.go, nil checks are performed before accessing Settings:if config.Settings.Terminal == nil { config.Settings.Terminal = &schema.TerminalSettings{...} }In
pkg/config/config.go, default values are assigned when Settings is nil:if atmosConfig.Settings.Terminal == nil { atmosConfig.Settings.Terminal = defaultCliConfig.Settings.Terminal }In
pkg/merge/merge.go, default values are set for uninitialized fields:if atmosConfig.Settings.ListMergeStrategy == "" { atmosConfig.Settings.ListMergeStrategy = ListMergeStrategyReplace }The code consistently checks for nil pointers and provides appropriate defaults, making this a safe change.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for direct Settings field access that might not handle nil ast-grep --pattern 'AtmosConfiguration { $$$ Settings $$$ }' # Search for potential nil pointer dereferences rg -A 2 '\.Settings\.'Length of output: 13395
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
pkg/schema/schema.go (2)
55-58: Consider adding field descriptions.The
SyntaxHighlightOptionsstruct would benefit from field documentation to explain the purpose of each option.Add documentation comments:
type SyntaxHighlightOptions struct { + // LineNumbers enables line number display in highlighted output LineNumbers bool `yaml:"line_numbers" json:"line_numbers" mapstructure:"line_numbers"` + // Wrap enables text wrapping in highlighted output Wrap bool `yaml:"wrap" json:"wrap" mapstructure:"wrap"` }
78-84: Consider adding validation for list merge strategy.The
Settingsstruct includes aListMergeStrategyfield, but there's no indication of valid values.Add an enum or constant for valid merge strategies:
// Valid list merge strategies const ( ListMergeStrategyOverwrite = "overwrite" ListMergeStrategyAppend = "append" ListMergeStrategyPrepend = "prepend" )
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
pkg/schema/schema.go(2 hunks)
🔇 Additional comments (4)
pkg/schema/schema.go (4)
19-19: LGTM! Pointer type change improves flexibility.The change from
AtmosSettingsto*AtmosSettingsallows for nil values, which is appropriate for optional settings.
49-53: LGTM! AtmosSettings struct updated with new fields.The struct now includes terminal settings and maintains backward compatibility with existing fields.
69-76: LGTM! Well-structured terminal settings.The
TerminalSettingsstruct effectively combines basic terminal configuration with syntax highlighting settings.
60-67: Verify lexer and formatter compatibility.The
SyntaxHighlightingSettingsstruct introduces configuration for syntax highlighting. Ensure that:
- The lexer values are validated against supported Chroma lexers
- The formatter values are validated against supported Chroma formatters
Run this script to check Chroma's supported lexers and formatters:
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
pkg/schema/schema.go (2)
47-68: Consider adding field validation tags.The terminal and syntax highlighting settings are well-structured, but consider adding validation tags for fields that need constraints:
max_widthmight need a minimum valuelexer,formatter, andstylemight need to be validated against a list of supported valuesExample validation tags:
type TerminalSettings struct { MaxWidth int `yaml:"max_width,omitempty" json:"max_width,omitempty" mapstructure:"max_width" validate:"omitempty,min=20"` // ... other fields ... } type SyntaxHighlightingSettings struct { Lexer string `yaml:"lexer" json:"lexer" mapstructure:"lexer" validate:"required,oneof=yaml json go"` Formatter string `yaml:"formatter" json:"formatter" mapstructure:"formatter" validate:"required,oneof=terminal html"` Style string `yaml:"style" json:"style" mapstructure:"style" validate:"required,oneof=monokai github dracula"` // ... other fields ... }
70-76: Add GoDoc comments for the Settings struct.The new
Settingsstruct would benefit from documentation explaining its purpose and the significance of each field.Add documentation like this:
// Settings represents the global configuration settings for Atmos. // It combines various setting groups including dependencies, Spacelift integration, // templates, and terminal configurations. type Settings struct { // DependsOn defines component dependencies DependsOn DependsOn `yaml:"depends_on,omitempty" json:"depends_on,omitempty" mapstructure:"depends_on"` // Spacelift contains Spacelift-specific settings Spacelift SettingsSpacelift `yaml:"spacelift,omitempty" json:"spacelift,omitempty" mapstructure:"spacelift"` // Templates defines template processing settings Templates Templates `yaml:"templates,omitempty" json:"templates,omitempty" mapstructure:"templates"` // ListMergeStrategy defines how lists are merged in configurations ListMergeStrategy string `yaml:"list_merge_strategy,omitempty" json:"list_merge_strategy,omitempty"` // Terminal contains terminal-specific settings including syntax highlighting Terminal *TerminalSettings `yaml:"terminal,omitempty" json:"terminal,omitempty" mapstructure:"terminal"` // Docs contains documentation generation settings Docs *Docs `yaml:"docs,omitempty" json:"docs,omitempty" mapstructure:"docs"` }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
pkg/schema/schema.go(2 hunks)
🔇 Additional comments (2)
pkg/schema/schema.go (2)
40-44: Clean and well-structured settings definition!The
AtmosSettingsstruct maintains good separation of concerns while making the terminal settings optional through the pointer type.
19-19: Verify handling of nil Settings field.The change from
AtmosSettingsto*AtmosSettingsis a breaking change. Ensure that all code paths handle potential nil values appropriately.✅ Verification successful
Nil checks are properly implemented, warrior! The code is battle-ready.
The codebase shows proper defensive programming with comprehensive nil checks for the Settings field:
- In
pkg/utils/highlight_utils.goandpkg/config/config.go, there are explicit checks forconfig.Settings == nil- When Settings is nil, default values are properly assigned:
if atmosConfig.Settings == nil { atmosConfig.Settings = defaultCliConfig.Settings }- Nested fields are also protected with nil checks:
if config.Settings.Terminal == nil { config.Settings.Terminal = &schema.TerminalSettings{...} }- All direct accesses to Settings fields are guarded by conditional checks that ensure the parent objects exist
The code demonstrates robust defensive programming practices, making the pointer type change safe and well-handled throughout the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Find all direct accesses to the Settings field that might need nil checks # Search for direct Settings field access rg -A 2 "\.Settings\." pkg/ # Search for Settings assignments rg -A 2 "\.Settings\s*=" pkg/Length of output: 8069
what
Colorize output when we attach TTY
Before:
After
Example 1
Example 2
references
https://github.com/alecthomas/chroma
Summary by CodeRabbit
New Features
Documentation