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>

diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt
index ddb6acc0257ef25c415de9e63e4fb9bacd1728ea..3109ce8fbeed31025e56554234be1b86c2f738a3 100644 (file)
--- 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 99e26850907b74374000675aa19bb42cf39c32b2..bdf1a4bbc99d77f814b2a2c7f130a2eedd7118d4 100644 (file)
--- 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 7d7b396c2370f0f7b9b0f2d48035d7260e6b464b..991d5bd9c03f525d536770908269209d716f39c1 100755 (executable)
--- a/t/t7810-grep.sh
+++ b/t/t7810-grep.sh
@@ -72,6 +72,11 @@ test_expect_success setup '
        # Still a no-op.
        function dummy() {}
        EOF
+       if test_have_prereq FUNNYNAMES
+       then
+               echo unusual >"\"unusual\" pathname" &&
+               echo unusual >"t/nested \"unusual\" pathname"
+       fi &&
        git add . &&
        test_tick &&
        git commit -m initial
@@ -481,6 +486,48 @@ do
                git grep --count -h -e b $H -- ab >actual &&
                test_cmp expected actual
        '
+
+       test_expect_success FUNNYNAMES "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 FUNNYNAMES "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 FUNNYNAMES "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 FUNNYNAMES "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