Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
gdevenyi wants to merge 1 commit into
base: master
Choose a base branch
from
Open

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

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions ChangeLog
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,10 @@
2.12.0 (unreleased)
-------------------

Bugfixes:

* Wrapper scripts forward the values of wrapped `ARG_LEFTOVERS` and `ARG_POSITIONAL_MULTI` arguments correctly - the `[@]` of the array expansion is no longer stripped (#205).

2.11.0 (2025-05-23)
-------------------

Expand Down
4 changes: 2 additions & 2 deletions src/collectors.m4
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -463,7 +463,7 @@ dnl $5, ...: Defaults
m4_define([_ARG_POSITIONAL_INF], _CHECK_INTEGER_TYPE(3, [minimal number of arguments])[m4_do(
[_CHECK_THAT_NUMBER_OF_PRECEDING_ARGUMENTS_IS_KNOWN([$1])],
[_CHECK_POSITIONAL_ARGNAME_IS_FREE([$1])],
[_POS_WRAPPED(${_varname([$1])[@]})],
[_POS_WRAPPED("${_varname([$1])@<:@@@:>@}")],

@matejak matejak Oct 12, 2026

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't like those trigraphs, it's definitely not wrong, but it is the nuclear option.
The kind of mistake looks obvious to me now, one level of quoting is stripped and therefore lost forever. So I would increase the nesting of quotes by one to [[@]] - _varname expands into m4_quote, and that's only one level of quotes.
It's great that you have added tests.

[m4_define([_POSITIONALS_INF], 1)],
[dnl We won't have to use stuff s.a. m4_quote(_INF_REPR), but _INF_REPR directly
],
Expand Down Expand Up @@ -502,7 +502,7 @@ argbash_api([ARG_POSITIONAL_MULTI], _CHECK_PASSED_ARGS_COUNT(3)_CHECK_INTEGER_TY
m4_define([_ARG_POSITIONAL_MULTI], [m4_do(
[_CHECK_THAT_NUMBER_OF_PRECEDING_ARGUMENTS_IS_KNOWN([$1])],
[_CHECK_POSITIONAL_ARGNAME_IS_FREE([$1])],
[_POS_WRAPPED(${_varname([$1])[@]})],
[_POS_WRAPPED("${_varname([$1])@<:@@@:>@}")],
[m4_define([_HIGHEST_POSITIONAL_VALUES_COUNT], m4_eval(_HIGHEST_POSITIONAL_VALUES_COUNT + [$3]))],
[_FILL_IN_VALUES_FOR_A_POSITIONAL_ARGUMENT([$1], [$2], _varname([$1]), [more])],
[dnl Minimal number of args is number of accepted - number of defaults (= $3 - ($# - 3))
Expand Down
7 changes: 7 additions & 0 deletions tests/regressiontests/Makefile
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ TESTS += \
test-wrapping-otherdir \
test-wrapping-more \
test-wrapping-excl \
test-wrapleftovers \
stability-wrapping \
test-infinity-minimal_call \
test-progs \
Expand Down Expand Up @@ -164,6 +165,7 @@ SCRIPTS += \
$(TESTDIR)/test-wrapping-otherdir.sh \
$(TESTDIR)/test-wrapping-more.sh \
$(TESTDIR)/test-wrapping-excl.sh \
$(TESTDIR)/test-wrapleftovers.sh \
$(TESTDIR)/test-infinity-minimal_call.sh \
$(TESTDIR)/test-progs.sh \
$(TESTDIR)/test-prog.sh \
Expand Down Expand Up @@ -496,6 +498,11 @@ test-wrapping-more: $(TESTDIR)/test-wrapping-more.sh $(TESTDIR)/test-onlyopt.m4
test-wrapping-excl: $(TESTDIR)/test-wrapping-excl.sh $(TESTDIR)/test-onlypos.m4
$(_test_onlypos)
test -z "$(SHELLCHECK)" || $(SHELLCHECK) "$(TESTDIR)/test-wrapping-excl.sh"
test-wrapleftovers: $(TESTDIR)/test-wrapleftovers.sh $(TESTDIR)/test-wrapleftovers-lib.m4
! grep -q '@}' $<
$< hello a b | grep -q 'CMD=hello,PAIR=<a><b>,LEFTOVERS=0,<>,CMDLINE=2,<a><b>,'
$< hello 'a b' c 'd e' '' | grep -q 'CMD=hello,PAIR=<a b><c>,LEFTOVERS=2,<d e><>,CMDLINE=4,<a b><c><d e><>,'
test -z "$(SHELLCHECK)" || $(SHELLCHECK) "$(TESTDIR)/test-wrapleftovers.sh"
stability-wrapping: $(TESTDIR)/test-wrapping.sh $(TESTDIR)/test-wrapping2.sh
diff -q $< $(word 2,$^)
test -z "$(SHELLCHECK)" || $(SHELLCHECK) "$(TESTDIR)/test-wrapping.sh"
Expand Down
8 changes: 8 additions & 0 deletions tests/regressiontests/make/tests/tests-base.m4
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -262,6 +262,14 @@ ADD_TEST_BASH([test-wrapping-excl], [[
$(_test_onlypos)
]], [$(TESTDIR)/test-onlypos.m4])

dnl Wrapping a script that declares ARG_LEFTOVERS and ARG_POSITIONAL_MULTI has to keep the [@] array expansion
dnl in the generated forwarding code of the wrapper (the [@] used to be stripped by m4 as a quote).
ADD_TEST_BASH([test-wrapleftovers], [[
! grep -q '@}' $<
$< hello a b | grep -q 'CMD=hello,PAIR=<a><b>,LEFTOVERS=0,<>,CMDLINE=2,<a><b>,'
$< hello 'a b' c 'd e' '' | grep -q 'CMD=hello,PAIR=<a b><c>,LEFTOVERS=2,<d e><>,CMDLINE=4,<a b><c><d e><>,'
]], [$(TESTDIR)/test-wrapleftovers-lib.m4])
Comment on lines +267 to +271

@gdevenyi gdevenyi Sep 13, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


ADD_SCRIPT([test-wrapping2])
ADD_TEST_BASH([stability-wrapping], [[
diff -q $< $(word 2,$^)
Expand Down
13 changes: 13 additions & 0 deletions tests/regressiontests/test-wrapleftovers-lib.m4
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
#!/usr/bin/env bash

# ARG_POSITIONAL_MULTI([pair], [Two values], 2)
# ARG_LEFTOVERS([args])
# ARGBASH_GO

# [ <-- needed because of Argbash

printf 'PAIR='; printf '<%s>' "${_arg_pair[@]}"
printf ',LEFTOVERS=%s,' "${#_arg_leftovers[@]}"; printf '<%s>' "${_arg_leftovers[@]}"
echo ','

# ] <-- needed because of Argbash
20 changes: 20 additions & 0 deletions tests/regressiontests/test-wrapleftovers.m4
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
#!/usr/bin/env bash

# SC2154: the wrapped library has no optional args, so _args_*_opt is referenced but not assigned.
# shellcheck disable=SC2154

m4_define(_DEFAULT_WRAP_FLAGS, [])

# ARG_POSITIONAL_SINGLE([cmd], [the command])
# ARGBASH_WRAP([test-wrapleftovers-lib])
# ARG_DEFAULTS_POS()
# ARGBASH_GO

# [ <-- needed because of Argbash

printf 'CMD=%s,PAIR=' "$_arg_cmd"; printf '<%s>' "${_arg_pair[@]}"
printf ',LEFTOVERS=%s,' "${#_arg_leftovers[@]}"; printf '<%s>' "${_arg_leftovers[@]}"
printf ',CMDLINE=%s,' "${#_args_test_wrapleftovers_lib[@]}"; printf '<%s>' "${_args_test_wrapleftovers_lib[@]}"
echo ','

# ] <-- needed because of Argbash
Loading