@matheusbsbs / matheus-bagliano / commits / 45115d8490
Fork 0

grep: follow conventions for printing paths w/ unusual chars

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@hurrell.net> Helped-by: Johannes Schindelin <johannes.schindelin@gmx.de> Signed-off-by: Matheus Tavares <matheus.bernardino@usp.br> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Matheus Tavares committed Apr 19, 2020 at 03:33 UTC 45115d84901876b9300c1b6eba333cd8b25b1991
3 files changed +85 -14
Documentation/git-grep.txt
+4 -2
@@ -206,8 +206,10 @@ providing this option will cause it to die.
206
207 -z::
208 --null::
209 - Output \0 instead of the character that normally follows a
210 - file name.
209 + Use \0 as the delimiter for pathnames in the output, and print
210 + them verbatim. Without this option, pathnames with "unusual"
211 + characters are quoted as explained for the configuration
212 + variable core.quotePath (see git-config(1)).
213
214 -o::
215 --only-matching::
builtin/grep.c
+34 -12
@@ -295,6 +295,38 @@ static int grep_cmd_config(const char *var, const char *value, void *cb)
295 return st;
296 }
297
298 +static void grep_source_name(struct grep_opt *opt, const char *filename,
299 + int tree_name_len, struct strbuf *out)
300 +{
301 + strbuf_reset(out);
302 +
303 + if (opt->null_following_name) {
304 + if (opt->relative && opt->prefix_length) {
305 + struct strbuf rel_buf = STRBUF_INIT;
306 + const char *rel_name =
307 + relative_path(filename + tree_name_len,
308 + opt->prefix, &rel_buf);
309 +
310 + if (tree_name_len)
311 + strbuf_add(out, filename, tree_name_len);
312 +
313 + strbuf_addstr(out, rel_name);
314 + strbuf_release(&rel_buf);
315 + } else {
316 + strbuf_addstr(out, filename);
317 + }
318 + return;
319 + }
320 +
321 + if (opt->relative && opt->prefix_length)
322 + quote_path_relative(filename + tree_name_len, opt->prefix, out);
323 + else
324 + quote_c_style(filename + tree_name_len, out, NULL, 0);
325 +
326 + if (tree_name_len)
327 + strbuf_insert(out, 0, filename, tree_name_len);
328 +}
329 +
330 static int grep_oid(struct grep_opt *opt, const struct object_id *oid,
331 const char *filename, int tree_name_len,
332 const char *path)
@@ -302,13 +334,7 @@ static int grep_oid(struct grep_opt *opt, const struct object_id *oid,
334 struct strbuf pathbuf = STRBUF_INIT;
335 struct grep_source gs;
336
305 - if (opt->relative && opt->prefix_length) {
306 - quote_path_relative(filename + tree_name_len, opt->prefix, &pathbuf);
307 - strbuf_insert(&pathbuf, 0, filename, tree_name_len);
308 - } else {
309 - strbuf_addstr(&pathbuf, filename);
310 - }
311 -
337 + grep_source_name(opt, filename, tree_name_len, &pathbuf);
338 grep_source_init(&gs, GREP_SOURCE_OID, pathbuf.buf, path, oid);
339 strbuf_release(&pathbuf);
340
@@ -334,11 +360,7 @@ static int grep_file(struct grep_opt *opt, const char *filename)
360 struct strbuf buf = STRBUF_INIT;
361 struct grep_source gs;
362
337 - if (opt->relative && opt->prefix_length)
338 - quote_path_relative(filename, opt->prefix, &buf);
339 - else
340 - strbuf_addstr(&buf, filename);
341 -
363 + grep_source_name(opt, filename, 0, &buf);
364 grep_source_init(&gs, GREP_SOURCE_FILE, buf.buf, filename, filename);
365 strbuf_release(&buf);
366
t/t7810-grep.sh
+47
@@ -72,6 +72,11 @@ test_expect_success setup '
72 # Still a no-op.
73 function dummy() {}
74 EOF
75 + if test_have_prereq FUNNYNAMES
76 + then
77 + echo unusual >"\"unusual\" pathname" &&
78 + echo unusual >"t/nested \"unusual\" pathname"
79 + fi &&
80 git add . &&
81 test_tick &&
82 git commit -m initial
@@ -481,6 +486,48 @@ do
486 git grep --count -h -e b $H -- ab >actual &&
487 test_cmp expected actual
488 '
489 +
490 + test_expect_success FUNNYNAMES "grep $L should quote unusual pathnames" '
491 + cat >expected <<-EOF &&
492 + ${HC}"\"unusual\" pathname":unusual
493 + ${HC}"t/nested \"unusual\" pathname":unusual
494 + EOF
495 + git grep unusual $H >actual &&
496 + test_cmp expected actual
497 + '
498 +
499 + test_expect_success FUNNYNAMES "grep $L in subdir should quote unusual relative pathnames" '
500 + cat >expected <<-EOF &&
501 + ${HC}"nested \"unusual\" pathname":unusual
502 + EOF
503 + (
504 + cd t &&
505 + git grep unusual $H
506 + ) >actual &&
507 + test_cmp expected actual
508 + '
509 +
510 + test_expect_success FUNNYNAMES "grep -z $L with unusual pathnames" '
511 + cat >expected <<-EOF &&
512 + ${HC}"unusual" pathname:unusual
513 + ${HC}t/nested "unusual" pathname:unusual
514 + EOF
515 + git grep -z unusual $H >actual &&
516 + tr "\0" ":" <actual >actual-replace-null &&
517 + test_cmp expected actual-replace-null
518 + '
519 +
520 + test_expect_success FUNNYNAMES "grep -z $L in subdir with unusual relative pathnames" '
521 + cat >expected <<-EOF &&
522 + ${HC}nested "unusual" pathname:unusual
523 + EOF
524 + (
525 + cd t &&
526 + git grep -z unusual $H
527 + ) >actual &&
528 + tr "\0" ":" <actual >actual-replace-null &&
529 + test_cmp expected actual-replace-null
530 + '
531 done
532
533 cat >expected <<EOF