Re: [PATCH v2 3/5] tests: only automatically unset matching values from test_config
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jun 16, 2022, 21:18 UTC
- Message-ID
- <xmqq35g4s3xl.fsf@gitster.g>
- In-Reply-To
- <20220616205456.19081-4-jacob.e.keller@intel.com>
Jacob Keller <jacob.keller@gmail.com> writes:
Show 5 quoted lines
> + # Only enable --fixed-value if we have two parameters > + if test $# < 2 > + then > + fixedvalue= > + fi
Two comments:
* Does "<" do what you expect to do? Did you mean "-lt"?
* Using "bug in the test script: $*" and diagnosing missing parameters, instead of silently ignoring the option the developer wrote, would be more preferrable.
Show 10 quoted lines
> + git ${config_dir:+-C "$config_dir"} config ${global:+--global} ${fixedvalue:+--fixed-value} --unset-all "$1" "$2"
> config_status=$?
> case "$config_status" in
> 5) # ok, nothing to unset
> @@ -575,7 +586,7 @@ test_config () {
> esac
> shift
> done
> - test_when_finished "test_unconfig ${config_dir:+-C '$config_dir'} ${global:+--global} '$1'" &&
> + test_when_finished "test_unconfig ${config_dir:+-C '$config_dir'} --fixed-value ${global:+--global} '$1' '$2'" &&Why are $1 and $2 enclosed in a pair of single quotes? Is the assumption that they do not contain a single quote themselves?
I guess that is true also for config_dir and shares the same problem, so you are not introducing a new problem.
> git ${config_dir:+-C "$config_dir"} config ${global:+--global} "$1" "$2"
> }