Eric Sunshine <sunshine@xxxxxxxxxxxxxx> writes:
> Fortunately, alternation is unnecessary in this case and can easily be
> avoided, so replace it with a series of simple expressions such as
> `s/^var1/.../p;s/^var2/.../p`.
Simple is good.
> While at it, tighten the expressions so they match the variable names
> exactly rather than matching prefixes (i.e. use `s/^var1=/.../p`).
Good eyes. That is quite good.
> @@ -148,13 +148,18 @@ test_run_perf_ () {
> . '"$TEST_DIRECTORY"/test-lib-functions.sh'
> test_export () {
> [ $# != 0 ] || return 0
> - test_export_="$test_export_\\|$1"
> + test_export_="$test_export_ $1"
> shift
> test_export "$@"
> }
This "recursion to consume $@ one by one, instead of looping" caught
my eyes a bit, but the bug being fixed is not caused by it, so it is
fine to let it pass. Given that the arguments to test_export are
supposed to be all variable names (i.e. no funny characters anywhere
in it), I think it could just be
test_export () {
test_export_="$*"
}
though.
Oh, does anybody need to clear test_export_ to an empty string (or
unset it), by the way?
> '"$1"'
> ret=$?
> -set | sed -n "s'"/'/'\\\\''/g"';s/^\\($test_export_\\)/export '"'&'"'/p" >test_vars
> +needles=
> +for v in $test_export_
> +do
> + needles="$needles;s/^$v=/export $v=/p"
> +done
> +set | sed -n "s'"/'/'\\\\''/g"'$needles" >test_vars
> exit $ret' >&3 2>&4
> eval_ret=$?
Other than these, none of which were "this is wrong and needs to be
fixed", I have no comments. The patch is quite readably done.
Will queue. Thanks.
