From 2d07c19e97e01f42ab495effd7502047fe3c963e Mon Sep 17 00:00:00 2001 From: sdairs Date: Fri, 30 Oct 2026 15:46:52 +0100 Subject: [PATCH] docs(cli): pin subcommand order in --help (#1158) Reorder the cloud and local command enums so help lists entry points and main resources first, add a Subcommand order rule to the cli-help-text skill, and pin the order at the top level, cloud and local with a test. Co-Authored-By: Claude Opus 5.5 --- .agents/skills/cli-help-text/SKILL.md | 9 ++ crates/clickhousectl/src/cli.rs | 59 +++++++++ crates/clickhousectl/src/cloud/cli.rs | 68 +++++----- crates/clickhousectl/src/local/cli.rs | 178 +++++++++++++------------- 4 files changed, 191 insertions(+), 123 deletions(-) diff --git a/.agents/skills/cli-help-text/SKILL.md b/.agents/skills/cli-help-text/SKILL.md index 8ba58926..65d6e070 100644 --- a/.agents/skills/cli-help-text/SKILL.md +++ b/.agents/skills/cli-help-text/SKILL.md @@ -57,6 +57,15 @@ No `long_about`, `before_help`, `after_long_help`, or any other `after_help` hea - Both local clients order common arguments as name, host, port, version, query, queries-file. - Keep release-only URL hiding as it is. +## Subcommand order + +clap lists subcommands in enum declaration order, so order the variants, not `display_order`. + +- Entry points first (`local`, `cloud auth`, `local init`), then the main resources, then secondary resources, + then admin and rarely used commands. +- Within a resource: list, get, create, update, delete, then lifecycle verbs, then sub-resources. +- A test pins the order at the top level, `cloud` and `local`. Place a new command deliberately and update it. + ## `CONTEXT FOR AGENTS:` At most 8 content lines, ideally 3 to 6. Write densely: the block is for agents, not people, so pack related diff --git a/crates/clickhousectl/src/cli.rs b/crates/clickhousectl/src/cli.rs index cc1065bc..322df352 100644 --- a/crates/clickhousectl/src/cli.rs +++ b/crates/clickhousectl/src/cli.rs @@ -293,6 +293,65 @@ mod tests { assert!(failures.is_empty(), "{}", failures.join("\n")); } + /// Help lists subcommands in declaration order, so a new command must be placed + /// deliberately (see "Subcommand order" in the `cli-help-text` skill). + #[test] + fn top_level_subcommands_keep_their_help_order() { + fn visible_subcommands(command: &clap::Command) -> Vec<&str> { + command + .get_subcommands() + .filter(|child| !child.is_hide_set()) + .map(clap::Command::get_name) + .collect() + } + + let root = Cli::command(); + let mut top = vec!["local", "cloud", "skills", "update"]; + if cfg!(feature = "telemetry") { + top.push("telemetry"); + } + assert_eq!(visible_subcommands(&root), top); + + let cloud = root.find_subcommand("cloud").unwrap(); + assert_eq!( + visible_subcommands(cloud), + [ + "auth", + "org", + "service", + "postgres", + "query-api-endpoint", + "saved-query", + "backup", + "clickpipe", + "clickstack", + "udf", + "key", + "member", + "invitation", + "activity", + ] + ); + + let local = root.find_subcommand("local").unwrap(); + assert_eq!( + visible_subcommands(local), + [ + "init", + "clickhouse", + "postgres", + "status", + "client", + "install", + "list", + "use", + "which", + "remove", + "udf", + ] + ); + } + #[test] fn shared_flags_have_identical_help_at_every_declaration() { let shared = [ diff --git a/crates/clickhousectl/src/cloud/cli.rs b/crates/clickhousectl/src/cloud/cli.rs index 1f3c2704..8bec1a2d 100644 --- a/crates/clickhousectl/src/cloud/cli.rs +++ b/crates/clickhousectl/src/cloud/cli.rs @@ -113,14 +113,6 @@ impl CloudArgs { #[allow(clippy::large_enum_variant)] #[derive(Subcommand)] pub enum CloudCommands { - /// Manage user-defined functions - #[command(after_help = "CONTEXT FOR AGENTS: - Writes require API key authentication; list/get support OAuth. - Function names: `cloud udf list`; service IDs: `cloud service list`. - Create uploads a ZIP archive and starts an asynchronous build. - Typical flow: deploy NAME --service , or create -> get until ready -> attach -> attachment get.")] - Udf(UdfArgs), - /// Manage authentication (OAuth login, API keys) Auth { #[command(subcommand)] @@ -151,6 +143,20 @@ CONTEXT FOR AGENTS: command: ServiceCommands, }, + /// Manage ClickHouse Cloud Postgres services (Beta) + #[command(after_help = "\ +CONTEXT FOR AGENTS: + Write commands need API key auth. + Service IDs: `cloud postgres list`. + Credentials come only from `create` and `reset-password`; treat `get` as never returning them. + `get` shows host/user; connections use port 5432, database postgres and TLS. + Role changes return after API acceptance; see each command's help for verification. + Typical flow: `create` -> `get ` until running -> `certs get --output ca.pem` -> connect.")] + Postgres { + #[command(subcommand)] + command: crate::cloud::postgres::PostgresCommands, + }, + /// Manage Query API endpoints #[command(after_help = "CONTEXT FOR AGENTS: Writes require API key authentication; list/get support OAuth. @@ -215,6 +221,26 @@ CONTEXT FOR AGENTS: command: ClickStackCommands, }, + /// Manage user-defined functions + #[command(after_help = "CONTEXT FOR AGENTS: + Writes require API key authentication; list/get support OAuth. + Function names: `cloud udf list`; service IDs: `cloud service list`. + Create uploads a ZIP archive and starts an asynchronous build. + Typical flow: deploy NAME --service , or create -> get until ready -> attach -> attachment get.")] + Udf(UdfArgs), + + /// Manage API keys + #[command(after_help = "\ +CONTEXT FOR AGENTS: + `create` prints a generated key secret exactly once — capture stdout or create a new key. + Role IDs come from `cloud org role list` and must be UUIDs here. + `update` replaces --role-id and --ip-allow wholesale; omitted flags are left as-is. + Next: `cloud auth login --api-key --api-secret ` to use a new key.")] + Key { + #[command(subcommand)] + command: KeyCommands, + }, + /// Manage organization members #[command(after_help = "\ CONTEXT FOR AGENTS: @@ -238,18 +264,6 @@ CONTEXT FOR AGENTS: command: InvitationCommands, }, - /// Manage API keys - #[command(after_help = "\ -CONTEXT FOR AGENTS: - `create` prints a generated key secret exactly once — capture stdout or create a new key. - Role IDs come from `cloud org role list` and must be UUIDs here. - `update` replaces --role-id and --ip-allow wholesale; omitted flags are left as-is. - Next: `cloud auth login --api-key --api-secret ` to use a new key.")] - Key { - #[command(subcommand)] - command: KeyCommands, - }, - /// View the organization activity log #[command(after_help = "\ CONTEXT FOR AGENTS: @@ -259,20 +273,6 @@ CONTEXT FOR AGENTS: #[command(subcommand)] command: ActivityCommands, }, - - /// Manage ClickHouse Cloud Postgres services (Beta) - #[command(after_help = "\ -CONTEXT FOR AGENTS: - Write commands need API key auth. - Service IDs: `cloud postgres list`. - Credentials come only from `create` and `reset-password`; treat `get` as never returning them. - `get` shows host/user; connections use port 5432, database postgres and TLS. - Role changes return after API acceptance; see each command's help for verification. - Typical flow: `create` -> `get ` until running -> `certs get --output ca.pem` -> connect.")] - Postgres { - #[command(subcommand)] - command: crate::cloud::postgres::PostgresCommands, - }, } impl CloudCommands { diff --git a/crates/clickhousectl/src/local/cli.rs b/crates/clickhousectl/src/local/cli.rs index d3c6e76d..4be0748e 100644 --- a/crates/clickhousectl/src/local/cli.rs +++ b/crates/clickhousectl/src/local/cli.rs @@ -187,75 +187,50 @@ impl LocalArgs { #[derive(Subcommand)] pub enum LocalCommands { - /// Install a ClickHouse version or Postgres image - #[command(after_help = INSTALL_AFTER_HELP)] - Install { - /// Version ("latest", "stable", "lts", 25.12, 25.12.9.61) or image selector (postgres@18) - version: InstallVersionArg, - - /// Re-install even if already installed - #[arg(long)] - force: bool, - }, - - /// List installed or available ClickHouse versions + /// Initialize a project directory for ClickHouse and Postgres #[command(after_help = "\ CONTEXT FOR AGENTS: - Default output is exact installed version strings — the form `clickhousectl local remove` needs. - --remote lists minor versions (e.g. 26.3) probed from builds.clickhouse.com, not exact builds.")] - List { - /// List minor versions available for download - #[arg(long)] - remote: bool, - }, + `.clickhouse/` holds runtime data and is git-ignored; the `clickhouse/` and `postgres/` SQL + scaffolds are meant to be committed. + Idempotent — re-running only creates what is missing. + Next: `clickhousectl local clickhouse start`")] + Init, - /// Set the default ClickHouse version - #[command(after_help = "\ + /// Manage local ClickHouse servers + #[command( + visible_alias = "ch", + alias = "server", + after_help = "\ CONTEXT FOR AGENTS: - Auto-installs the version if it is not already present. - Symlinks `~/.local/bin/clickhouse` to it, putting `clickhouse client`, `clickhouse benchmark` - and `clickhouse format` on PATH; --no-global skips that. - Sets the version used by `clickhousectl local client` and `clickhousectl local clickhouse`.")] - Use { - /// Version to set as default ("latest", "stable", "lts", 25.12, or 25.12.5.44) - version: UseVersionArg, - - /// Do not create or update the ~/.local/bin/clickhouse symlink - #[arg(long)] - no_global: bool, + Covers ClickHouse only; `local postgres` manages Postgres and `local status` lists both. + Data persists across stop/start; only `remove` deletes it. + Retain the name `start` returns (it may be generated) for later `stop`/`remove`. + `local remove ` deletes an installed binary, not server data. + Custom configs inherit built-in defaults; find available names with `clickhouse configs`. + Typical flow: `clickhouse start dev` -> `local client dev` -> `clickhouse stop dev`" + )] + Clickhouse { + #[command(subcommand)] + command: ServerCommands, }, - /// Remove an installed ClickHouse version - #[command(after_help = "\ + /// Manage local Postgres instances (Docker-backed) + #[command( + visible_alias = "pg", + after_help = "\ CONTEXT FOR AGENTS: - Fails if any server is running on this version, in any project — versions are shared, so a server - started from another directory blocks removal too (`local clickhouse list --global` lists them). - Fails if the version is the current default; switch with `clickhousectl local use ` first. - --force overrides both guards. The exact build may not be re-downloadable.")] - Remove { - /// Exact installed version to remove (not "latest"/"stable"/"lts") - // Keep this opaque: removal matches an installed directory name instead of resolving a version spec. - version: String, - - /// Stop any server using this version and remove it even if it is the default - /// - /// Servers in other projects are stopped too; clears ~/.clickhouse/default and - /// the ~/.local/bin/clickhouse symlink when the default is removed. - #[arg(long)] - force: bool, + Requires Docker installed and running. + Each instance is keyed on (name, major version); pass --version when one name has two majors. + Covers Postgres only; `local status` lists ClickHouse and Postgres together. + Typical flow: `postgres start` -> `postgres client` -> `postgres dotenv --local` -> `postgres stop`" + )] + Postgres { + #[command(subcommand)] + command: PostgresCommands, }, - /// Show the current default ClickHouse version - Which, - - /// Initialize a project directory for ClickHouse and Postgres - #[command(after_help = "\ -CONTEXT FOR AGENTS: - `.clickhouse/` holds runtime data and is git-ignored; the `clickhouse/` and `postgres/` SQL - scaffolds are meant to be committed. - Idempotent — re-running only creates what is missing. - Next: `clickhousectl local clickhouse start`")] - Init, + /// List ClickHouse and Postgres instances in this project + Status, /// Connect to a running ClickHouse server with clickhouse-client #[command( @@ -322,41 +297,66 @@ CONTEXT FOR AGENTS: args: Vec, }, - /// Manage local ClickHouse servers - #[command( - visible_alias = "ch", - alias = "server", - after_help = "\ + /// Install a ClickHouse version or Postgres image + #[command(after_help = INSTALL_AFTER_HELP)] + Install { + /// Version ("latest", "stable", "lts", 25.12, 25.12.9.61) or image selector (postgres@18) + version: InstallVersionArg, + + /// Re-install even if already installed + #[arg(long)] + force: bool, + }, + + /// List installed or available ClickHouse versions + #[command(after_help = "\ CONTEXT FOR AGENTS: - Covers ClickHouse only; `local postgres` manages Postgres and `local status` lists both. - Data persists across stop/start; only `remove` deletes it. - Retain the name `start` returns (it may be generated) for later `stop`/`remove`. - `local remove ` deletes an installed binary, not server data. - Custom configs inherit built-in defaults; find available names with `clickhouse configs`. - Typical flow: `clickhouse start dev` -> `local client dev` -> `clickhouse stop dev`" - )] - Clickhouse { - #[command(subcommand)] - command: ServerCommands, + Default output is exact installed version strings — the form `clickhousectl local remove` needs. + --remote lists minor versions (e.g. 26.3) probed from builds.clickhouse.com, not exact builds.")] + List { + /// List minor versions available for download + #[arg(long)] + remote: bool, }, - /// Manage local Postgres instances (Docker-backed) - #[command( - visible_alias = "pg", - after_help = "\ + /// Set the default ClickHouse version + #[command(after_help = "\ CONTEXT FOR AGENTS: - Requires Docker installed and running. - Each instance is keyed on (name, major version); pass --version when one name has two majors. - Covers Postgres only; `local status` lists ClickHouse and Postgres together. - Typical flow: `postgres start` -> `postgres client` -> `postgres dotenv --local` -> `postgres stop`" - )] - Postgres { - #[command(subcommand)] - command: PostgresCommands, + Auto-installs the version if it is not already present. + Symlinks `~/.local/bin/clickhouse` to it, putting `clickhouse client`, `clickhouse benchmark` + and `clickhouse format` on PATH; --no-global skips that. + Sets the version used by `clickhousectl local client` and `clickhousectl local clickhouse`.")] + Use { + /// Version to set as default ("latest", "stable", "lts", 25.12, or 25.12.5.44) + version: UseVersionArg, + + /// Do not create or update the ~/.local/bin/clickhouse symlink + #[arg(long)] + no_global: bool, }, - /// List ClickHouse and Postgres instances in this project - Status, + /// Show the current default ClickHouse version + Which, + + /// Remove an installed ClickHouse version + #[command(after_help = "\ +CONTEXT FOR AGENTS: + Fails if any server is running on this version, in any project — versions are shared, so a server + started from another directory blocks removal too (`local clickhouse list --global` lists them). + Fails if the version is the current default; switch with `clickhousectl local use ` first. + --force overrides both guards. The exact build may not be re-downloadable.")] + Remove { + /// Exact installed version to remove (not "latest"/"stable"/"lts") + // Keep this opaque: removal matches an installed directory name instead of resolving a version spec. + version: String, + + /// Stop any server using this version and remove it even if it is the default + /// + /// Servers in other projects are stopped too; clears ~/.clickhouse/default and + /// the ~/.local/bin/clickhouse symlink when the default is removed. + #[arg(long)] + force: bool, + }, /// Manage executable UDFs for local servers #[command(after_help = "\