On Mon, Nov 23, 2020 at 04:05:06PM +0000, Derrick Stolee via GitGitGadget wrote: > Allowing myself a sensible chuckle at the commit subject using glob syntax on a series about regex matching. ;) > > The config builtin does its own regex matching of values for the --get, > --get-all, and --get-regexp modes. Plumb the existing 'flags' parameter > to the get_value() method so we can initialize the value_regex argument > as a fixed string instead of a regex pattern. > > Signed-off-by: Derrick Stolee <dstolee@xxxxxxxxxxxxx> > --- > builtin/config.c | 15 ++++++++++----- > t/t1300-config.sh | 22 ++++++++++++++++++++++ > 2 files changed, 32 insertions(+), 5 deletions(-) > > diff --git a/builtin/config.c b/builtin/config.c > index 3e49e04411..d3772b5efe 100644 > --- a/builtin/config.c > +++ b/builtin/config.c > if (use_key_regexp && regexec(key_regexp, key_, 0, NULL, 0)) > return 0; > + if (fixed_value && strcmp(value_regex, (value_?value_:""))) > + return 0; Ooh, I can see you're matching style, but the combination of the spaceless ternary and the trailing underscore is making me so itchy ;) > if (regexp != NULL && > (do_not_match ^ !!regexec(regexp, (value_?value_:""), 0, NULL, 0))) > return 0; > @@ -298,7 +301,7 @@ static int collect_config(const char *key_, const char *value_, void *cb) > return format_config(&values->items[values->nr++], key_, value_); > } > > -static int get_value(const char *key_, const char *regex_) > +static int get_value(const char *key_, const char *regex_, int flags) Very reasonable, and I appreciate passing 'flags' instead of passing 'fixed' here. Room for growth :) [snip] a bunch of straightforward plumbing-through. > +test_expect_success '--get and --get-all with --fixed-value' ' > + GLOB="a+b*c?d[e]f.g" && > + rm -f config && Again this tells me that your other tests want 'test_when_finished' instead. > + git config --file=config fixed.test bogus && > + git config --file=config --add fixed.test "$GLOB" && > + > + git config --file=config --get fixed.test bogus && > + test_must_fail git config --file=config --get fixed.test "$GLOB" && > + git config --file=config --get --fixed-value fixed.test "$GLOB" && > + test_must_fail git config --file=config --get --fixed-value fixed.test non-existent && > + > + git config --file=config --get-all fixed.test bogus && > + test_must_fail git config --file=config --get-all fixed.test "$GLOB" && > + git config --file=config --get-all --fixed-value fixed.test "$GLOB" && > + test_must_fail git config --file=config --get-all --fixed-value fixed.test non-existent && > + > + git config --file=config --get-regexp fixed+ bogus && > + test_must_fail git config --file=config --get-regexp fixed+ "$GLOB" && > + git config --file=config --get-regexp --fixed-value fixed+ "$GLOB" && > + test_must_fail git config --file=config --get-regexp --fixed-value fixed+ non-existent > +' Otherwise the test seems fine to me, although I wouldn't yell if it grew some comments :) With the exception of the 'test_when_finished', which might not even match style (I didn't look): Reviewed-by: Emily Shaffer <emilyshaffer@xxxxxxxxxx>
