Re: [PATCH 4/7] config: add --literal-value option, un-implemented
From: Junio C Hamano <hidden>
Date: 2020-11-19 22:43:06
"Derrick Stolee via GitGitGadget" [off-list ref] writes:
+--literal-value:: + When used with the `value_regex` argument, treat `value_regex` as + an exact string instead of a regular expression. This will restrict + the name/value pairs that are matched to only those where the value + is exactly equal to the `value_regex`.
Makes sense.
quoted hunk
--type <type>:: 'git config' will ensure that any input or output is valid under the given type constraint(s), and will canonicalize outgoing values in `<type>`'sdiff --git a/builtin/config.c b/builtin/config.c index e7c7f3d455..ad6c695737 100644 --- a/builtin/config.c +++ b/builtin/config.c@@ -34,6 +34,7 @@ static int respect_includes_opt = -1; static struct config_options config_options; static int show_origin; static int show_scope; +static int literal; #define ACTION_GET (1<<0) #define ACTION_GET_ALL (1<<1)@@ -52,6 +53,16 @@ static int show_scope; #define ACTION_GET_COLORBOOL (1<<14) #define ACTION_GET_URLMATCH (1<<15) +#define ACTION_LITERAL_ALLOWED (\ + ACTION_GET |\ + ACTION_GET_ALL |\ + ACTION_GET_REGEXP |\ + ACTION_REPLACE_ALL |\ + ACTION_UNSET |\ + ACTION_UNSET_ALL |\ + ACTION_SET_ALL\ +)
I am not sure about this, though.
Even if an action can potentially take value_regex, e.g.
git config --get name [value_regex]
should we allow litral when value_regex is not given? IOW
$ git config --get --literal-value vari.able v2.26.0+next
may make sense, but shouldn't
$ git config --get --literal-value vari.able
be an error?
To put it differently, I think the macro being defined is not
ACTION_LITERAL_ALLOWED, but ACTION_VALUE_REGEX_ALLOWED. i.e. list of
actions that can potentially take value_regex. A command line that
gives value_regex to an action that is not listed there is an error.
+ if (literal && !(actions & ACTION_LITERAL_ALLOWED)) {
+ error(_("--literal only applies with 'value_regex'"));
+ usage_builtin_config();
+ }This check is not incorrect per-se, because an action that never takes value_regex is by definition incompatible with the literal value option. But it does not flag it as an error to ask a value_regex to be treated as a literal string when there is no value_regex given. Having said that, I think it is OK, at least for now, to leave such an error undetected---it falls into the "if it hurts, don't do it" category. Thanks.
quoted hunk
if (actions & PAGING_ACTIONS) setup_auto_pager("config", 1);diff --git a/t/t1300-config.sh b/t/t1300-config.sh index 74e0f84c0a..73f5ca4361 100755 --- a/t/t1300-config.sh +++ b/t/t1300-config.sh@@ -1965,4 +1965,17 @@ test_expect_success '--replace-all and value_regex' ' test_cmp expect .git/config ' +test_expect_success 'refuse --literal-value for incompatible actions' ' + git config dev.null bogus && + test_must_fail git config --literal-value --add dev.null bogus && + test_must_fail git config --literal-value --get-urlmatch dev.null bogus && + test_must_fail git config --literal-value --get-urlmatch dev.null bogus && + test_must_fail git config --literal-value --rename-section dev null && + test_must_fail git config --literal-value --remove-section dev && + test_must_fail git config --literal-value --list && + test_must_fail git config --literal-value --get-color dev.null && + test_must_fail git config --literal-value --get-colorbool dev.null && + test_must_fail git config --literal-value --edit +' + test_done