add pager to describe workflow

[samtholiya]

what

• Updated describe workflows with pager
  

why

• Makes easier for users to view the content

references

• DEC-3251

Summary by CodeRabbit

• New Features

  • Enhanced the "describe workflows" command with improved flag parsing, validation, and default values.
  • Added support for output in both YAML and JSON formats, with flexible output types ("list", "map", or "all").
  • Introduced paged and TTY-aware output display for easier viewing of workflow descriptions.
  • Added explicit handling of the "pager" flag in describe stack and describe affected commands for better terminal output control.

• Bug Fixes

  • Improved error handling for invalid flag values in the "describe workflows" command.

• Tests

  • Added comprehensive unit tests for flag parsing, argument handling, and command execution.

• Refactor

  • Modularized the "describe workflows" command for better maintainability and easier testing.
  • Refined pager usage in describe stacks command to allow injection of custom pager instances.
  • Removed unsupported flag type test to streamline test coverage.

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (2)

cmd/describe_workflows.go (1)

84-87: Avoid re-defining validation errors – reuse the ones from exec

ErrInvalidFormat and ErrInvalidOutputType already exist in the exec package. Re-declaring them here breaks error identity (errors.Is/As will no longer work) and duplicates maintenance effort.

-var (
-	ErrInvalidOutputType = fmt.Errorf("invalid output type specified. Valid values are 'list', 'map', and 'all'")
-	ErrInvalidFormat     = fmt.Errorf("invalid format specified. Valid values are 'yaml' and 'json'")
-)
+// import "github.com/cloudposse/atmos/internal/exec" above and
+// rely on exec.ErrInvalidOutputType / exec.ErrInvalidFormat

cmd/describe_stacks.go (1)

58-65: Minor duplication & consistency nit

1. The global call to setCliArgsForDescribeStackCli works but bypasses the indirection provided via g.setCliArgsForDescribeStackCli, slightly weakening testability/injection.
2. The pager flag is already processed inside setCliArgsForDescribeStackCli; the second retrieval here is redundant but harmless.

No action required, just flagging for future cleanup.

📜 Review details

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

📥 Commits

Reviewing files that changed from the base of the PR and between cd3236e and e9eb6b0.

📒 Files selected for processing (4)

• cmd/describe_affected.go (1 hunks)
• cmd/describe_stacks.go (1 hunks)
• cmd/describe_workflows.go (3 hunks)
• internal/exec/describe_stacks.go (1 hunks)

🧰 Additional context used

🧠 Learnings (2)

cmd/describe_stacks.go (1)

Learnt from: samtholiya
PR: cloudposse/atmos#1255
File: cmd/describe_affected.go:122-123
Timestamp: 2025-05-22T19:58:32.988Z
Learning: The "pager" flag is defined as a PersistentFlag at the describe command level in cmd/describe.go, making it available to all subcommands including describeAffectedCmd without needing to redeclare it.

cmd/describe_affected.go (2)

Learnt from: samtholiya
PR: cloudposse/atmos#1255
File: cmd/describe_affected.go:122-123
Timestamp: 2025-05-22T19:58:32.988Z
Learning: The "pager" flag is defined as a PersistentFlag at the describe command level in cmd/describe.go, making it available to all subcommands including describeAffectedCmd without needing to redeclare it.

Learnt from: samtholiya
PR: cloudposse/atmos#1266
File: cmd/describe_affected.go:0-0
Timestamp: 2025-06-07T19:28:21.289Z
Learning: In the Atmos codebase, using panic for unsupported flag types in flag processing functions like setDescribeAffectedFlagValueInCliArgs is the expected behavior rather than returning errors. This pattern is preferred for developer errors when unsupported types are added to the flagsKeyValue map.

🧬 Code Graph Analysis (1)

cmd/describe_stacks.go (1)

pkg/schema/schema.go (2)

• Settings (759-763)
• Terminal (200-207)

🪛 golangci-lint (1.64.8)

cmd/describe_affected.go

[error] 42-42: ineffectual assignment to err

(ineffassign)

⏰ Context from checks skipped due to timeout of 90000ms (4)

• GitHub Check: Build (windows-latest, windows)
• GitHub Check: Analyze (go)
• GitHub Check: Lint (golangci)
• GitHub Check: Summary

🔇 Additional comments (1)

internal/exec/describe_stacks.go (1)

93-95: Good catch—reuse the injected pager creator

Switching to d.pageCreator removes an unnecessary allocation and keeps the injected dependency path consistent with the rest of the refactor. ✅