feat: add pagination and searching to atmos docs command
#848
Conversation
📝 WalkthroughWalkthroughThe pull request introduces modifications to the documentation display within the Atmos CLI. Key changes include the implementation of a new Changes
Possibly related PRs
Suggested labels
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
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (3)
pkg/utils/doc_utils.go (1)
29-32: Consider adding timeout for pager executionThe pager execution could potentially hang indefinitely. Consider adding a context with timeout.
+import "context" -cmd := exec.Command(args[0], args[1:]...) +ctx, cancel := context.WithTimeout(context.Background(), 30*time.Second) +defer cancel() +cmd := exec.CommandContext(ctx, args[0], args[1:]...)website/docs/cli/configuration/configuration.mdx (2)
154-160: Documentation looks good, but consider adding environment variable support.The new
docsconfiguration section is well-documented with clear examples. However, it would be beneficial to support environment variables for these settings, similar to other configuration options.Consider adding support for:
ATMOS_SETTINGS_DOCS_MAX_WIDTHATMOS_SETTINGS_DOCS_PAGINATION
Line range hint
1-1000: Add environment variables for the new docs settings to the table.The environment variables table should be updated to include the new
docssettings for completeness.Add these entries to the environment variables table:
| ATMOS_SETTINGS_LIST_MERGE_STRATEGY | settings.list_merge_strategy | Specifies how lists are merged in Atmos stack manifests. The following strategies are supported: `replace`, `append`, `merge` | +| ATMOS_SETTINGS_DOCS_MAX_WIDTH | settings.docs.max-width | The maximum width for displaying component documentation in the terminal | +| ATMOS_SETTINGS_DOCS_PAGINATION | settings.docs.pagination | When enabled, displays component documentation in a pager instead of directly in the terminal |
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
cmd/docs.go(1 hunks)pkg/schema/schema.go(1 hunks)pkg/utils/doc_utils.go(1 hunks)website/docs/cli/configuration/configuration.mdx(2 hunks)
🔇 Additional comments (3)
cmd/docs.go (1)
100-102: LGTM! Error handling is properly implemented
The code correctly handles errors from DisplayDocs and uses the pagination setting from the CLI configuration.
pkg/schema/schema.go (1)
37-38: LGTM! Schema changes are well-structured
The new Pagination field is properly defined with consistent tags and appropriate type.
website/docs/cli/configuration/configuration.mdx (1)
181-195: LGTM! Clear and well-structured documentation.
The documentation for the docs settings is comprehensive and follows the established format. The description list (<dl>) structure maintains consistency with the rest of the document.
|
@RoseSecurity thanks for the PR! We have a task interally to implement the same pager and search as in glow (using the underlying go modules). 
I suppose we can take this PR as a short-term workaround. I think the UI glow beats I guess this current implementation would work if |
|
Or maybe we can support both, a custom pager, and a nice built-in one. |
|
Feel free to approve this, or we can close it if needed (my feelings won't be hurt)! I added an initial function to if pager || cmd.Flags().Changed("pager") {
pagerCmd := os.Getenv("PAGER")
if pagerCmd == "" {
pagerCmd = "less -r"
}
pa := strings.Split(pagerCmd, " ")
c := exec.Command(pa[0], pa[1:]...) // nolint:gosec
c.Stdin = strings.NewReader(out)
c.Stdout = os.Stdout
return c.Run() |
|
@aknysh let's merge this after you're 👍 review |
osterman
requested review from
aknysh
and removed request for
johncblandii and
jamengual
|
These changes were released in v1.124.0. |
what
utilsDocstructwhy
references
Summary by CodeRabbit
Release Notes
New Features
docssection in theatmos.yamlconfiguration file for enhanced documentation settings.Bug Fixes
Documentation
atmos.yamlconfiguration file, including detailed descriptions of new settings and merge strategies.Chores