Skip to content

fix(cli): route legacy completions through public binary - #5530

Closed
wuisabel-gif wants to merge 2 commits into
Hmbown:mainfrom
wuisabel-gif:wuisabel/5526-completions
Closed

fix(cli): route legacy completions through public binary#5530
wuisabel-gif wants to merge 2 commits into
Hmbown:mainfrom
wuisabel-gif:wuisabel/5526-completions

Conversation

@wuisabel-gif

Copy link
Copy Markdown
Contributor

Summary

Addresses the approved scope of #5526.

The legacy codewhale completions <shell> command now uses the same canonical completion generator as codewhale completion <shell> instead of forwarding to the codewhale-tui runtime. Generated scripts use the public codewhale command name.

The legacy plural command remains available as a compatibility alias. The canonical singular command is unchanged.

Coverage

Added provider-free CLI tests that verify:

  • codewhale completions powershell parses directly in the CLI;
  • Bash, Fish, PowerShell, and Zsh scripts contain codewhale;
  • generated scripts do not teach the retired codewhale-tui binary name.

Verification

  • cargo fmt --all -- --check
  • git diff --check
  • cargo test -p codewhale-cli --lib --locked (217 passed)

No provider credentials or network access are required.

No-Issue: This PR addresses the approved compatibility slice of #5526 without automatically closing the broader report.

Keep `codewhale completions <shell>` as a compatibility alias while generating scripts from the canonical public CLI name. Add provider-free parser and shell coverage for Hmbown#5526.
@wuisabel-gif
wuisabel-gif requested a review from Hmbown as a code owner August 21, 2026 07:01
@github-actions

Copy link
Copy Markdown
Contributor

Thanks @wuisabel-gif for taking the time to contribute.

This repository is observing a maintainer-managed PR intake gate in dry-run mode, so this pull request is staying open. This note helps maintainers prepare the allowlist before any enforcement is considered.

Please read CONTRIBUTING.md for the expected contribution shape. A maintainer can grant recurring PR access by commenting /lgtm on a pull request.

Keep the generated TUI changelog aligned with the root changelog for Hmbown#5526.
@wuisabel-gif

Copy link
Copy Markdown
Contributor Author

CI update:

  • Version drift failed because the root changelog entry for Deprecated shell completion #5526 needed the generated crates/tui/CHANGELOG.md sync. That is now fixed in commit 95df027f1, and the checks have restarted.
  • The Rust Clippy job reports five Clippy 1.98 findings in existing files outside this PR: commands/groups/core/voice.rs, llm_client/mod.rs, tui/ui/event_loop.rs, and tui/views/status_picker.rs (two findings). None of the completion-related files are involved.

The completion implementation and all 217 local CLI library tests pass. Please let me know whether you would prefer those unrelated baseline lint cleanups in a separate PR or included as a mechanical follow-up here.

@Lstarsky0 Lstarsky0 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Built both sides and diffed the scripts the two versions actually emit. On 75dca2cfb the PowerShell script registers for codewhale-tui and carries the runtime's 29 subcommands; on 95df027f1 it registers for codewhale and carries the CLI's 42. Sixteen of those are new — config, run, thread, web, metrics, update and friends were never completable before, which is the bigger win here than the binary name.

Three go the other way. pr, scorecard and session-diagnostics were in the old script, aren't in the new one, and are still typeable: codewhale hands anything it doesn't recognise to the runtime — a made-up subcommand gets the same "requires an interactive terminal" error that codewhale pr --help does — and all three are real commands (Pr at crates/tui/src/lib.rs:323, Scorecard at :342, SessionDiagnostics at :276). So they still work in a terminal and no longer complete. Defensible if the script is meant to describe the public CLI surface and nothing else, but it isn't visible from the diff.

Smaller: every_completion_shell_uses_the_public_binary_name lists Bash, Fish, PowerShell and Zsh by hand and misses Elvish, the fifth clap_complete::Shell variant. Shell::value_variants() covers all of them and keeps covering them. And let _ = io::stdout().write_all(&output) throws the write result away where run() could ? it.

cargo test -p codewhale-cli --lib --locked on 95df027 is 217 passed here too.

@Hmbown

Hmbown commented Aug 23, 2026

Copy link
Copy Markdown
Owner

Thanks @wuisabel-gif — this slice is now superseded by the v0.9.11 integration merged in #5542 (main @ 8ef856f). The merged fix goes a bit further than this PR: codewhale completion <shell> (with completions as a visible alias) renders in-process from the CLI's own command tree, registers both codewhale and codew for Bash/Zsh/Fish/PowerShell/Elvish, and is pinned by per-shell tests (generated_completion_scripts_register_the_published_command_names, completions_is_an_alias_for_completion in crates/cli/src/lib.rs). Your scoping of #5526 is credited in the v0.9.11 CHANGELOG. Closing as superseded rather than merging, since crates/cli/src/lib.rs now conflicts; please reopen or open a follow-up if you see a shell the merged version still misses.

@Hmbown Hmbown closed this Aug 23, 2026
Lstarsky0 pushed a commit to Lstarsky0/CodeWhale that referenced this pull request Aug 23, 2026
Record the integrated contributions behind PRs Hmbown#5523, Hmbown#5524, and Hmbown#5525, and acknowledge the independently reviewed overlap in Hmbown#5530 without counting it as a duplicate merge.

Signed-off-by: CodeWhale Bot <bot@codewhale.net>
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