Skip to content

Commit 08a44dd

Browse files
ostermanclaudeautofix-ci[bot]aknysh
authored
fix: disable pager mouse interactions and add global --pager flag (#1430)
* fix: disable pager mouse interactions and add global --pager flag - Remove tea.WithMouseCellMotion() to enable mouse text selection in pager - Change pager default from enabled to disabled for better scripting/automation - Add global --pager flag supporting true/false/command values - Add color field to terminal settings (deprecates no_color in config) - Support ATMOS_COLOR and COLOR environment variables - Update documentation with new global-flags.mdx page - Add comprehensive tests for pager and color functionality BREAKING CHANGE: Pager is now disabled by default. Use --pager flag or set pager: true in atmos.yaml to enable. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com> * [autofix.ci] apply automated fixes * fix: address PR feedback - improve code quality and documentation accuracy - Replace if-else chain with switch statement for better readability (cmd/root.go) - Fix documentation: move Stack/Component flags to Command-Specific section - Remove Breaking Change section from documentation (better suited for release notes) These changes address code quality concerns raised by GitHub's advanced security scanner and improve the accuracy of the documentation. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com> * fix: address PR feedback - improve code quality and robustness This commit addresses comprehensive feedback from PR review covering code quality, documentation accuracy, test coverage, and consistency. Changes implemented: • Fix case/whitespace handling in pager value parsing (schema.go) • Enhance --pager documentation with examples and quoting guidance • Fix --no-color flag parsing to properly handle true/false values • Remove redundant per-command pager logic in favor of global handling • Replace direct NoColor field access with IsColorEnabled() method • Fix log level comparison to use canonical constants instead of strings • Add comprehensive test coverage for PAGER env vars and flag precedence Technical improvements: - Pager values now normalized with strings.ToLower(strings.TrimSpace()) - Flag parsing avoids variable shadowing and properly validates values - Removed conflicting per-command pager overrides - Enhanced environment variable precedence testing - Documentation includes practical examples for shell quoting All existing tests pass with improved coverage and robustness. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com> * fix: change default log level from Info to Warning and update tests This change reduces Atmos output verbosity by changing the default log level from Info to Warning. Also fixes global flags documentation by removing non-global flag sections. - Change default log level from "Info" to "Warning" in config files - Update all test files to use "Warning" instead of "Info" as default - Remove non-global flag sections from global-flags.mdx documentation - Update documentation examples to reflect new default log level 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com> * fix: final packer test cleanup and documentation refinements - Fix logger level inconsistencies in packer tests (use WarnLevel to match ATMOS_LOGS_LEVEL=Warning) - Add proper resource cleanup with defer blocks to prevent writes to closed pipes - Remove misleading PAGER=cat reference from CI/CD example in global-flags.mdx - Replace with proper ATMOS_PAGER=off for pager disabling - Fix backslash escaping issues in test assertions 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com> * fix: add missing viper import in config_test.go - Add viper import required for TestEnvironmentVariableHandling test function - Resolves undefined viper error after merge conflict resolution 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com> * feat: add NO_PAGER environment variable support and improve documentation - Add NO_PAGER environment variable support following standard CLI conventions - Implement proper precedence: CLI flags > NO_PAGER > ATMOS_PAGER > PAGER > config - Add comprehensive test coverage for NO_PAGER functionality (6 new test cases) - Update global-flags.mdx with NO_PAGER documentation and examples - Improve terminal.mdx by removing redundant deprecation warnings - Add clear configuration precedence documentation for pager and color settings - Focus on promoting current best practices rather than deprecated features 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com> * [autofix.ci] apply automated fixes * fix: enable pager in TestDescribeAffected mock expectations - Initialize atmosConfig with proper Terminal.Pager settings - Set Pager to "false" initially when pager should be disabled - Enable Pager to "true" for tests expecting pager.Run() calls - Ensures mock expectations align with actual pager invocation logic - Fix linting issue: use tagged switch instead of if-else 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com> * fix: correct pager flag syntax in TestExecuteDescribeComponentCmd_Success_YAMLWithPager - Change `--pager more` to `--pager=more` to fix flag parsing issue - Cobra was treating `more` as a separate argument instead of the pager value - This was causing "Unknown command component-1" errors in tests 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com> * fix: resolve contradictory color configuration and update snapshots - Set default value for settings.terminal.color to true in config - Ensures consistent color configuration: color=true, no_color=false by default - Color is enabled by default when TTY is detected, as intended - Update golden snapshots for config output to include color: true - Update help snapshots to include new global --pager flag - Fixes contradictory config where both color=false and no_color=false appeared - Fix linting issues: add period to comment and alias log import 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com> * fix: improve documentation and test cleanup for pager and logger - Add GoDoc comment to IsPagerEnabled method describing its purpose and return value - Fix inline comments to be complete sentences with proper punctuation - Fix packer validate and version tests to properly handle logger levels: - Set logger level to Warning to match ATMOS_LOGS_LEVEL environment variable - Restore original logger level and output using defer for proper cleanup - Prevent writing to closed pipes by restoring logger output to os.Stderr 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com> * fix: increase Windows retry delays for better CI reliability - Increase initial retry delay from 100ms to 200ms - Increase progressive delays to 200ms, 500ms, 1000ms - Increase file operation delay to 200ms - These changes improve reliability of Windows GitHub Actions runners by giving file operations more time to complete 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com> * fix: update Windows delay test expectations to match increased retry delays - Update TestWindowsFileDelay to expect 200ms delay (was 150ms max) - Update retry test timing expectations for new delay values - TestRetryOnWindows_RetryThenSuccess: expect 650ms+ (was 250ms+) - TestRetryOnWindows_AllFailures: expect 650ms+ (was 250ms+) - Comments updated to reflect actual delay values: 200ms, 500ms, 1000ms This fixes the TestWindowsFileDelay test failure caused by increasing windowsFileDelay from 100ms to 200ms to improve Windows CI reliability. The original terraform_output_function_(no_tty) test is now passing. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com> * fix: improve log level comparison and reduce complexity in validate_editorconfig - Replace direct string comparison with logger.ParseLogLevel() for consistent log level parsing in cmd/validate_editorconfig.go - Refactor nested if statements to reduce complexity (nestif linter) - Extract boolean variables to clarify logic for trace level detection - Added import for github.com/cloudposse/atmos/pkg/logger package The PAGER=cat documentation requested for update was not found in the current website/docs/cli/global-flags.mdx file at the specified location. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com> * docs: clarify pager settings and improve terminal documentation structure - Fix confusing categorization of pager settings in terminal.mdx - Clarify that there are TWO different pager settings: 1. Terminal.Pager: Main pager for all terminal output 2. SyntaxHighlighting.Pager: Separate pager for syntax-highlighted content - Update YAML example to show both settings and their different purposes - Improve description of syntax highlighting pager to clarify it's separate from the main terminal pager setting - Add comments in YAML example to distinguish terminal vs syntax settings This resolves confusion where pager appeared to be incorrectly categorized as a syntax highlighting setting when it's actually a terminal setting. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com> * refactor: remove unused syntax_highlighting.pager setting - Remove HighlightedOutputPager field from SyntaxHighlighting struct - Remove unused pager setting from highlight defaults and configurations - Update tests to remove references to unused syntax highlighting pager - Clean up documentation to remove confusing syntax highlighting pager - Simplify YAML example to only show implemented terminal settings The syntax_highlighting.pager setting was defined in the schema and set in defaults, but never actually used anywhere in the codebase. It was dead code that created confusion about pager functionality. Only the main Terminal.Pager setting is functional and controls all terminal output paging behavior. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com> * docs: improve NO_PAGER comment to explain negative boolean logic Expand the nolint comment to clearly explain why NO_PAGER uses direct os.Getenv() instead of viper.BindEnv(): 1. NO_PAGER uses negative logic (NO_PAGER=true disables pager) 2. Atmos config uses positive boolean names (pager: true enables pager) 3. We don't want a confusing "no_pager" config field in the schema 4. NO_PAGER should remain environment-only, not a config file setting This clarifies the design decision to keep NO_PAGER as a standard CLI environment variable convention rather than making it configurable. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com> * test: update snapshots to remove syntax_highlighting.pager references Regenerate golden snapshots for: - TestCLICommands_atmos_describe_config_imports.stdout.golden - TestCLICommands_atmos_describe_configuration.stdout.golden These snapshots were expecting `pager: false` under the `syntax_highlighting` section, but this field was removed as unused dead code. The snapshots now correctly reflect the current schema structure without the unused HighlightedOutputPager field. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com> * test: update remaining snapshots to remove syntax_highlighting.pager references Updates three test snapshots that still contained references to the removed syntax_highlighting.pager setting: - TestCLICommands_indentation.stdout.golden - TestCLICommands_atmos_describe_config_-f_yaml.stdout.golden - TestCLICommands_atmos_describe_config.stdout.golden This completes the cleanup from the previous refactoring that removed the unused syntax_highlighting.pager configuration option. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com> * fix: remove remaining pager reference from test fixture Removes the last remaining `pager: false` reference from the test fixture at tests/fixtures/scenarios/atmos-describe-affected-with-dependents-and-locked/atmos.yaml. This completes the cleanup of all syntax_highlighting.pager references that were causing Windows acceptance test failures due to snapshot mismatches. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com> --------- Co-authored-by: Claude <noreply@anthropic.com> Co-authored-by: autofix-ci[bot] <114827586+autofix-ci[bot]@users.noreply.github.com> Co-authored-by: Andriy Knysh <aknysh@users.noreply.github.com>
1 parent e494ebc commit 08a44dd

File tree

63 files changed

+1207
-177
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

63 files changed

+1207
-177
lines changed

atmos.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -352,7 +352,7 @@ settings:
352352
# Terminal settings for displaying content
353353
terminal:
354354
max_width: 120 # Maximum width for terminal output
355-
pager: true # Pager setting for all terminal output
355+
pager: false # Pager setting for all terminal output
356356
unicode: true # Use unicode characters
357357

358358
syntax_highlighting:

cmd/describe.go

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,6 @@ var describeCmd = &cobra.Command{
1515

1616
func init() {
1717
describeCmd.PersistentFlags().StringP("query", "q", "", "Query the results of an `atmos describe` command using `yq` expressions")
18-
describeCmd.PersistentFlags().String("pager", "true", "Disable / Enable the paging user experience")
1918

2019
RootCmd.AddCommand(describeCmd)
2120
}

cmd/describe_affected.go

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -70,13 +70,7 @@ func getRunnableDescribeAffectedCmd(
7070
}
7171
}
7272

73-
if cmd.Flags().Changed("pager") {
74-
// TODO: update this post pr:https://github.com/cloudposse/atmos/pull/1174 is merged
75-
props.CLIConfig.Settings.Terminal.Pager, err = cmd.Flags().GetString("pager")
76-
if err != nil {
77-
return err
78-
}
79-
}
73+
// Global --pager flag is now handled in cfg.InitCliConfig
8074

8175
err = newDescribeAffectedExec(props.CLIConfig).Execute(&props)
8276
return err

cmd/describe_component.go

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -61,11 +61,6 @@ var describeComponentCmd = &cobra.Command{
6161
return err
6262
}
6363

64-
pager, err := flags.GetString("pager")
65-
if err != nil {
66-
return err
67-
}
68-
6964
component := args[0]
7065

7166
err = e.NewDescribeComponentExec().ExecuteDescribeComponentCmd(e.DescribeComponentParams{
@@ -75,7 +70,6 @@ var describeComponentCmd = &cobra.Command{
7570
ProcessYamlFunctions: processYamlFunctions,
7671
Skip: skip,
7772
Query: query,
78-
Pager: pager,
7973
Format: format,
8074
File: file,
8175
})

cmd/describe_config.go

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -33,13 +33,7 @@ var describeConfigCmd = &cobra.Command{
3333
return err
3434
}
3535

36-
if cmd.Flags().Changed("pager") {
37-
// TODO: update this post pr:https://github.com/cloudposse/atmos/pull/1174 is merged
38-
atmosConfig.Settings.Terminal.Pager, err = cmd.Flags().GetString("pager")
39-
if err != nil {
40-
return err
41-
}
42-
}
36+
// Global --pager flag is now handled in cfg.InitCliConfig
4337

4438
err = e.NewDescribeConfig(&atmosConfig).ExecuteDescribeConfigCmd(query, format, "")
4539
return err

cmd/describe_dependents.go

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -54,12 +54,7 @@ func getRunnableDescribeDependentsCmd(
5454
return err
5555
}
5656

57-
if cmd.Flags().Changed("pager") {
58-
atmosConfig.Settings.Terminal.Pager, err = cmd.Flags().GetString("pager")
59-
if err != nil {
60-
return err
61-
}
62-
}
57+
// Global --pager flag is now handled in cfg.InitCliConfig
6358

6459
describe.Component = args[0]
6560
err = newDescribeDependentsExec(&atmosConfig).Execute(describe)

cmd/describe_stacks.go

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -70,13 +70,7 @@ func getRunnableDescribeStacksCmd(
7070
return err
7171
}
7272

73-
if cmd.Flags().Changed("pager") {
74-
// TODO: update this post pr:https://github.com/cloudposse/atmos/pull/1174 is merged
75-
atmosConfig.Settings.Terminal.Pager, err = cmd.Flags().GetString("pager")
76-
if err != nil {
77-
return err
78-
}
79-
}
73+
// Global --pager flag is now handled in cfg.InitCliConfig
8074

8175
err = g.newDescribeStacksExec.Execute(&atmosConfig, describe)
8276
return err

cmd/describe_workflows.go

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -52,12 +52,7 @@ func getRunnableDescribeWorkflowsCmd(
5252
return err
5353
}
5454

55-
pager, err := cmd.Flags().GetString("pager")
56-
if err != nil {
57-
return err
58-
}
59-
60-
atmosConfig.Settings.Terminal.Pager = pager
55+
// Global --pager flag is now handled in cfg.InitCliConfig
6156
err = describeWorkflowsExec.Execute(&atmosConfig, describeWorkflowArgs)
6257
return err
6358
}

cmd/packer_init_test.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,8 @@ func TestPackerInitCmd(t *testing.T) {
1111
workDir := "../tests/fixtures/scenarios/packer"
1212
t.Setenv("ATMOS_CLI_CONFIG_PATH", workDir)
1313
t.Setenv("ATMOS_BASE_PATH", workDir)
14-
t.Setenv("ATMOS_LOGS_LEVEL", "Info")
15-
log.SetLevel(log.InfoLevel)
14+
t.Setenv("ATMOS_LOGS_LEVEL", "Warning")
15+
log.SetLevel(log.WarnLevel)
1616

1717
RootCmd.SetArgs([]string{"packer", "init", "aws/bastion", "-s", "nonprod"})
1818
err := Execute()

cmd/packer_inspect_test.go

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -14,22 +14,27 @@ func TestPackerInspectCmd(t *testing.T) {
1414
workDir := "../tests/fixtures/scenarios/packer"
1515
t.Setenv("ATMOS_CLI_CONFIG_PATH", workDir)
1616
t.Setenv("ATMOS_BASE_PATH", workDir)
17-
t.Setenv("ATMOS_LOGS_LEVEL", "Info")
18-
log.SetLevel(log.InfoLevel)
17+
t.Setenv("ATMOS_LOGS_LEVEL", "Warning")
18+
log.SetLevel(log.WarnLevel)
1919

2020
oldStd := os.Stdout
2121
r, w, _ := os.Pipe()
2222
os.Stdout = w
2323
log.SetOutput(w)
2424

25+
// Ensure cleanup happens before any reads
26+
defer func() {
27+
os.Stdout = oldStd
28+
log.SetOutput(os.Stderr)
29+
}()
30+
2531
RootCmd.SetArgs([]string{"packer", "inspect", "aws/bastion", "-s", "nonprod"})
2632
err := Execute()
2733
assert.NoError(t, err, "'TestPackerInspectCmd' should execute without error")
2834

29-
// Restore std
35+
// Close write end after Execute
3036
err = w.Close()
3137
assert.NoError(t, err)
32-
os.Stdout = oldStd
3338

3439
// Read the captured output
3540
var buf bytes.Buffer

0 commit comments

Comments
 (0)