From 349928bffbb5a8414a5a73fb1a1b0782dde39121 Mon Sep 17 00:00:00 2001 From: Tim-1e <72056245+Tim-1e@users.noreply.github.com> Date: Wed, 15 Jul 2026 22:29:03 +0800 Subject: [PATCH 1/2] feat(cli): align parse help across Python and Rust --- .../python/src/holocubic_cli_python/cli.py | 99 +++++++++++++------ implementations/python/tests/test_cli.py | 30 ++++++ implementations/rust/src/main.rs | 89 +++++++++++++++-- implementations/rust/tests/cli_help.rs | 59 +++++++++++ 4 files changed, 240 insertions(+), 37 deletions(-) create mode 100644 implementations/rust/tests/cli_help.rs diff --git a/implementations/python/src/holocubic_cli_python/cli.py b/implementations/python/src/holocubic_cli_python/cli.py index 084ffe3..703abb8 100644 --- a/implementations/python/src/holocubic_cli_python/cli.py +++ b/implementations/python/src/holocubic_cli_python/cli.py @@ -29,6 +29,32 @@ from .url import normalize_device_url +class _HelpAfterErrorParser(argparse.ArgumentParser): + """Keep argparse semantics while showing the relevant command help on errors.""" + + _missing_command: str | None = None + + def show_help_when_command_is_missing(self, destination: str) -> None: + self._missing_command = destination + + def error(self, message: str) -> None: + if message == f"the following arguments are required: {self._missing_command}": + self.print_help(sys.stderr) + self.exit(2) + print(f"{self.prog}: error: {message}", file=sys.stderr) + print(file=sys.stderr) + self.print_help(sys.stderr) + self.exit(2) + + +def _add_command( + subparsers: Any, name: str, description: str, **kwargs: Any +) -> argparse.ArgumentParser: + return subparsers.add_parser( + name, help=description, description=description, **kwargs + ) + + def _positive_integer(value: str) -> int: try: parsed = int(value) @@ -70,7 +96,7 @@ def _add_transfer_options( def build_parser() -> argparse.ArgumentParser: - parser = argparse.ArgumentParser( + parser = _HelpAfterErrorParser( prog="cubic-py", description="Manage HoloCubic DevTools devices and SD-card files", ) @@ -94,76 +120,89 @@ def build_parser() -> argparse.ArgumentParser: ) parser.add_argument("--config", help="override the configuration file") commands = parser.add_subparsers(dest="command", required=True) + parser.show_help_when_command_is_missing("command") - device = commands.add_parser("device", help="manage saved devices") + device = _add_command(commands, "device", "manage saved devices") device_commands = device.add_subparsers(dest="device_command", required=True) - device_add = device_commands.add_parser("add", help="verify and save a device") + device.show_help_when_command_is_missing("device_command") + device_add = _add_command(device_commands, "add", "verify and save a device") device_add.add_argument("name") device_add.add_argument("device_host") device_add.add_argument("--no-use", action="store_false", dest="use", default=True) - device_commands.add_parser("list", help="list saved devices") - device_use = device_commands.add_parser("use", help="select a saved device") + _add_command(device_commands, "list", "list saved devices") + device_use = _add_command(device_commands, "use", "select a saved device") device_use.add_argument("name") - device_remove = device_commands.add_parser("remove", help="remove a saved device") + device_remove = _add_command(device_commands, "remove", "remove a saved device") device_remove.add_argument("name") - commands.add_parser("ping", help="test the selected device") - commands.add_parser("info", help="show device capabilities and transfer limits") - list_parser = commands.add_parser("ls", help="list a remote directory") + _add_command(commands, "ping", "test the selected device") + _add_command(commands, "info", "show device capabilities and transfer limits") + list_parser = _add_command(commands, "ls", "list a remote directory") list_parser.add_argument("remote", nargs="?") - stat_parser = commands.add_parser( - "stat", help="show remote file or directory metadata" + stat_parser = _add_command( + commands, "stat", "show remote file or directory metadata" ) stat_parser.add_argument("remote") - cat_parser = commands.add_parser("cat", help="write a remote file to stdout") + cat_parser = _add_command(commands, "cat", "write a remote file to stdout") cat_parser.add_argument("remote") - mkdir_parser = commands.add_parser( - "mkdir", help="create a remote directory and missing parents" + mkdir_parser = _add_command( + commands, "mkdir", "create a remote directory and missing parents" ) mkdir_parser.add_argument("remote") - move_parser = commands.add_parser("mv", help="rename or move a remote path") + move_parser = _add_command(commands, "mv", "rename or move a remote path") move_parser.add_argument("source") move_parser.add_argument("target") - remove_parser = commands.add_parser("rm", help="remove a remote file or directory") + remove_parser = _add_command(commands, "rm", "remove a remote file or directory") remove_parser.add_argument("remote") remove_parser.add_argument("-r", "--recursive", action="store_true") remove_parser.add_argument("-y", "--yes", action="store_true") - push_parser = commands.add_parser( - "push", aliases=["upload"], help="upload a file or directory recursively" + push_parser = _add_command( + commands, + "push", + "upload a file or directory recursively", + aliases=["upload"], ) push_parser.add_argument("local") push_parser.add_argument("remote", nargs="?") _add_transfer_options(push_parser) - pull_parser = commands.add_parser( - "pull", aliases=["download"], help="download a file or directory recursively" + pull_parser = _add_command( + commands, + "pull", + "download a file or directory recursively", + aliases=["download"], ) pull_parser.add_argument("remote") pull_parser.add_argument("local", nargs="?") _add_transfer_options(pull_parser, download=True) - devrun = commands.add_parser("devrun", help="read, save, or run DevRun source") + devrun = _add_command(commands, "devrun", "read, save, or run DevRun source") devrun_commands = devrun.add_subparsers(dest="devrun_command", required=True) - devrun_read = devrun_commands.add_parser("read", help="read DevRun source") + devrun.show_help_when_command_is_missing("devrun_command") + devrun_read = _add_command(devrun_commands, "read", "read DevRun source") devrun_read.add_argument("output", nargs="?") devrun_read.add_argument("-f", "--force", action="store_true") for name in ("save", "run"): - child = devrun_commands.add_parser( - name, help=f"{'save and run' if name == 'run' else 'save'} DevRun source" + description = f"{'save and run' if name == 'run' else 'save'} DevRun source" + child = _add_command( + devrun_commands, + name, + description, ) child.add_argument("file") - app = commands.add_parser("app", help="list and install SD-card apps") + app = _add_command(commands, "app", "list and install SD-card apps") app_commands = app.add_subparsers(dest="app_command", required=True) - app_commands.add_parser("list", help="list installed apps") - app_install = app_commands.add_parser( - "install", help="validate and upload an app directory" + app.show_help_when_command_is_missing("app_command") + _add_command(app_commands, "list", "list installed apps") + app_install = _add_command( + app_commands, "install", "validate and upload an app directory" ) app_install.add_argument("directory") app_install.add_argument("--id") app_install.add_argument("-f", "--force", action="store_true") - app_remove = app_commands.add_parser( - "remove", help="remove an installed app directory" + app_remove = _add_command( + app_commands, "remove", "remove an installed app directory" ) app_remove.add_argument("id") app_remove.add_argument("-y", "--yes", action="store_true", required=True) diff --git a/implementations/python/tests/test_cli.py b/implementations/python/tests/test_cli.py index df41c39..3bad11b 100644 --- a/implementations/python/tests/test_cli.py +++ b/implementations/python/tests/test_cli.py @@ -111,6 +111,36 @@ def test_usage_errors_exit_two_and_json_stdout_is_clean(self) -> None: self.assertEqual(result.returncode, 2) self.assertEqual(result.stdout, "") + def test_parse_errors_show_full_relevant_help(self) -> None: + with tempfile.TemporaryDirectory() as directory: + cwd = Path(directory) + config = cwd / "config.json" + cases = ( + ((), False, "Manage HoloCubic DevTools", "device"), + ( + ("unknown-command",), + True, + "Manage HoloCubic DevTools", + "device", + ), + (("stat",), True, "usage: cubic-py stat", "remote"), + (("device",), False, "manage saved devices", "add"), + ( + ("--timeout", "invalid", "info"), + True, + "Manage HoloCubic DevTools", + "device", + ), + ) + for arguments, expect_error, help_marker, command_marker in cases: + with self.subTest(arguments=arguments): + result = self.run_cli(cwd, config, *arguments, check=False) + self.assertEqual(result.returncode, 2) + self.assertEqual(result.stdout, "") + self.assertEqual("error:" in result.stderr, expect_error) + self.assertIn(help_marker, result.stderr) + self.assertIn(command_marker, result.stderr) + if __name__ == "__main__": unittest.main() diff --git a/implementations/rust/src/main.rs b/implementations/rust/src/main.rs index 4d16419..2a44bca 100644 --- a/implementations/rust/src/main.rs +++ b/implementations/rust/src/main.rs @@ -1,10 +1,13 @@ +use std::env; +use std::ffi::OsString; use std::fs::{self, OpenOptions}; use std::io::{self, Write}; use std::path::PathBuf; use std::process::ExitCode; use std::time::Instant; -use clap::{ArgAction, Args, Parser, Subcommand}; +use clap::error::{ContextKind, ErrorKind}; +use clap::{ArgAction, Args, Command, CommandFactory, Parser, Subcommand}; use holocubic_cli_rust::app::{validate_app_directory, validate_app_id}; use holocubic_cli_rust::client::{CubicClient, public_info}; use holocubic_cli_rust::config::{ @@ -75,6 +78,78 @@ struct Cli { command: Commands, } +fn help_command_for_args(args: &[OsString]) -> Command { + let mut command = Cli::command(); + let mut skip_option_value = false; + + for argument in args.iter().skip(1) { + if skip_option_value { + skip_option_value = false; + continue; + } + + let value = argument.to_string_lossy(); + match value.as_ref() { + "-H" | "--host" | "--timeout" | "--config" | "--retries" | "--max-depth" + | "--max-entries" | "--max-bytes" | "--id" => { + skip_option_value = true; + continue; + } + "--json" | "--quiet" | "--force" | "-f" | "--recursive" | "-r" | "--yes" | "-y" + | "--no-use" => continue, + "--" | "--help" | "-h" | "--version" | "-V" => break, + _ if value.starts_with("--host=") + || value.starts_with("--timeout=") + || value.starts_with("--config=") + || value.starts_with("--retries=") + || value.starts_with("--max-depth=") + || value.starts_with("--max-entries=") + || value.starts_with("--max-bytes=") + || value.starts_with("--id=") + || (value.starts_with("-H") && value.len() > 2) => + { + continue; + } + _ if value.starts_with('-') => break, + _ => {} + } + + let Some(subcommand) = command.find_subcommand(argument).cloned() else { + break; + }; + command = subcommand; + } + + command +} + +fn print_parse_error(mut error: clap::Error, args: &[OsString]) -> ExitCode { + let code = error.exit_code(); + if matches!( + error.kind(), + ErrorKind::DisplayHelp + | ErrorKind::DisplayVersion + | ErrorKind::DisplayHelpOnMissingArgumentOrSubcommand + ) { + let _ = error.print(); + return ExitCode::from(code as u8); + } + + error.remove(ContextKind::Usage); + let rendered = error.render().to_string(); + let summary = rendered + .split("\n\nFor more information") + .next() + .unwrap_or(&rendered) + .trim_end(); + eprintln!("{summary}\n"); + + let mut help = help_command_for_args(args); + let _ = help.write_long_help(&mut io::stderr()); + eprintln!(); + ExitCode::from(code as u8) +} + #[derive(Subcommand)] enum Commands { /// Manage saved devices. @@ -811,13 +886,13 @@ impl<'a> Runtime<'a> { } fn main() -> ExitCode { - let cli = match Cli::try_parse() { + let mut args: Vec = env::args_os().collect(); + if let Some(executable) = args.first_mut() { + *executable = OsString::from("cubic-rs"); + } + let cli = match Cli::try_parse_from(&args) { Ok(cli) => cli, - Err(error) => { - let code = error.exit_code(); - let _ = error.print(); - return ExitCode::from(code as u8); - } + Err(error) => return print_parse_error(error, &args), }; match Runtime::new(&cli).and_then(|runtime| runtime.execute()) { Ok(()) => ExitCode::SUCCESS, diff --git a/implementations/rust/tests/cli_help.rs b/implementations/rust/tests/cli_help.rs new file mode 100644 index 0000000..f8eeceb --- /dev/null +++ b/implementations/rust/tests/cli_help.rs @@ -0,0 +1,59 @@ +use std::process::{Command, Output}; + +fn run_cli(arguments: &[&str]) -> Output { + Command::new(env!("CARGO_BIN_EXE_cubic-rs")) + .args(arguments) + .output() + .expect("run cubic-rs") +} + +fn assert_parse_help(arguments: &[&str], expect_error: bool, markers: &[&str]) { + let output = run_cli(arguments); + assert_eq!(output.status.code(), Some(2)); + assert!(output.stdout.is_empty()); + + let stderr = String::from_utf8(output.stderr).expect("UTF-8 stderr"); + assert!( + !stderr.contains("cubic-rs.exe"), + "help should use the stable executable name:\n{stderr}" + ); + assert_eq!( + stderr.contains("error:"), + expect_error, + "unexpected parse-error state:\n{stderr}" + ); + for marker in markers { + assert!( + stderr.contains(marker), + "missing help marker {marker:?}:\n{stderr}" + ); + } +} + +#[test] +fn parse_errors_show_full_relevant_help() { + assert_parse_help( + &[], + false, + &["Manage HoloCubic DevTools", "Commands:", "device"], + ); + assert_parse_help( + &["unknown-command"], + true, + &["Manage HoloCubic DevTools", "Commands:", "device"], + ); + assert_parse_help( + &["stat"], + true, + &[ + "Show remote file or directory metadata", + "Usage:", + "", + ], + ); + assert_parse_help( + &["device"], + false, + &["Manage saved devices", "Commands:", "add"], + ); +} From 56846ac32e98481fe3d3bd52b22e81feec767029 Mon Sep 17 00:00:00 2001 From: Tim-1e <72056245+Tim-1e@users.noreply.github.com> Date: Wed, 15 Jul 2026 22:29:03 +0800 Subject: [PATCH 2/2] ci(python): add PyPI trusted publishing --- .github/workflows/publish-python.yml | 61 ++++++++++++++++++++++++++++ docs/RELEASING.md | 51 ++++++++++++++++++----- 2 files changed, 101 insertions(+), 11 deletions(-) create mode 100644 .github/workflows/publish-python.yml diff --git a/.github/workflows/publish-python.yml b/.github/workflows/publish-python.yml new file mode 100644 index 0000000..1a0858b --- /dev/null +++ b/.github/workflows/publish-python.yml @@ -0,0 +1,61 @@ +name: Publish Python package + +on: + push: + tags: + - "python-v*" + +permissions: + contents: read + +concurrency: + group: publish-python-${{ github.ref }} + cancel-in-progress: false + +jobs: + publish: + if: github.repository == 'Tim-1e/holocubic-cli' + runs-on: ubuntu-latest + environment: + name: pypi + url: https://pypi.org/project/holocubic-cli-python/ + permissions: + contents: read + id-token: write + defaults: + run: + working-directory: implementations/python + + steps: + - uses: actions/checkout@v6 + - name: Install uv + uses: astral-sh/setup-uv@08807647e7069bb48b6ef5acd8ec9567f424441b # v8.1.0 + - name: Install Python 3.13 + run: uv python install 3.13 + - name: Verify release tag + shell: bash + run: | + VERSION="$(uv version --short)" + EXPECTED_TAG="python-v${VERSION}" + if [[ "${GITHUB_REF_NAME}" != "${EXPECTED_TAG}" ]]; then + echo "Expected tag ${EXPECTED_TAG}, got ${GITHUB_REF_NAME}" >&2 + exit 1 + fi + - name: Sync locked environment + run: uv sync --locked + - name: Lint + run: uvx ruff check src tests + - name: Check formatting + run: uvx ruff format --check src tests + - name: Test + run: uv run --locked python -m unittest discover -s tests -v + - name: Build distributions + run: uv build --clear --no-sources + - name: Check distribution metadata + run: uvx twine check dist/* + - name: Smoke-test wheel + run: uv run --isolated --no-project --with dist/*.whl cubic-py --version + - name: Smoke-test source distribution + run: uv run --isolated --no-project --with dist/*.tar.gz cubic-py --version + - name: Publish to PyPI with OIDC + run: uv publish --trusted-publishing always dist/* diff --git a/docs/RELEASING.md b/docs/RELEASING.md index 1c0ecb1..795202e 100644 --- a/docs/RELEASING.md +++ b/docs/RELEASING.md @@ -44,28 +44,57 @@ Configure npm Trusted Publishing with these exact values: ## Python to PyPI +The public package is `holocubic-cli-python` and uses PyPI Trusted Publishing. Confirm the name and version in `implementations/python/pyproject.toml`, then build and inspect both distributions: ```sh cd implementations/python -python -m pip install --upgrade build twine -python -m pip install --editable . -python -m unittest discover -s tests -v -python -m build -python -m twine check dist/* +uv sync --locked +uvx ruff check src tests +uvx ruff format --check src tests +uv run --locked python -m unittest discover -s tests -v +uv build --clear --no-sources +uvx twine check dist/* +uv publish --dry-run dist/* ``` -TestPyPI and production PyPI use separate accounts and credentials: +Smoke-test the built wheel and source distribution rather than the editable +checkout. For example: ```sh -python -m twine upload --repository testpypi dist/* -python -m twine upload dist/* +uv run --isolated --no-project --with dist/*.whl cubic-py --version +uv run --isolated --no-project --with dist/*.tar.gz cubic-py --version ``` -Verify installation in a clean virtual environment after each upload. For -later releases, add PyPI Trusted Publishing through GitHub Actions instead of -storing a long-lived API token. +After confirming the version, create and push a tag that exactly matches +`python-v`. For example, version `0.1.0` must use: + +```sh +git tag python-v0.1.0 +git push origin python-v0.1.0 +``` + +The tag starts `.github/workflows/publish-python.yml`. The workflow rejects a +tag that differs from `pyproject.toml`, repeats the checks above, tests both +distribution formats, and publishes through OIDC without a `PYPI_TOKEN` +secret. + +Configure the PyPI Trusted Publisher with these exact values: + +| Field | Value | +| --- | --- | +| PyPI project | `holocubic-cli-python` | +| Owner | `Tim-1e` | +| Repository | `holocubic-cli` | +| Workflow filename | `publish-python.yml` | +| Environment name | `pypi` | + +For an existing project, add the publisher under `Manage` → `Publishing`. For +a project that has not been created yet, the same values can be registered as +a pending publisher under the account-level `Publishing` page. TestPyPI is a +separate registry and requires its own account, credentials, and trusted +publisher configuration. ## Rust to crates.io