feat(cli-reference): improve intent pills, add shortcut/alias support#3361
Merged
Conversation
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Mpdreamz
temporarily deployed
to
integration-tests
GitHub Actions
Inactive
8 tasks
Contributor
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Enterprise Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThis PR introduces two major features to the CLI documentation system: (1) CLI shortcuts with synthetic alias pages that redirect to canonical command/namespace pages, including ancestor namespace options and global options rendering on command pages, and (2) optional descriptions for CLI modifier flags with CSS-driven tooltips on hover. The shortcut support flows from schema definition through file objects, markdown generation, and navigation/layout rendering. The modifier descriptions follow a similar path from block models through view models to rendered tooltips. Both features extend existing nullable-collection handling in namespace definitions to permit sparse schema structures. Sequence DiagramsequenceDiagram
participant Schema
participant BuildSynth as BuildSyntheticFiles
participant GenPages as CliMarkdownGenerator
participant NavGen as NavigationSetup
Schema->>BuildSynth: schema.Shortcuts, schema.Namespaces
BuildSynth->>BuildSynth: create CliAliasFile for each shortcut
BuildSynth->>BuildSynth: compute ancestor options per command
BuildSynth->>GenPages: NamespacePage(ns, shortcuts)
BuildSynth->>GenPages: CommandPage(cmd, shortcuts, ancestorOptions)
GenPages-->>BuildSynth: rendered pages
BuildSynth->>NavGen: synthetic alias entities
NavGen-->>NavGen: render [alias] navigation badges
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches✨ Simplify code
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/Elastic.Markdown/Extensions/CliReference/CliMarkdownGenerator.cs (1)
495-500:⚠️ Potential issue | 🟠 Major | ⚡ Quick winOnly sort required-first for options, not arguments.
AppendParametersis used for positional argument sections too. Reordering that list changes the documented invocation order, so commands with optional positionals will render incorrect argument sequences.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/Elastic.Markdown/Extensions/CliReference/CliMarkdownGenerator.cs` around lines 495 - 500, AppendParameters currently reorders parameters by p.Required for all parameter types, which breaks positional argument order; change the logic to only apply OrderByDescending(p => p.Required) to option parameters while preserving original sequence for positional parameters. Inside AppendParameters, split parameters into positional (e.g., where p.Name != "_" and not starting with '-' or using a property like p.IsOption if available) and option groups, leave the positional group in its incoming order, sort only the option group by p.Required (and other desired ordering), then concatenate them for rendering; reference AppendParameters, parameters, CliParamSchema, p.Required, p.Name and p.Hidden when locating and updating the code.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/Elastic.Documentation.Site/Assets/markdown/cli-modifiers.css`:
- Around line 18-19: The tooltip is only shown on hover (selector
&[data-tooltip]:hover::after) which prevents keyboard users from seeing it;
update the CSS to also show the tooltip on focus (e.g., change the selector to
include :focus and/or :focus-visible such as &[data-tooltip]:hover::after,
&[data-tooltip]:focus::after { `@apply` opacity-100; }) and in
CliModifiersView.cshtml make each .cli-modifier span keyboard-focusable by
adding tabindex="0" to the span markup so focus triggers the tooltip.
In `@src/Elastic.Markdown/_Layout.cshtml`:
- Around line 16-19: Prevent redirects from archive layouts and safely encode
the URL before embedding in JS: change the condition to only run when the page
is not an archive (e.g., check Model.IsArchive != true or a similar archive flag
together with Model.RedirectUrl) and output the redirect URL using a JS-safe
encoder instead of raw interpolation (use HttpUtility.JavaScriptStringEncode or
serialize via JsonConvert/Json.Serialize and render with Html.Raw) so the value
is properly quoted/escaped before calling window.location.replace.
In `@src/Elastic.Markdown/Extensions/CliReference/CliMarkdownGenerator.cs`:
- Around line 856-863: The fallback currently constructs a truncated alias
suffix when canonicalUsage doesn't start with canonicalPrefix (see
canonicalPrefix, canonicalUsage, suffix, fullPath, binaryName, to) which can
produce incomplete alias examples; change the logic so that if canonicalUsage
does not start with canonicalPrefix you do not synthesize an alias usage at all
(i.e., treat it as unsafe to rewrite and skip emitting the alias variant)
instead of falling back to using fullPath[to.Length..]. Ensure the code path
that emits alias usage checks this condition and returns/omits the alias when
the match fails.
- Around line 96-99: Remove the inline JS redirect insertion so layout-level
RedirectUrl logic can control redirects: delete the call that appends the script
using canonicalUrl (the
sb.AppendLine($"<script>window.location.replace('{canonicalUrl}');</script>")
line) and keep only the subsequent no-JS fallback text that appends the shortcut
description using binaryName, shortcut.From and canonicalFull; ensure no other
code in CliMarkdownGenerator.cs injects a client-side redirect for alias pages.
In
`@src/Elastic.Markdown/Extensions/CliReference/CliReferenceDocsBuilderExtension.cs`:
- Around line 212-220: The loop over schema.Shortcuts can silently overwrite
existing entries in _syntheticFiles when SyntheticPath(...) for shortcut.From
collides; update the foreach handling in CliReferenceDocsBuilderExtension so
that after computing aliasPath (using SyntheticPath) you check whether
_syntheticFiles already contains that key, and if so emit an error/log message
referencing the conflicting shortcut (include schema and shortcut.From/To) and
skip creating the CliEntityInfo for that alias instead of assigning to
_syntheticFiles; only create and assign aliasInfo when the path is not already
claimed to prevent silent hijacks.
---
Outside diff comments:
In `@src/Elastic.Markdown/Extensions/CliReference/CliMarkdownGenerator.cs`:
- Around line 495-500: AppendParameters currently reorders parameters by
p.Required for all parameter types, which breaks positional argument order;
change the logic to only apply OrderByDescending(p => p.Required) to option
parameters while preserving original sequence for positional parameters. Inside
AppendParameters, split parameters into positional (e.g., where p.Name != "_"
and not starting with '-' or using a property like p.IsOption if available) and
option groups, leave the positional group in its incoming order, sort only the
option group by p.Required (and other desired ordering), then concatenate them
for rendering; reference AppendParameters, parameters, CliParamSchema,
p.Required, p.Name and p.Hidden when locating and updating the code.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Enterprise
Run ID: aefffade-919e-4b8c-a179-3b7a72920d0b
📒 Files selected for processing (19)
src/Elastic.Documentation.Configuration/Toc/CliReference/CliSchema.cssrc/Elastic.Documentation.Navigation/Isolated/Node/DocumentationSetNavigation.cssrc/Elastic.Documentation.Site/Assets/markdown/cli-modifiers.csssrc/Elastic.Documentation.Site/Navigation/_TocTreeNav.cshtmlsrc/Elastic.Markdown/Extensions/CliReference/CliAliasFile.cssrc/Elastic.Markdown/Extensions/CliReference/CliCommandFile.cssrc/Elastic.Markdown/Extensions/CliReference/CliMarkdownGenerator.cssrc/Elastic.Markdown/Extensions/CliReference/CliNamespaceFile.cssrc/Elastic.Markdown/Extensions/CliReference/CliReferenceDocsBuilderExtension.cssrc/Elastic.Markdown/HtmlWriter.cssrc/Elastic.Markdown/IO/MarkdownFile.cssrc/Elastic.Markdown/MarkdownLayoutViewModel.cssrc/Elastic.Markdown/Myst/Directives/CliModifiers/CliModifiersBlock.cssrc/Elastic.Markdown/Myst/Directives/CliModifiers/CliModifiersView.cshtmlsrc/Elastic.Markdown/Myst/Directives/CliModifiers/CliModifiersViewModel.cssrc/Elastic.Markdown/Myst/Directives/DirectiveHtmlRenderer.cssrc/Elastic.Markdown/Page/Index.cshtmlsrc/Elastic.Markdown/Page/IndexViewModel.cssrc/Elastic.Markdown/_Layout.cshtml
- Add :focus-visible to tooltip CSS and tabindex=0 to pills (keyboard a11y) - JS-encode redirect URL and guard against Archive layout in _Layout - Remove dead <script> from AliasPage markdown (blocked by DisableHtml) - Skip alias usage when canonical prefix doesn't match (prevents garbled output) - Emit error and skip on shortcut path collision instead of silent overwrite - Preserve positional parameter order in AppendParameters; sort only options Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Mpdreamz
temporarily deployed
to
integration-tests
GitHub Actions
Inactive
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Intent modifier pills
The row of colored pills that indicate a command's operational characteristics (destructive, auth required, idempotent, etc.) has been improved in two ways:
CLI shortcut / alias support
The upstream CLI schema now ships a
shortcutslist (e.g.es→stack es,elasticsearch→stack es). The docs now reflect this:/cli/es) immediately redirects the browser to the canonical page (/cli/stack/es/) so there's no dead-end stub.🤖 Generated with Claude Code