From f3ec255f8fe5f27ea29ddfc49831e5fa8d15be68 Mon Sep 17 00:00:00 2001 From: Ramesh Padmanabhaiah <22363102+codeforester@users.noreply.github.com> Date: Thu, 10 Sep 2026 13:27:08 +0530 Subject: [PATCH] refactor(repo): migrate clone parsing to shared helper --- cli/bash/commands/basectl/subcommands/repo.sh | 50 +++++++++++++------ cli/bash/commands/basectl/tests/repo.bats | 25 ++++++++++ docs/base-bash-libs-migration-matrix.md | 1 + 3 files changed, 62 insertions(+), 14 deletions(-) diff --git a/cli/bash/commands/basectl/subcommands/repo.sh b/cli/bash/commands/basectl/subcommands/repo.sh index 1be167ae..75bb37ec 100644 --- a/cli/bash/commands/basectl/subcommands/repo.sh +++ b/cli/bash/commands/basectl/subcommands/repo.sh @@ -7,6 +7,7 @@ readonly _base_repo_subcommand_sourced import_base_lib git/lib_git.sh import_base_lib gh/lib_gh.sh import_base_lib str/lib_str.sh +import_base_lib arg/lib_arg.sh source "$BASE_HOME/cli/bash/commands/basectl/subcommands/github_policy.sh" # shellcheck source=cli/bash/commands/basectl/subcommands/inspection_json.sh @@ -2198,6 +2199,15 @@ base_repo_clone() { local spec="" local status local target + local -a parser_args=() positionals=() + # shellcheck disable=SC2034 # base_arg_parse receives caller-owned arrays by name. + local -a option_specs=( + "owner|value|--owner" + "path|value|--path" + "dry_run|flag|--dry-run" + "verbose|flag|-v" + ) + local -A parsed_options=() while (($#)); do case "$1" in @@ -2210,11 +2220,11 @@ base_repo_clone() { base_repo_clone_usage_error "Option '--owner' requires an argument." return $? } - owner="$2" + parser_args+=("$1" "$2") shift 2 ;; --owner=*) - owner="${1#--owner=}" + parser_args+=("$1") shift ;; --path) @@ -2222,20 +2232,19 @@ base_repo_clone() { base_repo_clone_usage_error "Option '--path' requires an argument." return $? } - path="$2" + parser_args+=("$1" "$2") shift 2 ;; --path=*) - path="${1#--path=}" + parser_args+=("$1") shift ;; --dry-run) - dry_run=1 + parser_args+=("$1") shift ;; -v) - base_std_set_log_level DEBUG - export BASE_BASH_LIBS_LOG_DEBUG=1 + parser_args+=("$1") shift ;; -*) @@ -2243,20 +2252,33 @@ base_repo_clone() { return $? ;; *) - if [[ -n "$spec" ]]; then - base_repo_clone_usage_error "The 'repo clone' command accepts exactly one repository name." - return $? - fi - spec="$1" + parser_args+=("$1") shift ;; esac done - [[ -n "$spec" ]] || { + if ! base_arg_parse parsed_options positionals option_specs -- "${parser_args[@]}"; then + base_repo_clone_usage_error "Could not parse repo clone arguments." + return $? + fi + if ((${#positionals[@]} == 0)); then base_repo_clone_usage_error "Repository name is required." return $? - } + fi + if ((${#positionals[@]} > 1)); then + base_repo_clone_usage_error "The 'repo clone' command accepts exactly one repository name." + return $? + fi + + spec="${positionals[0]}" + owner="${parsed_options[owner]:-}" + path="${parsed_options[path]:-}" + dry_run="${parsed_options[dry_run]:-0}" + if [[ "${parsed_options[verbose]:-0}" == "1" ]]; then + base_std_set_log_level DEBUG + export BASE_BASH_LIBS_LOG_DEBUG=1 + fi if [[ "$spec" == */* ]]; then [[ "$spec" != */*/* ]] || { diff --git a/cli/bash/commands/basectl/tests/repo.bats b/cli/bash/commands/basectl/tests/repo.bats index 982cbdf5..e41d7555 100644 --- a/cli/bash/commands/basectl/tests/repo.bats +++ b/cli/bash/commands/basectl/tests/repo.bats @@ -339,6 +339,31 @@ EOF [ ! -e "$repo_dir" ] } +@test "basectl repo clone preserves shared parser duplicate values" { + local repo_dir="$TEST_TMPDIR/duplicate-options/base-demo" + + run_basectl repo clone base-demo \ + --owner ignored \ + --owner codeforester \ + --path "$TEST_TMPDIR/duplicate-options/ignored" \ + --path "$repo_dir" \ + --dry-run \ + --dry-run + + [ "$status" -eq 0 ] + [ "$(line_at "$output" 1)" = "[DRY-RUN] Would clone codeforester/base-demo (git@github.com:codeforester/base-demo.git) into $repo_dir." ] + [ "$(line_at "$output" 2)" = "[DRY-RUN] Would run: gh repo clone codeforester/base-demo $repo_dir" ] + [ ! -e "$repo_dir" ] +} + +@test "basectl repo clone keeps help precedence over invalid repository input" { + run_basectl repo clone invalid/repository/name --help + + [ "$status" -eq 0 ] + [[ "$output" == *"basectl repo clone [options]"* ]] + [[ "$output" != *"Repository must be"* ]] +} + @test "basectl repo clone supports explicit owner slash repo and https clone protocol" { local repo_dir="$TEST_TMPDIR/custom/bankbuddy" diff --git a/docs/base-bash-libs-migration-matrix.md b/docs/base-bash-libs-migration-matrix.md index aa148974..33652a10 100644 --- a/docs/base-bash-libs-migration-matrix.md +++ b/docs/base-bash-libs-migration-matrix.md @@ -50,6 +50,7 @@ Every migration or parser-adjacent change must record: | `devcontainer.sh` | Optional project positional; `-v`, `--workspace `, `--format `, and `--write`. No forwarded-argument boundary. | Help is immediate. Parser and positional failures print usage/error text and return 2. Formats are validated locally; normal results are delegated to the setup layer. | `cli/bash/commands/basectl/tests/devcontainer.bats` | Migrated; preserve the parser seam and workspace/project validation. | | `devenv_report.sh` | Optional project positional; `-v`, `--workspace `, and `--format `. No forwarded-argument boundary. | Help is immediate. Parser, positional, format, and workspace/project failures return 2 with usage/error text; report output is delegated. | `cli/bash/commands/basectl/tests/devenv-report.bats` | Migrated; preserve the parser seam and workspace/project validation. | | `export_context.sh` | Optional project positional; `-v`, `--workspace `, `--format `, `--output `, `--print`, and `--list-files`. No forwarded-argument boundary. | Help is immediate. Invalid parser, positional, format, or project context returns 2 with usage/error text. Export content belongs to the delegated layer. | `cli/bash/commands/basectl/tests/export-context.bats` | Migrated; preserve output-mode and project-context validation. | +| `repo.sh` (`repo clone`) | Exactly one repository name positional; `--owner `, `--path `, `--dry-run`, and `-v`. No forwarded-argument boundary; the top-level wrapper rejects equals-form options before delegation. | `-h`, `--help`, and `help` are immediate. Missing values, unknown options, and duplicate positionals return 2 with the existing usage/error text; duplicate values keep the last value and duplicate flags remain accepted. Clone URL/path resolution and `gh repo clone` delegation remain local. | `cli/bash/commands/basectl/tests/repo.bats` clone cases | Migrated in the #2144 leaf; preserve owner inference, destination safety, dry-run output, and GitHub delegation. | | `gh.sh` (`auth status` / `auth refresh`) | `auth status` owns `--hostname `. `auth refresh` owns `--hostname `, repeatable `--scope `, comma-separated `--scopes `, and `--clipboard`; the pre-parser normalizes both scope spellings into the shared repeatable option. No forwarded-argument boundary. | Help remains command-scoped. Missing values and unknown options return 2 with the existing usage/error text; duplicate hostname values keep the last value; scope order is preserved. Unsupported equals-form options remain rejected, matching the legacy contract. GitHub CLI output and delegated status remain unchanged. | `cli/bash/commands/basectl/tests/gh.bats` auth cases | Migrated in the #2144 leaf; preserve hostname handling, scope ordering, and credential safety boundaries. | | `gh_branch_worktree.sh` (`branch stale`) | No positionals; `--days ` and `--format `. No forwarded-argument boundary; the pre-parser preserves the legacy space-separated value forms before shared parsing. | Help is immediate. Duplicate values keep the last value. Invalid days and unsupported formats return 2; valid JSON format selection keeps usage and upstream failures inside the inspection envelope. Unsupported equals-form options remain rejected. | `cli/bash/commands/basectl/tests/gh-branch-worktree.bats` and `inspection-json.bats` | Migrated in the #2144 leaf; preserve stale-branch age validation, JSON envelope/status, and Git reference inspection boundaries. | | `gh_branch_worktree.sh` (`branch prune`) | No positionals; `--dry-run`, `--yes`, `--remote`, and `--closed-unmerged`. No forwarded-argument boundary. | Help and unknown options remain immediate. Duplicate flags remain accepted. `--dry-run` and `--yes` remain mutually exclusive; dry-run is the default. Branch/worktree eligibility, GitHub verification, output, and deletion status remain local to the pruning layer. | `cli/bash/commands/basectl/tests/gh-branch-worktree.bats` prune cases | Migrated in the #2144 leaf; preserve default dry-run and all deletion safety boundaries. |