Fixed wrapping of scripts with positional args

matejak · matejak · commit 98ff692f8703 · 2016-07-10T21:40:56.000+02:00
diff --git a/ChangeLog b/ChangeLog
@@ -1,11 +1,15 @@
-1.3.0 (???)
+1.2.1 (???)
 ------------------
 
 New features:
 
 * TBD: Support for leftover args (kindof - not when upper bound of args is not
   known)
 
+Bugfixes:
+
+* Improved the wrapping of scripts to work with positional args
+
 1.2.0 (2016-07-08)
 ------------------
 
diff --git a/bin/argbash.sh b/bin/argbash.sh
@@ -86,6 +86,7 @@ for (( ii = 0; ii <  ${#POSITIONALS[@]}; ii++))
 do
 	eval "${POSITIONAL_NAMES[$ii]}=\"${POSITIONALS[$ii]}\"" || { echo "Error during argument parsing, possibly an Argbash bug." >&2; exit 1; }
 done
+
 # OTHER STUFF GENERATED BY Argbash
 SCRIPT_DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && pwd )"
 
diff --git a/resources/Makefile b/resources/Makefile
@@ -15,6 +15,7 @@ EXAMPLES = \
 	examples/minimal.sh \
 	examples/simple.sh \
 	examples/simple-standalone.sh \
+	examples/simple-wrapper.sh \
 	$(NUL)
 
 ROOT_DIR := $(dir $(realpath $(lastword $(MAKEFILE_LIST))))
diff --git a/resources/examples/minimal.sh b/resources/examples/minimal.sh
@@ -65,6 +65,7 @@ for (( ii = 0; ii <  ${#POSITIONALS[@]}; ii++))
 do
 	eval "${POSITIONAL_NAMES[$ii]}=\"${POSITIONALS[$ii]}\"" || { echo "Error during argument parsing, possibly an Argbash bug." >&2; exit 1; }
 done
+
 # OTHER STUFF GENERATED BY Argbash
 
 ### END OF CODE GENERATED BY Argbash (sortof) ### ])
diff --git a/resources/examples/simple-standalone.sh b/resources/examples/simple-standalone.sh
@@ -64,6 +64,7 @@ for (( ii = 0; ii <  ${#POSITIONALS[@]}; ii++))
 do
 	eval "${POSITIONAL_NAMES[$ii]}=\"${POSITIONALS[$ii]}\"" || { echo "Error during argument parsing, possibly an Argbash bug." >&2; exit 1; }
 done
+
 # OTHER STUFF GENERATED BY Argbash
 
 ### END OF CODE GENERATED BY Argbash (sortof) ### ])
diff --git a/resources/examples/simple-wrapper.m4 b/resources/examples/simple-wrapper.m4
@@ -0,0 +1,18 @@
+#!/bin/bash
+
+# ARG_POSITIONAL_SINGLE([directory])
+# ARGBASH_WRAP([simple], [filename])
+# ARG_HELP([This program tells you size of files in a given directory in units you choose.])
+# ARGBASH_GO
+
+# [ <-- needed because of Argbash
+
+test -f simple.sh || { echo "Missing the wrapped script, execute me from the directory where 'simple.sh' is."; exit 1; }
+test -d "$_ARG_DIRECTORY" || { echo "We expected a directory, got '$_ARG_DIRECTORY'."; exit 1; }
+
+for file in $_ARG_DIRECTORY/*
+do
+	test -f "$file" && echo "$file: $(./simple.sh "${_ARGS_SIMPLE_OPT[@]}" "$file")"
+done
+
+# ] <-- needed because of Argbash
diff --git a/resources/examples/simple-wrapper.sh b/resources/examples/simple-wrapper.sh
@@ -0,0 +1,77 @@
+#!/bin/bash
+
+# _ARG_POSITIONAL_SINGLE([directory])
+# ARGBASH_WRAP([simple],[filename])
+# ARG_HELP([This program tells you size of files in a given directory in units you choose.])
+# ARGBASH_GO()
+# needed because of Argbash --> m4_ignore([
+### START OF CODE GENERATED BY ARGBASH v1.2.0 one line above ###
+# Argbash is a bash code generator used to get arguments parsing right.
+# Argbash is FREE SOFTWARE, know your rights: https://github.com/matejak/argbash
+# THE DEFAULTS INITIALIZATION --- POSITIONALS
+# THE DEFAULTS INITIALIZATION --- OPTIONALS
+_ARG_UNIT="b"
+_ARG_VERBOSE=off
+
+# THE PRINT HELP FUNCION
+function print_help
+{
+	echo "This program tells you size of files in a given directory in units you choose."
+	echo "Usage: $0 [--unit <arg>] [--(no-)verbose] [--help] <directory>"
+	echo -e "\t<directory>: "
+	echo -e "\t-u,--unit: What unit we accept (b for bytes, k for kibibytes, M for mebibytes) (default: '"b"')"
+	echo -e "\t--verbose,--no-verbose:  (default: 'off')"
+	echo -e "\t-h,--help: Prints help"
+}
+
+# THE PARSING ITSELF
+while test $# -gt 0
+do
+	_key="$1"
+	case "$_key" in
+		-u|--unit)
+			test $# -lt 2 && { echo "Missing value for the optional argument '$_key'." >&2; exit 1; }
+			_ARG_UNIT="$2"
+			_ARGS_SIMPLE_OPT+=("$1" "$2")
+			shift
+			;;
+		--no-verbose|--verbose)
+			_ARG_VERBOSE="on"
+			_ARGS_SIMPLE_OPT+=("$1")
+			test "${1:0:5}" = "--no-" && _ARG_VERBOSE="off"
+			;;
+		-h|--help)
+			print_help
+			exit 0
+			;;
+		*)
+			POSITIONALS+=("$1")
+			;;
+	esac
+	shift
+done
+
+POSITIONAL_NAMES=('_ARG_DIRECTORY' )
+test ${#POSITIONALS[@]} -lt 1 && { ( echo "FATAL ERROR: Not enough positional arguments --- we require exactly 1, but got only ${#POSITIONALS[@]}."; print_help ) >&2; exit 1; }
+test ${#POSITIONALS[@]} -gt 1 && { ( echo "FATAL ERROR: There were spurious positional arguments --- we expect exactly 1, but got ${#POSITIONALS[@]} (the last one was: '${POSITIONALS[@]: -1}')."; print_help ) >&2; exit 1; }
+for (( ii = 0; ii <  ${#POSITIONALS[@]}; ii++))
+do
+	eval "${POSITIONAL_NAMES[$ii]}=\"${POSITIONALS[$ii]}\"" || { echo "Error during argument parsing, possibly an Argbash bug." >&2; exit 1; }
+done
+
+# OTHER STUFF GENERATED BY Argbash
+_ARGS_SIMPLE=("${_ARGS_SIMPLE_OPT[@]}" "${_ARGS_SIMPLE_POS[@]}")
+
+### END OF CODE GENERATED BY Argbash (sortof) ### ])
+# [ <-- needed because of Argbash
+
+
+test -f simple.sh || { echo "Missing the wrapped script, execute me from the directory where 'simple.sh' is."; exit 1; }
+test -d "$_ARG_DIRECTORY" || { echo "We expected a directory, got '$_ARG_DIRECTORY'."; exit 1; }
+
+for file in $_ARG_DIRECTORY/*
+do
+	test -f "$file" && echo "$file: $(./simple.sh "${_ARGS_SIMPLE_OPT[@]}" "$file")"
+done
+
+# ] <-- needed because of Argbash
diff --git a/resources/examples/simple.m4 b/resources/examples/simple.m4
@@ -1,13 +1,13 @@
 #!/bin/bash
 
 # ARG_POSITIONAL_SINGLE([filename])
-# ARG_OPTIONAL_SINGLE([unit], u, [What unit we accept (b for bytes, k for kilobytes, M for megabytes)], b)
+# ARG_OPTIONAL_SINGLE([unit], u, [What unit we accept (b for bytes, k for kibibytes, M for mebibytes)], b)
 # ARG_VERSION([echo $0 v0.1])
 # ARG_OPTIONAL_BOOLEAN(verbose)
 # ARG_HELP([This program tells you size of file that you pass to it in chosen units.])
 # ARGBASH_GO
 
-# opening escape square bracket: [
+# [ <-- needed because of Argbash
 
 # ARG_HELP  <-- Unlike one above, his one does not disappear, it is behind the escape bracket.
 
@@ -22,12 +22,12 @@ FILENAME="$_ARG_FILENAME"
 if [ $VERBOSE = on ]
 then
 	_b="bytes (B)"
-	_kb="kilobytes (kB)"
-	_mb="megabytes (MB)"
+	_kb="kibibytes (kiB)"
+	_mb="mebibytes (MiB)"
 else
 	_b="B"
-	_kb="kB"
-	_mb="MB"
+	_kb="kiB"
+	_mb="MiB"
 fi
 
 SIZE_BYTES=$(wc -c "$FILENAME" | cut -f 1 -d ' ')
@@ -43,4 +43,4 @@ test "$UNIT" = M && echo $SIZE_MEGABYTES $_mb && exit 0
 test "$VERBOSE" = on && echo "The unit '$UNIT' is not supported!"
 exit 1
 
-# closing escape square bracket: ]
+# ] <-- needed because of Argbash
diff --git a/resources/examples/simple.sh b/resources/examples/simple.sh
@@ -1,7 +1,7 @@
 #!/bin/bash
 
 # _ARG_POSITIONAL_SINGLE([filename])
-# ARG_OPTIONAL_SINGLE([unit],[u],[What unit we accept (b for bytes, k for kilobytes, M for megabytes)],[b])
+# ARG_OPTIONAL_SINGLE([unit],[u],[What unit we accept (b for bytes, k for kibibytes, M for mebibytes)],[b])
 # ARG_VERSION([echo $0 v0.1])
 # ARG_OPTIONAL_BOOLEAN([verbose])
 # ARG_HELP([This program tells you size of file that you pass to it in chosen units.])
@@ -21,7 +21,7 @@ function print_help
 	echo "This program tells you size of file that you pass to it in chosen units."
 	echo "Usage: $0 [--unit <arg>] [--version] [--(no-)verbose] [--help] <filename>"
 	echo -e "\t<filename>: "
-	echo -e "\t-u,--unit: What unit we accept (b for bytes, k for kilobytes, M for megabytes) (default: '"b"')"
+	echo -e "\t-u,--unit: What unit we accept (b for bytes, k for kibibytes, M for mebibytes) (default: '"b"')"
 	echo -e "\t-v,--version: Prints version"
 	echo -e "\t--verbose,--no-verbose:  (default: 'off')"
 	echo -e "\t-h,--help: Prints help"
@@ -65,12 +65,12 @@ for (( ii = 0; ii <  ${#POSITIONALS[@]}; ii++))
 do
 	eval "${POSITIONAL_NAMES[$ii]}=\"${POSITIONALS[$ii]}\"" || { echo "Error during argument parsing, possibly an Argbash bug." >&2; exit 1; }
 done
+
 # OTHER STUFF GENERATED BY Argbash
 
 ### END OF CODE GENERATED BY Argbash (sortof) ### ])
 # [ <-- needed because of Argbash
 
-# opening escape square bracket:
 
 # ARG_HELP  <-- Unlike one above, his one does not disappear, it is behind the escape bracket.
 
@@ -85,12 +85,12 @@ FILENAME="$_ARG_FILENAME"
 if [ $VERBOSE = on ]
 then
 	_b="bytes (B)"
-	_kb="kilobytes (kB)"
-	_mb="megabytes (MB)"
+	_kb="kibibytes (kiB)"
+	_mb="mebibytes (MiB)"
 else
 	_b="B"
-	_kb="kB"
-	_mb="MB"
+	_kb="kiB"
+	_mb="MiB"
 fi
 
 SIZE_BYTES=$(wc -c "$FILENAME" | cut -f 1 -d ' ')
@@ -106,5 +106,4 @@ test "$UNIT" = M && echo $SIZE_MEGABYTES $_mb && exit 0
 test "$VERBOSE" = on && echo "The unit '$UNIT' is not supported!"
 exit 1
 
-# closing escape square bracket:
 # ] <-- needed because of Argbash
diff --git a/src/list.m4 b/src/list.m4
@@ -31,6 +31,13 @@ m4_define([m4_list_len], [m4_do(
 	[m4_popdef([_LIST_NAME])],
 )])
 
+dnl
+dnl Pefrorm an action if a given list is empty
+dnl $1: The list's ID
+dnl $2: The action to do if the list is empty or not even defined
+dnl $3: The action to do if the list is defined and non-empty
+m4_define([m4_list_ifempty], [m4_if(m4_list_len([$1]), 0, [$2], [$3])])
+
 dnl
 dnl Given a list name, it expands to its contents, suitable to use e.g. in m4_foreach
 m4_define([m4_list_contents], [m4_do(
diff --git a/src/stuff.m4 b/src/stuff.m4
@@ -74,7 +74,10 @@ dnl $3: Help string
 dnl $4: Default, pass it through _sh_quote if needed beforehand (opt)
 dnl $5: Type
 m4_define([__some_opt], [m4_do(
-	[m4_ifdef([WRAPPED], [m4_define([_COLLECT_]_varname([$1]), [_ARGS_]_translit(WRAPPED))])],
+	[m4_ifdef([WRAPPED], [m4_do(
+		[m4_set_add([_ARGS_GROUPS], m4_expand([[_ARGS_]_translit(WRAPPED)]))],
+		[m4_define([_COLLECT_]_varname([$1]),  [_ARGS_]_translit(WRAPPED)_OPT)],
+	)])],
 	[m4_list_add([_ARGS_LONG], [$1])],
 	[dnl Check whether we didn't already use the arg, if not, add its tranliteration to the list of used ones
 ],
@@ -146,6 +149,11 @@ m4_define([_ARG_POSITIONAL_SINGLE], [m4_do(
 	[[$0($@)]],
 	[IF_POSITIONALS_INF([m4_fatal([We already expect arbitrary number of arguments before '$1'. This is not supported])], [])],
 	[IF_POSITIONALS_VARNUM([m4_fatal([The number of expected positional arguments before '$1' is unknown. This is not supported, define arguments that accept fixed number of values first.])], [])],
+	[m4_ifdef([WRAPPED], [m4_do(
+		[m4_set_add([_ARGS_GROUPS], m4_expand([[_ARGS_]_translit(WRAPPED)]))],
+		[m4_set_add([_POS_VARNAMES], m4_expand([[_ARGS_]_translit(WRAPPED)_POS]))],
+		[m4_list_add([_WRAPPED_ADD_SINGLE], m4_expand([[_ARGS_]_translit(WRAPPED)_POS+=@{:@"${_varname([$1])}"@:}@]))],
+	)])],
 	[dnl Number of possibly supplied positional arguments just went up
 ],
 	[m4_define([_POSITIONALS_MAX], m4_incr(_POSITIONALS_MAX))],
@@ -193,6 +201,11 @@ m4_define([_ARG_POSITIONAL_MULTI], [m4_do(
 	[[$0($@)]],
 	[IF_POSITIONALS_INF([m4_fatal([We already expect arbitrary number of arguments before '$1'. This is not supported])], [])],
 	[IF_POSITIONALS_VARNUM([m4_fatal([The number of expected positional arguments before '$1' is unknown. This is not supported, define arguments that accept fixed number of values first.])], [])],
+	[m4_ifdef([WRAPPED], [m4_do(
+		[m4_set_add([_ARGS_GROUPS], m4_expand([[_ARGS_]_translit(WRAPPED)]))],
+		[m4_set_add([_POS_VARNAMES], m4_expand([[_ARGS_]_translit(WRAPPED)_POS]))],
+		[m4_list_add([_WRAPPED_ADD_SINGLE], m4_expand([[_ARGS_]_translit(WRAPPED)_POS+=@{:@${_varname([$1])@<:@@@:>@}@:}@]))]
+	)])],
 	[m4_define([_POSITIONALS_MAX], m4_if([$3], -1, -1, m4_eval(_POSITIONALS_MAX + [$3])))],
 	[m4_list_add([_POSITIONALS_NAMES], [$1])],
 	[m4_list_add([_POSITIONALS_TYPES], [more])],
@@ -452,7 +465,7 @@ m4_define([_MAKE_HELP], [m4_do(
 m4_define([_ADD_OPTS_VALS], [m4_do(
 	[dnl If the arg comes from wrapped script/template, save it in an array
 ],
-	[m4_ifdef([_COLLECT_$1], [[_COLLECT_$1]+=("$[]1"m4_if($2, 2, ["$[]2"]))])],
+	[m4_ifdef([_COLLECT_$1], [_COLLECT_$1+=("$[]1"m4_if($2, 2, [ "$[]2"]))])],
 )])
 
 
@@ -603,6 +616,12 @@ do
 	eval "${POSITIONAL_NAMES[$ii]}=\"${POSITIONALS[$ii]}\"" || { echo "Error during argument parsing, possibly an Argbash bug." >&2; exit 1; }
 done]],
 		[m4_popdef([_NARGS_SPEC])],
+		[
+],
+		[m4_list_ifempty([_WRAPPED_ADD_SINGLE], [], [m4_set_foreach([_POS_VARNAMES], [varname], [varname=@{:@@:}@
+])])],
+		[m4_join([
+], m4_unquote(m4_list_contents([_WRAPPED_ADD_SINGLE])))],
 	)])],
 )])
 
@@ -660,6 +679,10 @@ m4_define([_MAKE_DEFAULTS], [m4_do(
 m4_define([_MAKE_OTHER], [m4_do(
 	[[# OTHER STUFF GENERATED BY Argbash
 ]],
+	dnl Put the stuff below into some condition block
+,	[m4_set_foreach([_ARGS_GROUPS], [agroup], [agroup=@{:@"${agroup[_OPT]@<:@@@:>@}" "${agroup[_POS]@<:@@@:>@}"@:}@
+])],
+	[],
 	[m4_list_declare([_OTHER])],
 	[_OTHER_FOREACH([item
 ])],
@@ -680,6 +703,7 @@ m4_define([ARGBASH_GO], [m4_do(
 	[m4_ifndef([WRAPPED], [_ARGBASH_GO([$0()])])],
 )])
 
+
 dnl $1: The macro call (the caller is supposed to pass [$0($@)])
 m4_define([ARGBASH_GO_BASE], [m4_do(
 	[[$1
@@ -725,11 +749,18 @@ m4_define([ARGBASH_WRAP], [m4_do(
 	[m4_popdef([WRAPPED])],
 )])
 
+
 dnl If I am wrapped:
 dnl defns are applied, but not recorded
 dnl some are not even recorded
 dnl It may be that the wrapped script contains args that we already have.
 dnl In this case: Raise an error (with a good advice)
 dnl
 
-dnl TODO: Add a repeated optional arg (s.a. -I in gcc)
+dnl Positional args wrapped:
+dnl - we keep a list POS_WRAPPED, where we store names of positional args that we want record.
+dnl - OR: When we encounter a wrapped positional arg, we store a code block <ARGS_ARRAY>+=pos_arg
+dnl   - and at the end, we just expand this block after pos_arg vars are filled.
+dnl - after positional args are assigned, we go through this list and append to the array of passed wrapped args 
+dnl
+dnl TODO: Implement leftover args
diff --git a/tests/regressiontests/Makefile b/tests/regressiontests/Makefile
@@ -9,6 +9,7 @@ REVERSE = $(TESTDIR)/reverse
 %.sh: %.m4 $(ARGBASH_BIN)
 	$(word 2,$^) $< -o $@
 
+# Let test-gen be last - those tests take the longest time to complete
 TESTS = \
 	test-most \
 	test-more \
@@ -19,12 +20,12 @@ TESTS = \
 	test-onlyopt \
 	test-onlypos \
 	test-ddash \
-	test-gen \
 	test-simple \
 	test-wrapping \
 	test-wrapping-more \
 	test-wrapping-excl \
 	stability-wrapping \
+	test-gen \
 	$(NUL)
 
 PHONIES += $(TESTS)
@@ -145,9 +146,10 @@ test-wrapping: $(TESTDIR)/test-wrapping.sh $(TESTDIR)/test-onlyopt.m4 $(TESTDIR)
 	$< -h | grep -q pos-arg
 	@# ! negates the return code
 	! $< -h | grep -q boo_l
-	$< LOOL | grep -q 'POS_S=LOOL,POS_OPT=pos-default'
-	$< LOOL | grep -q 'POS_S=LOOL,POS_OPT=pos-default'
-	$< LOOL --opt-arg lalala | grep -q OPT_S=lalala,
+	$< XX LOOL | grep -q 'POS_S0=XX,POS_S=LOOL,POS_OPT=pos-default'
+	$< XX LOOL | grep -q 'POS_S=LOOL,POS_OPT=pos-default'
+	$< XX LOOL --opt-arg lalala | grep -q OPT_S=lalala,
+	$< XX LOOL --opt-arg lalala | grep -q 'CMDLINE=--opt-arg lalala LOOL pos-default,'
 
 test-wrapping-more: $(TESTDIR)/test-wrapping-more.sh $(TESTDIR)/test-onlyopt.m4
 	$< -i -i -i | grep -q 'CMDLINE=-i -i -i,'
diff --git a/tests/regressiontests/test-wrapping.m4 b/tests/regressiontests/test-wrapping.m4
diff --git a/tests/unittests/list.m4 b/tests/unittests/list.m4
