Skip to content

fix: honor defaults on repeatable positionals - #515

Merged
codeforester merged 2 commits into
mainfrom
bug/507-20260917-bug-honor-defaults-on-repeatable-positional-declarations
Sep 18, 2026
Merged

codeforester merged 2 commits into
mainfrom
bug/507-20260917-bug-honor-defaults-on-repeatable-positional-declarations

Conversation

@codeforester

Copy link
Copy Markdown
Collaborator

Summary

Apply and validate a repeatable positional's default only when no values were supplied. A default satisfies required=true; an explicit empty argument remains an actual supplied value. Document the contract and cover imperative/table declarations, enum and validator rejection, preceding positionals, and explicit values.

Issue

Fixes #507

Validation

  • bats lib/bash/cli/tests/lib_cli.bats — passed (37 tests).
  • git diff --check — passed.
  • Full repository hosted validation is pending.

Demo Impact

None.

Docs Impact

Clarify repeatable positional default and required semantics in the CLI README.

API Impact

Previously ignored defaults for repeatable positionals are now observable in parsed results.

CI Impact

Adds direct and declarative regression coverage.

Security Notes

None.

@codeforester

Copy link
Copy Markdown
Collaborator Author

Two findings from review:

1. lib/bash/cli/lib_cli.sh:1307 — The repeatable positional's default is appended to BASE_BASH_LIBS_CLI_RESULT_POSITIONALS before it is validated, so a failing default leaves a rejected value sitting in the published result array even though parsing returns an error. Reproduction: declare base_cli_positional m run values repeatable=true default=fallback validator=valid_archive where valid_archive only accepts "archive", then call base_cli_parse m -- run with zero values. base_cli_parse returns 2 (correct), but BASE_BASH_LIBS_CLI_RESULT_POSITIONALS ends up containing the invalid "fallback" value. Any caller that inspects the result array instead of strictly gating on the return code sees a value that was in fact rejected. The new bats test for this exact scenario only checks stderr/status, not the leftover array state.

2. lib/bash/cli/lib_cli.sh:1305 — The new default-application block for repeatable positionals duplicates the existing block for non-repeatable positionals (lines 1317-1321) almost verbatim instead of sharing one helper (check default +set, append, validate, increment index, else check required and error). Not a crash, but a maintenance cost — a future fix to one (e.g. fixing the append-before-validate ordering in finding 1) is likely to be applied to only one copy, silently reintroducing the bug in the other. Extracting a shared __base_bash_libs_cli_apply_positional_default__ helper used by both call sites would remove the duplication.

@codeforester

Copy link
Copy Markdown
Collaborator Author

Addressed in commit 23ad69d: scalar and repeatable positional defaults now share one validation path, and a default is validated before it is appended to the published result array. Regression coverage verifies invalid defaults leave results empty. bats lib/bash/cli/tests/lib_cli.bats passes (37 tests).

@codeforester

Copy link
Copy Markdown
Collaborator Author

Validation note: the initial hosted macOS suite failed in the unrelated base_std_run reports external timeout-clock failures test in untouched lib_std code (the expected command counter file was absent). I reproduced the test locally in isolation and it passes. I requeued the failed hosted job; that rerun is currently queued.

@codeforester

Copy link
Copy Markdown
Collaborator Author

Confirmed fixed. The new shared __base_bash_libs_cli_apply_positional_default__() helper validates a default before publishing it, for both the repeatable and non-repeatable paths — reproduced the original failing-default repro and confirmed BASE_BASH_LIBS_CLI_RESULT_POSITIONALS no longer contains a rejected value when parse returns nonzero. This also resolved the duplication finding as a side effect (both paths now share one helper). 37/37 tests pass.

@codeforester
codeforester merged commit 711036d into main Sep 18, 2026
18 of 19 checks passed
@codeforester
codeforester deleted the bug/507-20260917-bug-honor-defaults-on-repeatable-positional-declarations branch September 18, 2026 17:15
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.

bug: honor defaults on repeatable positional declarations

1 participant