On Thu, Jun 16, 2022 at 2:18 PM Junio C Hamano <gitster@xxxxxxxxx> wrote:
>
> Jacob Keller <jacob.keller@xxxxxxxxx> writes:
>
> > + # 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.
I guess we should check that it has exactly 1 or 2 arguments, yea.
>
> > + 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.
>
Yea, I wasn't sure.
> > git ${config_dir:+-C "$config_dir"} config ${global:+--global} "$1" "$2"
> > }
