| Message ID | 20220616205456.19081-4-jacob.e.keller@intel.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | support negative refspecs in git remote show | expand |
Jacob Keller <jacob.keller@gmail.com> 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. > + 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" > }
On Thu, Jun 16, 2022 at 2:18 PM Junio C Hamano <gitster@pobox.com> wrote: > > Jacob Keller <jacob.keller@gmail.com> 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" > > }
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh index 2e160aa61233..0057b42988a4 100644 --- a/t/test-lib-functions.sh +++ b/t/test-lib-functions.sh @@ -506,15 +506,18 @@ test_modebits () { -e 's|^\(......\)S|\1-|' -e 's|^\(......\)s|\1x|' } -# Usage: test_unconfig [options] <name> +# Usage: test_unconfig [options] <name> <value-pattern> # -C <dir>: # Run all git commits in directory <dir> # --global: # Modify the global configuration instead of repository. +# --fixed-value: +# Match the value pattern as a fixed string instead of a regex. # # Unset a configuration variable, but don't fail if it doesn't exist. test_unconfig () { global= + fixedvalue= config_dir= while test $# != 0 do @@ -526,6 +529,9 @@ test_unconfig () { --global) global=yes ;; + --fixed-value) + fixedvalue=yes + ;; -*) BUG "invalid test_unconfig option: $1" ;; @@ -535,7 +541,12 @@ test_unconfig () { esac shift done - git ${config_dir:+-C "$config_dir"} config ${global:+--global} --unset-all "$1" + # Only enable --fixed-value if we have two parameters + if test $# < 2 + then + fixedvalue= + fi + 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'" && git ${config_dir:+-C "$config_dir"} config ${global:+--global} "$1" "$2" }
The test_config function is used to set a configuration value and then ensure that its unset at the end of the test using test_unconfig. This currently unsets all configurations with the specified name regardless of whether they match a value. The git config command can optionally only unset keys which have specific values. Add support for this by using the 2 argument form, and add handling for the --fixed-value option. Pass --fixed-value and both the key and value to the test_unconfig call from test_config. This ensures that test_config will only setup a trigger to remove the matching values instead of removing all configurations of the specified key. Directly using test_unconfig will still unset all values by default, unless a 2nd value-pattern parameter is provided. This matches the behavior of git config and allows some control over the behavior. Signed-off-by: Jacob Keller <jacob.e.keller@intel.com> --- t/test-lib-functions.sh | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-)