Skip to content

fix(help): add --clients to CLI help - #1800

Merged
DeusData merged 1 commit into
DeusData:mainfrom
kilianpaquier:fix/clients-cli-help
Sep 2, 2026
Merged

fix(help): add --clients to CLI help#1800
DeusData merged 1 commit into
DeusData:mainfrom
kilianpaquier:fix/clients-cli-help

Conversation

@kilianpaquier

Copy link
Copy Markdown
Contributor

What does this PR do?

Hello 👋

Just a small PR to add --clients to CLI's help.

Checklist

  • Every commit is signed off (git commit -s) — required, CI rejects
    unsigned commits (DCO, see CONTRIBUTING.md)
  • Tests pass locally (make -f Makefile.cbm test)
  • Lint passes (make -f Makefile.cbm lint-ci)

@github-actions

Copy link
Copy Markdown

Thanks for opening this — it has been seen, and it is queued.

This note is automated, but it is not a brush-off: it exists so you know where your PR stands instead of having to guess from silence.

Current review status: working through a backlog. 0.9.1-rc.1 is out, so the release freeze that held reviews is over — but it left a large queue of open pull requests behind it, and we are reading through them oldest-first. The background is in discussion #1144.

What that means for this PR, concretely:

  • It will not be closed for inactivity. No stale bot touches pull requests here.
  • It may still sit a while before a human reads it. That is on us, not on you.
  • Older PRs are read first, so a recent one is not being skipped — it is behind a queue.

Things that will genuinely speed it up whenever review does happen:

  • Keep it rebased on main — the tree is moving quickly right now, and a conflicting branch cannot be reviewed as the diff you intended.
  • Get CI green, or say which failures you believe are pre-existing.
  • Keep the change to one claim. Bundled features and refactors get split before they get merged, which costs you a round trip.
  • Every commit needs a sign-off (git commit -s) — CI enforces DCO.

If this fixes a bug, a reproduction we can run is worth more than a description of the symptom.

Thanks for contributing, and sorry in advance for the wait.

Signed-off-by: kilianpaquier <noreply@kilianpaquier.dev>
@DeusData DeusData added documentation Improvements or additions to documentation ux/behavior Display bugs, docs, adoption UX priority/normal Standard review queue; useful PR with ordinary maintainer urgency. labels Aug 24, 2026
@DeusData

Copy link
Copy Markdown
Owner

Thank you for the concise help fix. I checked current main: the install parser accepts --clients, but the install usage line in src/main.c omits it. The documentation gap is therefore confirmed.

I have labeled this as a normal-priority documentation and UX correction and queued it for review. CI is green. Our review queue is full, so detailed review may take a little time. Thank you for keeping this to the single help-surface change.

@DeusData

DeusData commented Sep 2, 2026

Copy link
Copy Markdown
Owner

👋 Approved — thank you, this is a real gap and not just tidying.

--clients= has been parsed since #1558 (cli.c:10198) but print_help never mentioned it, so the only way to discover the flag was to read the source or already know it existed. For a flag that decides which agent clients get configured on install, that is a genuine usability defect.

Two details I checked and liked:

  • Pointing at install --clients to list the tokens, rather than trying to enumerate 40-odd client names in the help text. Bare --clients really does print the list (cli.c:10205 handles it alongside --clients=help/list), so the cross-reference is accurate and stays accurate as clients are added.
  • The wording matches the flag's actual form, --clients=<tokens> with the =. That is the form the parser accepts, so nobody follows the help into an error.

I verified the surrounding help block on current main is byte-identical to your patch context, so it still applies cleanly and renders as intended, despite the branch being some way behind.

You left the test and lint boxes unticked — entirely reasonable for two printf lines, and CI is 34/34 green on it, so nothing needed from you there.

I will merge this once our Actions queue recovers; there is currently one job executing repo-wide against 24 queued, so merges are moving slowly today for reasons unrelated to your PR.

@DeusData
DeusData merged commit c895513 into DeusData:main Sep 2, 2026
34 checks passed
@DeusData

DeusData commented Sep 2, 2026

Copy link
Copy Markdown
Owner

Merged as c895513f. Thank you 👋 — small PR, real gap: --clients= had been parsed since #1558 but never appeared in print_help, so the only way to discover the flag was to read the source.

Pointing at install --clients to list the tokens, rather than enumerating client names inline, is what makes it stay correct as clients are added — and as it happens #1802 landed alongside this one and adds registry stable IDs to exactly that listing, so the cross-reference is now complete in both directions.

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

Labels

documentation Improvements or additions to documentation priority/normal Standard review queue; useful PR with ordinary maintainer urgency. ux/behavior Display bugs, docs, adoption UX

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants