Hi Matheus, On Fri, 17 Apr 2020, Matheus Tavares wrote: > grep does not follow the conventions used by other Git commands when > printing paths that contain unusual characters (as double-quotes or > newlines). Commands such as ls-files, commit, status and diff will: > > - Quote and escape unusual pathnames, by default. > - Print names verbatim and unquoted when "-z" is used. > > But grep *never* quotes/escapes absolute paths with unusual chars and > *always* quotes/escapes relative ones, even with "-z". Besides being > inconsistent in its own output, the deviation from other Git commands > can be confusing. So let's make it follow the two rules above and add > some tests for this new behavior. Note that, making grep quote/escape > all unusual paths by default, also make it fully compliant with the > core.quotePath configuration, which is currently ignored for absolute > paths. > > Reported-by: Greg Hurrell <greg@xxxxxxxxxxx> > Signed-off-by: Matheus Tavares <matheus.bernardino@xxxxxx> > --- > Documentation/git-grep.txt | 6 +++-- > builtin/grep.c | 46 ++++++++++++++++++++++++++++---------- > t/t7810-grep.sh | 44 ++++++++++++++++++++++++++++++++++++ > 3 files changed, 82 insertions(+), 14 deletions(-) Unfortunately, this causes eight test failures on Windows: https://dev.azure.com/gitgitgadget/git/_build/results?buildId=38023&view=ms.vss-test-web.build-test-results-tab Could you please have a look? I suspect that this has something to do with those new tests needing the `FUNNYNAMES` prereq. Ciao, Dscho > > diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt > index ddb6acc025..3109ce8fbe 100644 > --- a/Documentation/git-grep.txt > +++ b/Documentation/git-grep.txt > @@ -206,8 +206,10 @@ providing this option will cause it to die. > > -z:: > --null:: > - Output \0 instead of the character that normally follows a > - file name. > + Use \0 as the delimiter for pathnames in the output, and print > + them verbatim. Without this option, pathnames with "unusual" > + characters are quoted as explained for the configuration > + variable core.quotePath (see git-config(1)). > > -o:: > --only-matching:: > diff --git a/builtin/grep.c b/builtin/grep.c > index 99e2685090..bdf1a4bbc9 100644 > --- a/builtin/grep.c > +++ b/builtin/grep.c > @@ -295,6 +295,38 @@ static int grep_cmd_config(const char *var, const char *value, void *cb) > return st; > } > > +static void grep_source_name(struct grep_opt *opt, const char *filename, > + int tree_name_len, struct strbuf *out) > +{ > + strbuf_reset(out); > + > + if (opt->null_following_name) { > + if (opt->relative && opt->prefix_length) { > + struct strbuf rel_buf = STRBUF_INIT; > + const char *rel_name = > + relative_path(filename + tree_name_len, > + opt->prefix, &rel_buf); > + > + if (tree_name_len) > + strbuf_add(out, filename, tree_name_len); > + > + strbuf_addstr(out, rel_name); > + strbuf_release(&rel_buf); > + } else { > + strbuf_addstr(out, filename); > + } > + return; > + } > + > + if (opt->relative && opt->prefix_length) > + quote_path_relative(filename + tree_name_len, opt->prefix, out); > + else > + quote_c_style(filename + tree_name_len, out, NULL, 0); > + > + if (tree_name_len) > + strbuf_insert(out, 0, filename, tree_name_len); > +} > + > static int grep_oid(struct grep_opt *opt, const struct object_id *oid, > const char *filename, int tree_name_len, > const char *path) > @@ -302,13 +334,7 @@ static int grep_oid(struct grep_opt *opt, const struct object_id *oid, > struct strbuf pathbuf = STRBUF_INIT; > struct grep_source gs; > > - if (opt->relative && opt->prefix_length) { > - quote_path_relative(filename + tree_name_len, opt->prefix, &pathbuf); > - strbuf_insert(&pathbuf, 0, filename, tree_name_len); > - } else { > - strbuf_addstr(&pathbuf, filename); > - } > - > + grep_source_name(opt, filename, tree_name_len, &pathbuf); > grep_source_init(&gs, GREP_SOURCE_OID, pathbuf.buf, path, oid); > strbuf_release(&pathbuf); > > @@ -334,11 +360,7 @@ static int grep_file(struct grep_opt *opt, const char *filename) > struct strbuf buf = STRBUF_INIT; > struct grep_source gs; > > - if (opt->relative && opt->prefix_length) > - quote_path_relative(filename, opt->prefix, &buf); > - else > - strbuf_addstr(&buf, filename); > - > + grep_source_name(opt, filename, 0, &buf); > grep_source_init(&gs, GREP_SOURCE_FILE, buf.buf, filename, filename); > strbuf_release(&buf); > > diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh > index 7d7b396c23..ab495dba28 100755 > --- a/t/t7810-grep.sh > +++ b/t/t7810-grep.sh > @@ -72,6 +72,8 @@ test_expect_success setup ' > # Still a no-op. > function dummy() {} > EOF > + echo unusual >"\"unusual\" pathname" && > + echo unusual >"t/nested \"unusual\" pathname" && > git add . && > test_tick && > git commit -m initial > @@ -481,6 +483,48 @@ do > git grep --count -h -e b $H -- ab >actual && > test_cmp expected actual > ' > + > + test_expect_success "grep $L should quote unusual pathnames" ' > + cat >expected <<-EOF && > + ${HC}"\"unusual\" pathname":unusual > + ${HC}"t/nested \"unusual\" pathname":unusual > + EOF > + git grep unusual $H >actual && > + test_cmp expected actual > + ' > + > + test_expect_success "grep $L in subdir should quote unusual relative pathnames" ' > + cat >expected <<-EOF && > + ${HC}"nested \"unusual\" pathname":unusual > + EOF > + ( > + cd t && > + git grep unusual $H > + ) >actual && > + test_cmp expected actual > + ' > + > + test_expect_success "grep -z $L with unusual pathnames" ' > + cat >expected <<-EOF && > + ${HC}"unusual" pathname:unusual > + ${HC}t/nested "unusual" pathname:unusual > + EOF > + git grep -z unusual $H >actual && > + tr "\0" ":" <actual >actual-replace-null && > + test_cmp expected actual-replace-null > + ' > + > + test_expect_success "grep -z $L in subdir with unusual relative pathnames" ' > + cat >expected <<-EOF && > + ${HC}nested "unusual" pathname:unusual > + EOF > + ( > + cd t && > + git grep -z unusual $H > + ) >actual && > + tr "\0" ":" <actual >actual-replace-null && > + test_cmp expected actual-replace-null > + ' > done > > cat >expected <<EOF > -- > 2.26.0 > > >
