Skip to content

fix(tree): show completion built-in in tree output - #89

Merged
jpage-godaddy merged 5 commits into
mainfrom
autocomplete
Aug 6, 2026
Merged

fix(tree): show completion built-in in tree output#89
jpage-godaddy merged 5 commits into
mainfrom
autocomplete

Conversation

@jpage-godaddy

Copy link
Copy Markdown
Collaborator

Summary

  • tree (the built-in that lists the full command hierarchy) special-cased completion out of its output — a leftover filter added before the completion command existed. completion --install/completion <shell> always worked via direct dispatch; only the tree listing hid it.
  • Documents the completion built-in in concepts.md's Built-In Commands table, which omitted it entirely, and links to the existing completion.md page.

Test plan

  • cargo fmt --all --check
  • cargo clippy --all-targets -- -D warnings
  • RUSTDOCFLAGS='-D warnings' cargo doc --no-deps
  • cargo test --all-targets (updated foundation.rs assertion to expect completion in tree children)

`build_tree_from_clap_with_path` special-cased "completion" out of the
tree listing, a leftover from before the completion command existed.
It ran fine via direct dispatch but never appeared under `<bin> tree`,
contradicting docs describing tree as the full command hierarchy.

Also documents the completion built-in in concepts.md's Built-In
Commands table, which omitted it entirely.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes the tree built-in so it no longer hides the completion built-in from the rendered command hierarchy, and updates documentation to reflect completion as an always-available built-in command.

Changes:

  • Removed the completion name-based exclusion from build_tree_from_clap, relying solely on clap’s hidden flag (is_hide_set) to filter tree nodes.
  • Updated the foundation.rs test to assert that completion appears among visible tree children.
  • Documented the completion built-in in docs/concepts.md, linking to the existing completion.md page.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
cli-engine/src/tree.rs Stops special-casing completion out of tree output; only hidden subcommands are filtered.
cli-engine/tests/foundation.rs Updates tree assertion to expect completion to be present.
cli-engine/docs/concepts.md Adds completion to the Built-In Commands table with a pointer to completion.md.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

completion was discoverable via `tree` and direct invocation but
absent from the root --help page, which hardcodes its "Find Commands"
hints (search/tree/guide) rather than deriving them from the command
tree.
@jpage-godaddy
jpage-godaddy requested a lite review from Copilot August 6, 2026 17:39

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.

Suppressed comments (1)

cli-engine/tests/foundation.rs:10146

  • This assertion now depends on the iteration order of get_subcommands() for multiple entries, which can make the test brittle if registration order changes (or if clap’s ordering differs across versions/builds). Prefer comparing order-independently (e.g., sort the collected names before asserting, or assert membership for the expected items).
            .iter()
            .map(|child| child.name.as_str())
            .collect::<Vec<_>>(),
        vec!["visible", "completion"]

completion's own `shell` positional had no possible_values, so tab
completion never suggested bash/zsh/fish/powershell/elvish. Attach
PossibleValuesParser with ignore_case(true), matching parse_shell's
case-insensitive matching and pwsh alias, so no parsing behavior
changes — only completion candidates are added.

guide's topic arg was investigated for the same treatment but
skipped: attaching possible_values there would make clap reject an
unknown topic at parse time with its own generic error, breaking the
existing exact-match "unknown guide topic ... valid topics: ..."
custom error test. That needs clap_complete's unstable dynamic
completion engine to decouple candidates from enforcement, which is
a separate, larger change.
@jpage-godaddy
jpage-godaddy requested a lite review from Copilot August 6, 2026 18:23
Threads the registered guide names into the `guide` subcommand's
`topic` arg as clap possible values, refreshed via a new
sync_guide_topic_values() every time add_guides()/set_has_guide() run
so config-time and later per-module guide contributions both stay in
sync. `<bin> guide <TAB>` now completes real topics.

This intentionally changes how an unrecognized topic is rejected:
clap itself now rejects it at parse time (exit code 2, clap's
standard "invalid value ... possible values: ..." message) instead
of guide::guide_content's custom "unknown guide topic ... valid
topics: ..." message (exit code 1) ever running. Verified this
matches the framework's existing exit-code convention for other
bad-argument-value rejections (e.g. an unopted command's --limit),
and that neither the old nor new message was ever JSON-enveloped
under --output json, so this isn't a machine-readable-output
regression. Updated the one test that asserted the old exact message.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants