Keep the [@] array expansion when forwarding wrapped multi-valued positionals

[gdevenyi]

Fixes #205

ARG_POSITIONAL_INF (and thus ARG_LEFTOVERS) and ARG_POSITIONAL_MULTI passed ${_arg_name[@]} to _POS_WRAPPED with the [@] unquoted, so m4 treated the square brackets as quotes and a wrapper script (ARGBASH_WRAP) that wraps such a script ended up forwarding "${_arg_name@}" - which is exactly the symptom reported in #205 (the reporter used ARG_LEFTOVERS).

Reproduction on master: wrap a script that has ARG_LEFTOVERS and look at the assign_positional_args function of the wrapper:

_args_lib_pos+=(${_arg_leftovers@})

With this change the wrapper forwards ("${_arg_leftovers[@]}"), the same way ARG_POSITIONAL_SINGLE values are already forwarded.

New regression test test-wrapleftovers (a wrapper of a script that declares ARG_LEFTOVERS).

Tested with make check (the only failures are the pre-existing shellcheck SC2154 ones in the wrapping tests that #207 addresses).

🤖 Generated with Claude Code

https://claude.ai/code/session_017DxhC3HmTM7TQUzkBZnyH2

[Copilot]

🟡 Changes recommended

Four moderate findings remain around empty-leftovers handling and boundary-sensitive ARG_POSITIONAL_MULTI coverage.

Get a fresh assessment by requesting another Copilot review.

Pull request overview

Fixes wrapped forwarding of multi-valued positional arguments by preserving ${name[@]} through m4.

Changes:

• Preserves array expansion for ARG_POSITIONAL_INF and ARG_POSITIONAL_MULTI.
• Adds wrapper regression fixtures and tests.
• Documents the fix in ChangeLog.

File summaries

File	Reviewed changes and findings
tests/regressiontests/test-wrapleftovers.m4	Adds the wrapper regression template.
tests/regressiontests/test-wrapleftovers-lib.m4	Adds the wrapped leftovers fixture.
tests/regressiontests/Makefile	Registers and runs the generated test.
tests/regressiontests/make/tests/tests-base.m4	Adds leftovers assertions. Findings: ARG_POSITIONAL_MULTI is not covered (moderate, 2 votes); existing assertions do not verify argument boundaries or zero leftovers (moderate, 1 vote).
src/collectors.m4	Preserves [@] during forwarding. Findings: scalar initialization can forward an extra empty argument when there are no leftovers (moderate, 1 vote); the ARG_POSITIONAL_MULTI path lacks regression coverage (moderate, 1 vote).
ChangeLog	Documents the bug fix.

Review details

Suppressed comments (3)

src/collectors.m4:466

• When ARG_LEFTOVERS has no values and the wrapper uses ARG_DEFAULTS_POS(), _MAKE_DEFAULTS_POSITIONALS_LOOP initializes _arg_leftovers with a scalar assignment (src/stuff.m4:1214), not (). For a scalar, quoted ${_arg_leftovers[@]} still expands to one empty word, so this append records and forwards an extra empty argument instead of zero leftovers. Initialize multivalued positionals as empty arrays (or guard the append), and add a zero-leftovers assertion that checks the argument count.

	[_POS_WRAPPED("${_varname([$1])@<:@@@:>@}")],

src/collectors.m4:505

• The second changed call site is not covered by the new regression test: test-wrapleftovers exercises ARG_LEFTOVERS, which expands through ARG_POSITIONAL_INF, but no wrapper fixture declares ARG_POSITIONAL_MULTI. A regression in this ARG_POSITIONAL_MULTI path could therefore pass the suite; add a wrapped multi-valued positional case and assert that its elements, including a whitespace-containing value, remain separate.

	[_POS_WRAPPED("${_varname([$1])@<:@@@:>@}")],

tests/regressiontests/make/tests/tests-base.m4:270

• The assertions only use whitespace-free leftovers and render arrays with ${...[*]}, so they do not verify argument boundaries: a wrapper that collapsed the values into one element (or retained an empty element) would produce the same text. Add a whitespace-containing value and assert per-element output or the forwarded argument count, including the zero-leftovers case.

	$< hello foo bar baz | grep -q 'CMD=hello,LEFTOVERS=foo bar baz,CMDLINE=foo bar baz,'

• Files reviewed: 6/6 changed files
• Comments generated: 1
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[gdevenyi]

Addressed: the wrapped library now declares ARG_POSITIONAL_MULTI as well as ARG_LEFTOVERS, and the assertions print each element in angle brackets together with the element count, using a value with whitespace and an empty value. Regarding the empty-leftovers remark in the summary: multi-valued positionals are always initialized as arrays (_arg_leftovers=()), so no empty word is forwarded - the new zero-leftovers assertion checks the count.