{"thread":{"id":"53220","subject":"git-grep's \"-z\" option misbehaves in subdirectory","startedAt":"2020-04-13T21:56:06Z","lastAt":"2020-04-19T06:33:40Z","messageCount":13,"participants":["Greg Hurrell","Junio C Hamano","Matheus Tavares Bernardino","Matheus Tavares","Johannes Schindelin"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"395374","messageId":"CAOyLvt9=wRfpvGGJqLMi7=wLWu881pOur8c9qNEg+Xqhf8W2ww@mail.gmail.com","threadId":"53220","inReplyTo":null,"subject":"git-grep's \"-z\" option misbehaves in subdirectory","fromName":"Greg Hurrell","fromEmail":"greg@hurrell.net","sentAt":"2020-04-13T21:55:48Z","receivedAt":"2020-04-13T21:56:06Z","isPatch":false,"sender":{"key":"greg@hurrell.net","avatar":"https://avatars.githubusercontent.com/u/7074?v=4"},"body":"Hi,\n\nIt seems that `git-grep -lz` behaves differently depending on whether\nit is inside a subdirectory:\n\n$ mkdir demo\n$ cd demo\n$ git init\n$ echo content > 'an \"example\".txt'\n$ mkdir nested\n$ echo content > 'nested/other \"example\".txt'\n$ git add .\n$ git commit -m Initial\n$ git grep -lz content\nan \"example\".txt^@nested/other \"example\".txt^@\n\nNote that, as expected, the files are NUL-terminated and not wrapped\nin quotes. (\"^@\" represents NUL byte.)\n\n$ cd nested\n$ git grep -lz content\n\"other \\\"example\\\".txt\"^@\n\nAs soon as we move into a subdirectory, files are wrapped in quotes\nand contain escapes, despite the \"-z\" switch. git-ls-files doesn't\nexhibit this behavior:\n\n$ git ls-files -z\nother \"example\".txt\n\nAnd git-grep doesn't either, if you pass \"--full-name\":\n\n$ git grep -lz --full-name content\nnested/other \"example\".txt^@\n\nSeeing this on Git v2.25.0 on macOS (10.13.6).\n\nDoes this seem like a bug, or expected behavior?\n\nCheers,\nGreg\n"},{"id":"395381","messageId":"xmqq8sizarko.fsf@gitster.c.googlers.com","threadId":"53220","inReplyTo":"CAOyLvt9=wRfpvGGJqLMi7=wLWu881pOur8c9qNEg+Xqhf8W2ww@mail.gmail.com","subject":"Re: git-grep's \"-z\" option misbehaves in subdirectory","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-04-13T23:33:59Z","receivedAt":"2020-04-13T23:34:10Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Greg Hurrell writes:\n\n> $ git grep -lz --full-name content\n> nested/other \"example\".txt^@\n>\n> Seeing this on Git v2.25.0 on macOS (10.13.6).\n>\n> Does this seem like a bug, or expected behavior?\n\nSounds like a very ancient bug, possibly as old as the -z option\nitself.\n\n\n\n"},{"id":"395780","messageId":"20200414074204.677139-1-matheus.bernardino@usp.br","threadId":"53220","inReplyTo":"CAOyLvt9=wRfpvGGJqLMi7=wLWu881pOur8c9qNEg+Xqhf8W2ww@mail.gmail.com","subject":"","fromName":"Matheus Tavares","fromEmail":"matheus.bernardino@usp.br","sentAt":"2020-04-14T07:42:04Z","receivedAt":"2020-04-14T07:42:24Z","isPatch":false,"sender":{"key":"matheus.tavb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12701583?v=4"},"body":"On Mon, Apr 13, 2020 at 6:57 PM Greg Hurrell wrote:\n>\n> It seems that `git-grep -lz` behaves differently depending on whether\n> it is inside a subdirectory:\n[...]\n> $ git grep -lz content\n> an \"example\".txt^@nested/other \"example\".txt^@\n>\n> Note that, as expected, the files are NUL-terminated and not wrapped\n> in quotes. (\"^@\" represents NUL byte.)\n>\n> $ cd nested\n> $ git grep -lz content\n> \"other \\\"example\\\".txt\"^@\n>\n> As soon as we move into a subdirectory, files are wrapped in quotes\n> and contain escapes, despite the \"-z\" switch.\n\nI think this is a bug in \"plain\" git-grep, not in -z specifically. In some git\ncommands, -z has the effect of unquoting the output paths. For example, the\ndocs for git-ls-files says: \"-z: \\0 line termination on output and do not quote\nfilenames.\" In git-grep, however, the -z doc entry only says: \"Output \\0\ninstead of the character that normally follows a file name.\"\n\nSo -z does not seem to affect quoting in git-grep. But should we change this, to\nquote unusual pathnames (relative or not) by default, and let -z fall back to\nthe old behavior? This would be more consistent with other commands such as\ngit-ls-files and to the core.quotePath setting.\n\nHowever, perhaps output paths are never intended to be displayed quoted in\ngit-grep (with or without -z) in order to be consistent with GNU grep. I don't\nknow which of these options is correct (if any), so I would love to hear what\nothers have to say about it.\n\nBut if the second is correct, I think we can use the following patch to solve\nthe reported bug:\n\n(I'm just wondering: should we also add the information at core.quotePath that\ngit-grep does not comply with this setting, to be consistent with GNU grep {or\nfor any other reason}?)\n\n-- >8 --\nSubject: [RFC PATCH] grep: fix inconsistent output of unusual pathnames\n\nWhen grepping from the repository root, paths that contain unusual\ncharacters are not output quoted. However, they are quoted when grepping\nfrom a subdir without --full-name. For example:\n\n$ echo content >'an \"unusual\" name.txt'\n$ mkdir subdir\n$ echo content >'subdir/another \"unusual\" name.txt'\n$ git add .\n$ git commit -m content\n\nThen:\n$ git grep content\nan \"unusual\" name.txt:1:content\nsubdir/another \"unusual\" name.txt:1:content\n\nBut:\n$ cd subdir\n$ git grep content\n\"another \\\"unusual\\\" name.txt\":1:content\n\nFix this inconsistency by not quoting unusual pathnames (relative or\nnot), which is also the default behavior in GNU grep. Also add some\ntests to prevent regressions.\n\nNote: some non-related tests that compare git-grep output against\ngit-ls-files also had to be modified. This is because the testing repo\nnow contains some unusual pathnames. And git-ls-files will quote such\npaths by default, whereas git-grep won't.\n\nReported-by: Greg Hurrell \nSigned-off-by: Matheus Tavares \n---\n builtin/grep.c | 15 ++++++++++-----\n t/t7810-grep.sh | 33 ++++++++++++++++++++++++++++++---\n 2 files changed, 40 insertions(+), 8 deletions(-)\n\ndiff --git a/builtin/grep.c b/builtin/grep.c\nindex 99e2685090..ca21f98315 100644\n--- a/builtin/grep.c\n+++ b/builtin/grep.c\n@@ -303,8 +303,12 @@ static int grep_oid(struct grep_opt *opt, const struct object_id *oid,\n \tstruct grep_source gs;\n\n \tif (opt->relative && opt->prefix_length) {\n-\t\tquote_path_relative(filename + tree_name_len, opt->prefix, &pathbuf);\n-\t\tstrbuf_insert(&pathbuf, 0, filename, tree_name_len);\n+\t\tstruct strbuf rel_buf = STRBUF_INIT;\n+\t\tconst char *rel_name = relative_path(filename + tree_name_len,\n+\t\t\t\t\t\t opt->prefix, &rel_buf);\n+\t\tstrbuf_add(&pathbuf, filename, tree_name_len);\n+\t\tstrbuf_addstr(&pathbuf, rel_name);\n+\t\tstrbuf_release(&rel_buf);\n \t} else {\n \t\tstrbuf_addstr(&pathbuf, filename);\n \t}\n@@ -333,13 +337,14 @@ static int grep_file(struct grep_opt *opt, const char *filename)\n {\n \tstruct strbuf buf = STRBUF_INIT;\n \tstruct grep_source gs;\n+\tconst char *gs_name;\n\n \tif (opt->relative && opt->prefix_length)\n-\t\tquote_path_relative(filename, opt->prefix, &buf);\n+\t\tgs_name = relative_path(filename, opt->prefix, &buf);\n \telse\n-\t\tstrbuf_addstr(&buf, filename);\n+\t\tgs_name = filename;\n\n-\tgrep_source_init(&gs, GREP_SOURCE_FILE, buf.buf, filename, filename);\n+\tgrep_source_init(&gs, GREP_SOURCE_FILE, gs_name, filename, filename);\n \tstrbuf_release(&buf);\n\n \tif (num_threads > 1) {\ndiff --git a/t/t7810-grep.sh b/t/t7810-grep.sh\nindex 7d7b396c23..23911a3574 100755\n--- a/t/t7810-grep.sh\n+++ b/t/t7810-grep.sh\n@@ -72,6 +72,8 @@ test_expect_success setup '\n \t# Still a no-op.\n \tfunction dummy() {}\n \tEOF\n+\techo unusual >\"\\\"unusual\\\" pathname\" &&\n+\techo unusual >\"t/\\\"unusual\\\" pathname2\" &&\n \tgit add . &&\n \ttest_tick &&\n \tgit commit -m initial\n@@ -481,6 +483,26 @@ do\n \t\tgit grep --count -h -e b $H -- ab >actual &&\n \t\ttest_cmp expected actual\n \t'\n+\n+\ttest_expect_success \"grep $L should not quote unusual pathnames\" '\n+\t\tcat >expected <<-EOF &&\n+\t\t${HC}\"unusual\" pathname:unusual\n+\t\t${HC}t/\"unusual\" pathname2:unusual\n+\t\tEOF\n+\t\tgit grep unusual $H >actual &&\n+\t\ttest_cmp expected actual\n+\t'\n+\n+\ttest_expect_success \"grep $L should not quote unusual relative pathnames\" '\n+\t\tcat >expected <<-EOF &&\n+\t\t${HC}\"unusual\" pathname2:unusual\n+\t\tEOF\n+\t\t(\n+\t\t\tcd t &&\n+\t\t\tgit grep unusual $H\n+\t\t) >actual &&\n+\t\ttest_cmp expected actual\n+\t'\n done\n\n cat >expected <expected &&\n+\tgit ls-files -z >ls-files-z &&\n+\ttr \"\\0\" \"\\n\" expected &&\n \tgit grep -L -C1 nonexistent_string >actual &&\n \ttest_cmp expected actual\n '\n@@ -1686,7 +1709,9 @@ test_expect_success 'grep does not report i-t-a with -L --cached' '\n \techo \"intend to add this\" >intend-to-add &&\n \tgit add -N intend-to-add &&\n \ttest_when_finished \"git rm -f intend-to-add\" &&\n-\tgit ls-files | grep -v \"^intend-to-add\\$\" >expected &&\n+\tgit ls-files -z >ls-files-z &&\n+\ttr \"\\0\" \"\\n\" files &&\n+\tgrep -v \"^intend-to-add\\$\" files >expected &&\n \tgit grep -L --cached \"nonexistent_string\" >actual &&\n \ttest_cmp expected actual\n '\n@@ -1696,7 +1721,9 @@ test_expect_success 'grep does not report i-t-a and assume unchanged with -L' '\n \tgit add -N intend-to-add-assume-unchanged &&\n \ttest_when_finished \"git rm -f intend-to-add-assume-unchanged\" &&\n \tgit update-index --assume-unchanged intend-to-add-assume-unchanged &&\n-\tgit ls-files | grep -v \"^intend-to-add-assume-unchanged\\$\" >expected &&\n+\tgit ls-files -z >ls-files-z &&\n+\ttr \"\\0\" \"\\n\" files &&\n+\tgrep -v \"^intend-to-add-assume-unchanged\\$\" files >expected &&\n \tgit grep -L \"nonexistent_string\" >actual &&\n \ttest_cmp expected actual\n '\n--\n2.26.0\n\n"},{"id":"395613","messageId":"CAHd-oW4NK3E2umq9OXXW9TUyLKQwWN4R-b1KKK117tWPc=K7aw@mail.gmail.com","threadId":"53220","inReplyTo":"CAOyLvt9=wRfpvGGJqLMi7=wLWu881pOur8c9qNEg+Xqhf8W2ww@mail.gmail.com","subject":"Re: git-grep's \"-z\" option misbehaves in subdirectory","fromName":"Matheus Tavares Bernardino","fromEmail":"matheus.bernardino@usp.br","sentAt":"2020-04-16T18:59:12Z","receivedAt":"2020-04-16T18:59:30Z","isPatch":false,"sender":{"key":"matheus.tavb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12701583?v=4"},"body":"[Oops, I messed up the headers in my reply, so I'm not sure if the\nmessage was properly delivered. I'm replying again, just in case.]\n\nOn Mon, Apr 13, 2020 at 6:57 PM Greg Hurrell wrote:\n>\n> It seems that `git-grep -lz` behaves differently depending on whether\n> it is inside a subdirectory:\n[...]\n> $ git grep -lz content\n> an \"example\".txt^@nested/other \"example\".txt^@\n>\n> Note that, as expected, the files are NUL-terminated and not wrapped\n> in quotes. (\"^@\" represents NUL byte.)\n>\n> $ cd nested\n> $ git grep -lz content\n> \"other \\\"example\\\".txt\"^@\n>\n> As soon as we move into a subdirectory, files are wrapped in quotes\n> and contain escapes, despite the \"-z\" switch.\n\nNote that, differently from other Git commands, \"-z\" doesn't affect\nquoting/escaping in git-grep. For example, while git-ls-files' man\npage says:\n\n-z\n \\0 line termination on output **and do not quote filenames**.\n\nThe git-grep one only says:\n\n-z, --null\n Output \\0 instead of the character that normally follows a file name.\n\nIndeed, this inconsistency might be a little confusing. The reason for\nit may be because \"-z\" was added in git-grep [1] to mimic the GNU grep\n\"-Z\" flag, which outputs '\\0' after file names. In GNU grep, pathnames\nare never quoted (regardless of whether or not \"-Z\" is used). Also\nnote that, probably due to the same reason, git-grep doesn't obey the\ncore.quotePath setting (which would make it quote unusual pathnames by\ndefault).\n\nNevertheless, quoting relative paths but not absolute ones is a bug.\nFrom what I see, we have two options to fix it:\n\n1. Make git-grep more consistent with other Git commands by always\nquoting/escaping unusual pathnames (relative or not), unless \"-z\" is\ngiven or core.quotePath is set to \"false\".\n\n2. Keep git-grep compatible with GNU grep, ripgrep and other grepping\ntools, removing the quotes/escaping from relative paths as well. In\nthis case, I think it is also a good idea to add a note about this\nbehavior in core.quotePath and in git-grep's description of \"-z\" (as\nit may seem confusing to those who are used to the behavior of \"-z\" in\nother Git commands).\n\nIn my previous [and possibly corrupted] reply, I included a patch[2]\nin the direction of the second option. But thinking about it again,\nI'm not really sure which one is the best way forward here. I'd love\nto hear what others have to say about it, before sending a more\nrefined patch.\n\n[1]: See 83caecc, (git grep: Add \"-z/--null\" option as in GNU's grep.,\n2008-10-01)\n[2]: https://public-inbox.org/git/20200414074204.677139-1-matheus.bernardino@usp.br/\n(for some reason, the mail didn't reach lore.kernel.org. Probably my\nfault, messing with the headers :/)\n"},{"id":"395616","messageId":"xmqq1ronyz1s.fsf@gitster.c.googlers.com","threadId":"53220","inReplyTo":"CAHd-oW4NK3E2umq9OXXW9TUyLKQwWN4R-b1KKK117tWPc=K7aw@mail.gmail.com","subject":"Re: git-grep's \"-z\" option misbehaves in subdirectory","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-04-16T20:07:43Z","receivedAt":"2020-04-16T20:08:11Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matheus Tavares Bernardino writes:\n\n> Note that, differently from other Git commands, \"-z\" doesn't affect\n> quoting/escaping in git-grep. For example, while git-ls-files' man\n> page says:\n>\n> -z\n> \\0 line termination on output **and do not quote filenames**.\n>\n> The git-grep one only says:\n>\n> -z, --null\n> Output \\0 instead of the character that normally follows a file name.\n>\n> Indeed, this inconsistency might be a little confusing. The reason for\n> it may be ...\n\nLet me lift the need for speculation ;-) It was a simple oversight\nwhen the documentation was writte, and those \"add relative with\nquoting\" were bugs that not many people thought of testing with -z\noutput mode. It is unfortunate, but it happens that things fall\ninto cracks without anybody noticing X-<.\n\nThe reason why we quote paths in output from various commands\nwithout -z is to make them machine parseable; there is no reason to\nquote if \\0 delimiting is used as no paths contain NUL byte in it.\n\n> 1. Make git-grep more consistent with other Git commands by always\n> quoting/escaping unusual pathnames (relative or not), unless \"-z\" is\n> given or core.quotePath is set to \"false\".\n\nThis is the only sensible way forward, I would think.\n"},{"id":"395647","messageId":"eaae7214925189f562056b1ee6972c05dcf76a32.1587103366.git.matheus.bernardino@usp.br","threadId":"53220","inReplyTo":"xmqq1ronyz1s.fsf@gitster.c.googlers.com","subject":"[PATCH] grep: follow conventions for printing paths w/ unusual chars","fromName":"Matheus Tavares","fromEmail":"matheus.bernardino@usp.br","sentAt":"2020-04-17T06:04:03Z","receivedAt":"2020-04-17T06:04:28Z","isPatch":true,"sender":{"key":"matheus.tavb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12701583?v=4"},"body":"grep does not follow the conventions used by other Git commands when\nprinting paths that contain unusual characters (as double-quotes or\nnewlines). Commands such as ls-files, commit, status and diff will:\n\n- Quote and escape unusual pathnames, by default.\n- Print names verbatim and unquoted when \"-z\" is used.\n\nBut grep *never* quotes/escapes absolute paths with unusual chars and\n*always* quotes/escapes relative ones, even with \"-z\". Besides being\ninconsistent in its own output, the deviation from other Git commands\ncan be confusing. So let's make it follow the two rules above and add\nsome tests for this new behavior. Note that, making grep quote/escape\nall unusual paths by default, also make it fully compliant with the\ncore.quotePath configuration, which is currently ignored for absolute\npaths.\n\nReported-by: Greg Hurrell \nSigned-off-by: Matheus Tavares \n---\n Documentation/git-grep.txt | 6 +++--\n builtin/grep.c | 46 ++++++++++++++++++++++++++++----------\n t/t7810-grep.sh | 44 ++++++++++++++++++++++++++++++++++++\n 3 files changed, 82 insertions(+), 14 deletions(-)\n\ndiff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt\nindex ddb6acc025..3109ce8fbe 100644\n--- a/Documentation/git-grep.txt\n+++ b/Documentation/git-grep.txt\n@@ -206,8 +206,10 @@ providing this option will cause it to die.\n \n -z::\n --null::\n-\tOutput \\0 instead of the character that normally follows a\n-\tfile name.\n+\tUse \\0 as the delimiter for pathnames in the output, and print\n+\tthem verbatim. Without this option, pathnames with \"unusual\"\n+\tcharacters are quoted as explained for the configuration\n+\tvariable core.quotePath (see git-config(1)).\n \n -o::\n --only-matching::\ndiff --git a/builtin/grep.c b/builtin/grep.c\nindex 99e2685090..bdf1a4bbc9 100644\n--- a/builtin/grep.c\n+++ b/builtin/grep.c\n@@ -295,6 +295,38 @@ static int grep_cmd_config(const char *var, const char *value, void *cb)\n \treturn st;\n }\n \n+static void grep_source_name(struct grep_opt *opt, const char *filename,\n+\t\t\t int tree_name_len, struct strbuf *out)\n+{\n+\tstrbuf_reset(out);\n+\n+\tif (opt->null_following_name) {\n+\t\tif (opt->relative && opt->prefix_length) {\n+\t\t\tstruct strbuf rel_buf = STRBUF_INIT;\n+\t\t\tconst char *rel_name =\n+\t\t\t\trelative_path(filename + tree_name_len,\n+\t\t\t\t\t opt->prefix, &rel_buf);\n+\n+\t\t\tif (tree_name_len)\n+\t\t\t\tstrbuf_add(out, filename, tree_name_len);\n+\n+\t\t\tstrbuf_addstr(out, rel_name);\n+\t\t\tstrbuf_release(&rel_buf);\n+\t\t} else {\n+\t\t\tstrbuf_addstr(out, filename);\n+\t\t}\n+\t\treturn;\n+\t}\n+\n+\tif (opt->relative && opt->prefix_length)\n+\t\tquote_path_relative(filename + tree_name_len, opt->prefix, out);\n+\telse\n+\t\tquote_c_style(filename + tree_name_len, out, NULL, 0);\n+\n+\tif (tree_name_len)\n+\t\tstrbuf_insert(out, 0, filename, tree_name_len);\n+}\n+\n static int grep_oid(struct grep_opt *opt, const struct object_id *oid,\n \t\t const char *filename, int tree_name_len,\n \t\t const char *path)\n@@ -302,13 +334,7 @@ static int grep_oid(struct grep_opt *opt, const struct object_id *oid,\n \tstruct strbuf pathbuf = STRBUF_INIT;\n \tstruct grep_source gs;\n \n-\tif (opt->relative && opt->prefix_length) {\n-\t\tquote_path_relative(filename + tree_name_len, opt->prefix, &pathbuf);\n-\t\tstrbuf_insert(&pathbuf, 0, filename, tree_name_len);\n-\t} else {\n-\t\tstrbuf_addstr(&pathbuf, filename);\n-\t}\n-\n+\tgrep_source_name(opt, filename, tree_name_len, &pathbuf);\n \tgrep_source_init(&gs, GREP_SOURCE_OID, pathbuf.buf, path, oid);\n \tstrbuf_release(&pathbuf);\n \n@@ -334,11 +360,7 @@ static int grep_file(struct grep_opt *opt, const char *filename)\n \tstruct strbuf buf = STRBUF_INIT;\n \tstruct grep_source gs;\n \n-\tif (opt->relative && opt->prefix_length)\n-\t\tquote_path_relative(filename, opt->prefix, &buf);\n-\telse\n-\t\tstrbuf_addstr(&buf, filename);\n-\n+\tgrep_source_name(opt, filename, 0, &buf);\n \tgrep_source_init(&gs, GREP_SOURCE_FILE, buf.buf, filename, filename);\n \tstrbuf_release(&buf);\n \ndiff --git a/t/t7810-grep.sh b/t/t7810-grep.sh\nindex 7d7b396c23..ab495dba28 100755\n--- a/t/t7810-grep.sh\n+++ b/t/t7810-grep.sh\n@@ -72,6 +72,8 @@ test_expect_success setup '\n \t# Still a no-op.\n \tfunction dummy() {}\n \tEOF\n+\techo unusual >\"\\\"unusual\\\" pathname\" &&\n+\techo unusual >\"t/nested \\\"unusual\\\" pathname\" &&\n \tgit add . &&\n \ttest_tick &&\n \tgit commit -m initial\n@@ -481,6 +483,48 @@ do\n \t\tgit grep --count -h -e b $H -- ab >actual &&\n \t\ttest_cmp expected actual\n \t'\n+\n+\ttest_expect_success \"grep $L should quote unusual pathnames\" '\n+\t\tcat >expected <<-EOF &&\n+\t\t${HC}\"\\\"unusual\\\" pathname\":unusual\n+\t\t${HC}\"t/nested \\\"unusual\\\" pathname\":unusual\n+\t\tEOF\n+\t\tgit grep unusual $H >actual &&\n+\t\ttest_cmp expected actual\n+\t'\n+\n+\ttest_expect_success \"grep $L in subdir should quote unusual relative pathnames\" '\n+\t\tcat >expected <<-EOF &&\n+\t\t${HC}\"nested \\\"unusual\\\" pathname\":unusual\n+\t\tEOF\n+\t\t(\n+\t\t\tcd t &&\n+\t\t\tgit grep unusual $H\n+\t\t) >actual &&\n+\t\ttest_cmp expected actual\n+\t'\n+\n+\ttest_expect_success \"grep -z $L with unusual pathnames\" '\n+\t\tcat >expected <<-EOF &&\n+\t\t${HC}\"unusual\" pathname:unusual\n+\t\t${HC}t/nested \"unusual\" pathname:unusual\n+\t\tEOF\n+\t\tgit grep -z unusual $H >actual &&\n+\t\ttr \"\\0\" \":\" actual-replace-null &&\n+\t\ttest_cmp expected actual-replace-null\n+\t'\n+\n+\ttest_expect_success \"grep -z $L in subdir with unusual relative pathnames\" '\n+\t\tcat >expected <<-EOF &&\n+\t\t${HC}nested \"unusual\" pathname:unusual\n+\t\tEOF\n+\t\t(\n+\t\t\tcd t &&\n+\t\t\tgit grep -z unusual $H\n+\t\t) >actual &&\n+\t\ttr \"\\0\" \":\" actual-replace-null &&\n+\t\ttest_cmp expected actual-replace-null\n+\t'\n done\n \n cat >expected < writes:\n\n> +\tif (opt->relative && opt->prefix_length)\n> +\t\tquote_path_relative(filename + tree_name_len, opt->prefix, out);\n> +\telse\n> +\t\tquote_c_style(filename + tree_name_len, out, NULL, 0);\n\nYup. This solves the discrepancy reported correctly (i.e. both\nsides should do the quoting, the original only quoted when relative,\nand the new code corrects the other side).\n\n> +\tif (tree_name_len)\n> +\t\tstrbuf_insert(out, 0, filename, tree_name_len);\n\nI am not quite sure about this part, though. \n\nEarlier we inserted the latter part of filename (after offset\ntree_name_len) to strbuf \"out\" after quoting, and then we are\nprefixing the earlier part of the filename without quoting to that\nsame \"out\". Wouldn't a path ABCDEF (I do not literally mean that\nthese ascii alphabets need quoting---just imagine each of these\nstands for a different letter and some causes the path to be quoted)\nwith tree_name_len pointing somewhere in the middle be added as (an\nanalog of) ABC\"DEF\", i.e. literal prefix with remainder quoted? I\nwould (perhaps näively) expect that the whole thing would be placed\ninside a dq pair in such a case, even if the prefix part alone would\nnot require quoting.\n\nThanks.\n\n\n"},{"id":"395669","messageId":"CAHd-oW7B1wtHOE7KCtOR-kAXhuj8Du_MAYobWd9nSsSvm_sChQ@mail.gmail.com","threadId":"53220","inReplyTo":"xmqq8siuwqxp.fsf@gitster.c.googlers.com","subject":"Re: [PATCH] grep: follow conventions for printing paths w/ unusual chars","fromName":"Matheus Tavares Bernardino","fromEmail":"matheus.bernardino@usp.br","sentAt":"2020-04-17T21:19:07Z","receivedAt":"2020-04-17T21:19:23Z","isPatch":true,"sender":{"key":"matheus.tavb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12701583?v=4"},"body":"On Fri, Apr 17, 2020 at 3:45 AM Junio C Hamano wrote:\n>\n> Matheus Tavares writes:\n>\n> > + if (opt->relative && opt->prefix_length)\n> > + quote_path_relative(filename + tree_name_len, opt->prefix, out);\n> > + else\n> > + quote_c_style(filename + tree_name_len, out, NULL, 0);\n>\n> Yup. This solves the discrepancy reported correctly (i.e. both\n> sides should do the quoting, the original only quoted when relative,\n> and the new code corrects the other side).\n>\n> > + if (tree_name_len)\n> > + strbuf_insert(out, 0, filename, tree_name_len);\n>\n> I am not quite sure about this part, though.\n>\n> Earlier we inserted the latter part of filename (after offset\n> tree_name_len) to strbuf \"out\" after quoting, and then we are\n> prefixing the earlier part of the filename without quoting to that\n> same \"out\". Wouldn't a path ABCDEF (I do not literally mean that\n> these ascii alphabets need quoting---just imagine each of these\n> stands for a different letter and some causes the path to be quoted)\n> with tree_name_len pointing somewhere in the middle be added as (an\n> analog of) ABC\"DEF\", i.e. literal prefix with remainder quoted?\n\nRight. But the ABC prefix here is always a tree name, so the output is\none of the following:\n\n:\"unusual path\":line\n:\"unusual path\":line\n\"unusual path\":line\n\n(Always having the entire \"unusual path\" inside the double quotes, though)\n\n> I would (perhaps näively) expect that the whole thing would be placed\n> inside a dq pair in such a case, even if the prefix part alone would\n> not require quoting.\n\nHm, it might be just me, but I think that including the tree name and\npath in the same dq pair could be a bit confusing, as they can seem\nlike one unique thing.\n\nThere is also the case of ref names containing unusual chars, as dq's.\nSuch ref names are not currently escaped in the output of other\ncommands as checkout, branch or describe. But if we include the ref\nname in grep's output quote, it would be escaped, which would be a\nminor inconsistency (I don't know if that's relevant, though).\n"},{"id":"395671","messageId":"xmqqd085vlqg.fsf@gitster.c.googlers.com","threadId":"53220","inReplyTo":"CAHd-oW7B1wtHOE7KCtOR-kAXhuj8Du_MAYobWd9nSsSvm_sChQ@mail.gmail.com","subject":"Re: [PATCH] grep: follow conventions for printing paths w/ unusual chars","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-04-17T21:35:51Z","receivedAt":"2020-04-17T21:35:59Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Matheus Tavares Bernardino writes:\n\n> Right. But the ABC prefix here is always a tree name,...\n\nOh, then that is perfectly fine. I did not expect that the code\npasses : (like \"HEAD~2:t/Makefile\") to us as a\nsingle string.\n\nThanks.\n"},{"id":"395681","messageId":"nycvar.QRO.7.76.6.2004181509350.46@tvgsbejvaqbjf.bet","threadId":"53220","inReplyTo":"eaae7214925189f562056b1ee6972c05dcf76a32.1587103366.git.matheus.bernardino@usp.br","subject":"Re: [PATCH] grep: follow conventions for printing paths w/ unusual chars","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2020-04-18T13:13:11Z","receivedAt":"2020-04-18T13:13:24Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Matheus,\n\nOn Fri, 17 Apr 2020, Matheus Tavares wrote:\n\n> grep does not follow the conventions used by other Git commands when\n> printing paths that contain unusual characters (as double-quotes or\n> newlines). Commands such as ls-files, commit, status and diff will:\n>\n> - Quote and escape unusual pathnames, by default.\n> - Print names verbatim and unquoted when \"-z\" is used.\n>\n> But grep *never* quotes/escapes absolute paths with unusual chars and\n> *always* quotes/escapes relative ones, even with \"-z\". Besides being\n> inconsistent in its own output, the deviation from other Git commands\n> can be confusing. So let's make it follow the two rules above and add\n> some tests for this new behavior. Note that, making grep quote/escape\n> all unusual paths by default, also make it fully compliant with the\n> core.quotePath configuration, which is currently ignored for absolute\n> paths.\n>\n> Reported-by: Greg Hurrell \n> Signed-off-by: Matheus Tavares \n> ---\n> Documentation/git-grep.txt | 6 +++--\n> builtin/grep.c | 46 ++++++++++++++++++++++++++++----------\n> t/t7810-grep.sh | 44 ++++++++++++++++++++++++++++++++++++\n> 3 files changed, 82 insertions(+), 14 deletions(-)\n\nUnfortunately, this causes eight test failures on Windows:\nhttps://dev.azure.com/gitgitgadget/git/_build/results?buildId=38023&view=ms.vss-test-web.build-test-results-tab\n\nCould you please have a look? I suspect that this has something to do with\nthose new tests needing the `FUNNYNAMES` prereq.\n\nCiao,\nDscho\n\n>\n> diff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt\n> index ddb6acc025..3109ce8fbe 100644\n> --- a/Documentation/git-grep.txt\n> +++ b/Documentation/git-grep.txt\n> @@ -206,8 +206,10 @@ providing this option will cause it to die.\n>\n> -z::\n> --null::\n> -\tOutput \\0 instead of the character that normally follows a\n> -\tfile name.\n> +\tUse \\0 as the delimiter for pathnames in the output, and print\n> +\tthem verbatim. Without this option, pathnames with \"unusual\"\n> +\tcharacters are quoted as explained for the configuration\n> +\tvariable core.quotePath (see git-config(1)).\n>\n> -o::\n> --only-matching::\n> diff --git a/builtin/grep.c b/builtin/grep.c\n> index 99e2685090..bdf1a4bbc9 100644\n> --- a/builtin/grep.c\n> +++ b/builtin/grep.c\n> @@ -295,6 +295,38 @@ static int grep_cmd_config(const char *var, const char *value, void *cb)\n> \treturn st;\n> }\n>\n> +static void grep_source_name(struct grep_opt *opt, const char *filename,\n> +\t\t\t int tree_name_len, struct strbuf *out)\n> +{\n> +\tstrbuf_reset(out);\n> +\n> +\tif (opt->null_following_name) {\n> +\t\tif (opt->relative && opt->prefix_length) {\n> +\t\t\tstruct strbuf rel_buf = STRBUF_INIT;\n> +\t\t\tconst char *rel_name =\n> +\t\t\t\trelative_path(filename + tree_name_len,\n> +\t\t\t\t\t opt->prefix, &rel_buf);\n> +\n> +\t\t\tif (tree_name_len)\n> +\t\t\t\tstrbuf_add(out, filename, tree_name_len);\n> +\n> +\t\t\tstrbuf_addstr(out, rel_name);\n> +\t\t\tstrbuf_release(&rel_buf);\n> +\t\t} else {\n> +\t\t\tstrbuf_addstr(out, filename);\n> +\t\t}\n> +\t\treturn;\n> +\t}\n> +\n> +\tif (opt->relative && opt->prefix_length)\n> +\t\tquote_path_relative(filename + tree_name_len, opt->prefix, out);\n> +\telse\n> +\t\tquote_c_style(filename + tree_name_len, out, NULL, 0);\n> +\n> +\tif (tree_name_len)\n> +\t\tstrbuf_insert(out, 0, filename, tree_name_len);\n> +}\n> +\n> static int grep_oid(struct grep_opt *opt, const struct object_id *oid,\n> \t\t const char *filename, int tree_name_len,\n> \t\t const char *path)\n> @@ -302,13 +334,7 @@ static int grep_oid(struct grep_opt *opt, const struct object_id *oid,\n> \tstruct strbuf pathbuf = STRBUF_INIT;\n> \tstruct grep_source gs;\n>\n> -\tif (opt->relative && opt->prefix_length) {\n> -\t\tquote_path_relative(filename + tree_name_len, opt->prefix, &pathbuf);\n> -\t\tstrbuf_insert(&pathbuf, 0, filename, tree_name_len);\n> -\t} else {\n> -\t\tstrbuf_addstr(&pathbuf, filename);\n> -\t}\n> -\n> +\tgrep_source_name(opt, filename, tree_name_len, &pathbuf);\n> \tgrep_source_init(&gs, GREP_SOURCE_OID, pathbuf.buf, path, oid);\n> \tstrbuf_release(&pathbuf);\n>\n> @@ -334,11 +360,7 @@ static int grep_file(struct grep_opt *opt, const char *filename)\n> \tstruct strbuf buf = STRBUF_INIT;\n> \tstruct grep_source gs;\n>\n> -\tif (opt->relative && opt->prefix_length)\n> -\t\tquote_path_relative(filename, opt->prefix, &buf);\n> -\telse\n> -\t\tstrbuf_addstr(&buf, filename);\n> -\n> +\tgrep_source_name(opt, filename, 0, &buf);\n> \tgrep_source_init(&gs, GREP_SOURCE_FILE, buf.buf, filename, filename);\n> \tstrbuf_release(&buf);\n>\n> diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh\n> index 7d7b396c23..ab495dba28 100755\n> --- a/t/t7810-grep.sh\n> +++ b/t/t7810-grep.sh\n> @@ -72,6 +72,8 @@ test_expect_success setup '\n> \t# Still a no-op.\n> \tfunction dummy() {}\n> \tEOF\n> +\techo unusual >\"\\\"unusual\\\" pathname\" &&\n> +\techo unusual >\"t/nested \\\"unusual\\\" pathname\" &&\n> \tgit add . &&\n> \ttest_tick &&\n> \tgit commit -m initial\n> @@ -481,6 +483,48 @@ do\n> \t\tgit grep --count -h -e b $H -- ab >actual &&\n> \t\ttest_cmp expected actual\n> \t'\n> +\n> +\ttest_expect_success \"grep $L should quote unusual pathnames\" '\n> +\t\tcat >expected <<-EOF &&\n> +\t\t${HC}\"\\\"unusual\\\" pathname\":unusual\n> +\t\t${HC}\"t/nested \\\"unusual\\\" pathname\":unusual\n> +\t\tEOF\n> +\t\tgit grep unusual $H >actual &&\n> +\t\ttest_cmp expected actual\n> +\t'\n> +\n> +\ttest_expect_success \"grep $L in subdir should quote unusual relative pathnames\" '\n> +\t\tcat >expected <<-EOF &&\n> +\t\t${HC}\"nested \\\"unusual\\\" pathname\":unusual\n> +\t\tEOF\n> +\t\t(\n> +\t\t\tcd t &&\n> +\t\t\tgit grep unusual $H\n> +\t\t) >actual &&\n> +\t\ttest_cmp expected actual\n> +\t'\n> +\n> +\ttest_expect_success \"grep -z $L with unusual pathnames\" '\n> +\t\tcat >expected <<-EOF &&\n> +\t\t${HC}\"unusual\" pathname:unusual\n> +\t\t${HC}t/nested \"unusual\" pathname:unusual\n> +\t\tEOF\n> +\t\tgit grep -z unusual $H >actual &&\n> +\t\ttr \"\\0\" \":\" actual-replace-null &&\n> +\t\ttest_cmp expected actual-replace-null\n> +\t'\n> +\n> +\ttest_expect_success \"grep -z $L in subdir with unusual relative pathnames\" '\n> +\t\tcat >expected <<-EOF &&\n> +\t\t${HC}nested \"unusual\" pathname:unusual\n> +\t\tEOF\n> +\t\t(\n> +\t\t\tcd t &&\n> +\t\t\tgit grep -z unusual $H\n> +\t\t) >actual &&\n> +\t\ttr \"\\0\" \":\" actual-replace-null &&\n> +\t\ttest_cmp expected actual-replace-null\n> +\t'\n> done\n>\n> cat >expected < --\n> 2.26.0\n>\n>\n>\n"},{"id":"395682","messageId":"nycvar.QRO.7.76.6.2004181655300.46@tvgsbejvaqbjf.bet","threadId":"53220","inReplyTo":"nycvar.QRO.7.76.6.2004181509350.46@tvgsbejvaqbjf.bet","subject":"Re: [PATCH] grep: follow conventions for printing paths w/ unusual chars","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2020-04-18T14:56:57Z","receivedAt":"2020-04-18T14:57:09Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Matheus,\n\nOn Sat, 18 Apr 2020, Johannes Schindelin wrote:\n\n> On Fri, 17 Apr 2020, Matheus Tavares wrote:\n>\n> > grep does not follow the conventions used by other Git commands when\n> > printing paths that contain unusual characters (as double-quotes or\n> > newlines). Commands such as ls-files, commit, status and diff will:\n> >\n> > - Quote and escape unusual pathnames, by default.\n> > - Print names verbatim and unquoted when \"-z\" is used.\n> >\n> > But grep *never* quotes/escapes absolute paths with unusual chars and\n> > *always* quotes/escapes relative ones, even with \"-z\". Besides being\n> > inconsistent in its own output, the deviation from other Git commands\n> > can be confusing. So let's make it follow the two rules above and add\n> > some tests for this new behavior. Note that, making grep quote/escape\n> > all unusual paths by default, also make it fully compliant with the\n> > core.quotePath configuration, which is currently ignored for absolute\n> > paths.\n> >\n> > Reported-by: Greg Hurrell \n> > Signed-off-by: Matheus Tavares \n> > ---\n> > Documentation/git-grep.txt | 6 +++--\n> > builtin/grep.c | 46 ++++++++++++++++++++++++++++----------\n> > t/t7810-grep.sh | 44 ++++++++++++++++++++++++++++++++++++\n> > 3 files changed, 82 insertions(+), 14 deletions(-)\n>\n> Unfortunately, this causes eight test failures on Windows:\n> https://dev.azure.com/gitgitgadget/git/_build/results?buildId=38023&view=ms.vss-test-web.build-test-results-tab\n>\n> Could you please have a look? I suspect that this has something to do with\n> those new tests needing the `FUNNYNAMES` prereq.\n\nI need this commit to fix it:\nhttps://github.com/git-for-windows/git/commit/7ca815e1ab89d6ffdb1a17b3cbacdf22a508d33c\n\nI'll paste it here, for your convenience:\n\n-- snipsnap --\nFrom 7ca815e1ab89d6ffdb1a17b3cbacdf22a508d33c Mon Sep 17 00:00:00 2001\nFrom: Johannes Schindelin \nDate: Sat, 18 Apr 2020 16:49:43 +0200\nSubject: [PATCH] fixup??? grep: follow conventions for printing paths w/\n unusual chars\n\nIt is easy to be fooled by the Bash included in Git for Windows, which\nleads you to believe that quotes are valid parts of file names.\n\nOn Windows, they are not. But Cygwin (which is the base of MSYS2, which\nis the POSIX emulation layer used by that Bash) only _pretends_ that it\nis a valid file name character. In reality, it will map the character\ninto the private Unicode page. Cygwin knows about this. The rest of\nWindows applications (including Git for Windows), however, does not.\n\nAs a consequence, `>\\\"with\\ quotes\\\"` will claim to succeed, but the\nfile on disk will have Unicode characters in place of those quotes that\nliterally no application but Cygwin ones can handle, and this leads to\nthose beautiful new tests to fail.\n\nLet's just use the prereq we introduced to guard precisely against this\nproblem: `FUNNYNAMES`.\n\nSigned-off-by: Johannes Schindelin \n---\n t/t7810-grep.sh | 15 +++++++++------\n 1 file changed, 9 insertions(+), 6 deletions(-)\n\ndiff --git a/t/t7810-grep.sh b/t/t7810-grep.sh\nindex ab495dba28a7..991d5bd9c03f 100755\n--- a/t/t7810-grep.sh\n+++ b/t/t7810-grep.sh\n@@ -72,8 +72,11 @@ test_expect_success setup '\n \t# Still a no-op.\n \tfunction dummy() {}\n \tEOF\n-\techo unusual >\"\\\"unusual\\\" pathname\" &&\n-\techo unusual >\"t/nested \\\"unusual\\\" pathname\" &&\n+\tif test_have_prereq FUNNYNAMES\n+\tthen\n+\t\techo unusual >\"\\\"unusual\\\" pathname\" &&\n+\t\techo unusual >\"t/nested \\\"unusual\\\" pathname\"\n+\tfi &&\n \tgit add . &&\n \ttest_tick &&\n \tgit commit -m initial\n@@ -484,7 +487,7 @@ do\n \t\ttest_cmp expected actual\n \t'\n\n-\ttest_expect_success \"grep $L should quote unusual pathnames\" '\n+\ttest_expect_success FUNNYNAMES \"grep $L should quote unusual pathnames\" '\n \t\tcat >expected <<-EOF &&\n \t\t${HC}\"\\\"unusual\\\" pathname\":unusual\n \t\t${HC}\"t/nested \\\"unusual\\\" pathname\":unusual\n@@ -493,7 +496,7 @@ do\n \t\ttest_cmp expected actual\n \t'\n\n-\ttest_expect_success \"grep $L in subdir should quote unusual relative pathnames\" '\n+\ttest_expect_success FUNNYNAMES \"grep $L in subdir should quote unusual relative pathnames\" '\n \t\tcat >expected <<-EOF &&\n \t\t${HC}\"nested \\\"unusual\\\" pathname\":unusual\n \t\tEOF\n@@ -504,7 +507,7 @@ do\n \t\ttest_cmp expected actual\n \t'\n\n-\ttest_expect_success \"grep -z $L with unusual pathnames\" '\n+\ttest_expect_success FUNNYNAMES \"grep -z $L with unusual pathnames\" '\n \t\tcat >expected <<-EOF &&\n \t\t${HC}\"unusual\" pathname:unusual\n \t\t${HC}t/nested \"unusual\" pathname:unusual\n@@ -514,7 +517,7 @@ do\n \t\ttest_cmp expected actual-replace-null\n \t'\n\n-\ttest_expect_success \"grep -z $L in subdir with unusual relative pathnames\" '\n+\ttest_expect_success FUNNYNAMES \"grep -z $L in subdir with unusual relative pathnames\" '\n \t\tcat >expected <<-EOF &&\n \t\t${HC}nested \"unusual\" pathname:unusual\n \t\tEOF\n--\n2.26.1.windows.1\n\n"},{"id":"395710","messageId":"CAHd-oW4dkLMp3FYyRoRWVgcTMJ0oKnKfnK--3r_Mx9EfrLoEnQ@mail.gmail.com","threadId":"53220","inReplyTo":"nycvar.QRO.7.76.6.2004181655300.46@tvgsbejvaqbjf.bet","subject":"Re: [PATCH] grep: follow conventions for printing paths w/ unusual chars","fromName":"Matheus Tavares Bernardino","fromEmail":"matheus.bernardino@usp.br","sentAt":"2020-04-19T06:27:21Z","receivedAt":"2020-04-19T06:27:44Z","isPatch":true,"sender":{"key":"matheus.tavb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12701583?v=4"},"body":"Hi, Dscho\n\nOn Sat, Apr 18, 2020 at 11:57 AM Johannes Schindelin\n wrote:\n>\n> On Sat, 18 Apr 2020, Johannes Schindelin wrote:\n> >\n> > On Fri, 17 Apr 2020, Matheus Tavares wrote:\n...\n> > >\n> > > Reported-by: Greg Hurrell \n> > > Signed-off-by: Matheus Tavares \n> > > ---\n> > > Documentation/git-grep.txt | 6 +++--\n> > > builtin/grep.c | 46 ++++++++++++++++++++++++++++----------\n> > > t/t7810-grep.sh | 44 ++++++++++++++++++++++++++++++++++++\n> > > 3 files changed, 82 insertions(+), 14 deletions(-)\n> >\n> > Unfortunately, this causes eight test failures on Windows:\n> > https://dev.azure.com/gitgitgadget/git/_build/results?buildId=38023&view=ms.vss-test-web.build-test-results-tab\n> >\n> > Could you please have a look? I suspect that this has something to do with\n> > those new tests needing the `FUNNYNAMES` prereq.\n>\n> I need this commit to fix it:\n> https://github.com/git-for-windows/git/commit/7ca815e1ab89d6ffdb1a17b3cbacdf22a508d33c\n>\n> I'll paste it here, for your convenience:\n>\n> -- snipsnap --\n\nThanks for reporting and correcting the testing issue. I'll send a v2\nwith your patch squashed in.\n"},{"id":"395711","messageId":"77218d98948a19f91e88859519eb3c8b0e6b3bb6.1587277808.git.matheus.bernardino@usp.br","threadId":"53220","inReplyTo":"eaae7214925189f562056b1ee6972c05dcf76a32.1587103366.git.matheus.bernardino@usp.br","subject":"[PATCH v2] grep: follow conventions for printing paths w/ unusual chars","fromName":"Matheus Tavares","fromEmail":"matheus.bernardino@usp.br","sentAt":"2020-04-19T06:33:24Z","receivedAt":"2020-04-19T06:33:40Z","isPatch":true,"sender":{"key":"matheus.tavb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/12701583?v=4"},"body":"grep does not follow the conventions used by other Git commands when\nprinting paths that contain unusual characters (as double-quotes or\nnewlines). Commands such as ls-files, commit, status and diff will:\n\n- Quote and escape unusual pathnames, by default.\n- Print names verbatim and unquoted when \"-z\" is used.\n\nBut grep *never* quotes/escapes absolute paths with unusual chars and\n*always* quotes/escapes relative ones, even with \"-z\". Besides being\ninconsistent in its own output, the deviation from other Git commands\ncan be confusing. So let's make it follow the two rules above and add\nsome tests for this new behavior. Note that, making grep quote/escape\nall unusual paths by default, also make it fully compliant with the\ncore.quotePath configuration, which is currently ignored for absolute\npaths.\n\nReported-by: Greg Hurrell \nHelped-by: Johannes Schindelin \nSigned-off-by: Matheus Tavares \n---\n\nChanges in v2:\n- Squashed in Dscho's patch fixing the tests' failures in Windows by\n adding the FUNNYNAMES prereq.\n\n\n Documentation/git-grep.txt | 6 +++--\n builtin/grep.c | 46 +++++++++++++++++++++++++++----------\n t/t7810-grep.sh | 47 ++++++++++++++++++++++++++++++++++++++\n 3 files changed, 85 insertions(+), 14 deletions(-)\n\ndiff --git a/Documentation/git-grep.txt b/Documentation/git-grep.txt\nindex ddb6acc025..3109ce8fbe 100644\n--- a/Documentation/git-grep.txt\n+++ b/Documentation/git-grep.txt\n@@ -206,8 +206,10 @@ providing this option will cause it to die.\n \n -z::\n --null::\n-\tOutput \\0 instead of the character that normally follows a\n-\tfile name.\n+\tUse \\0 as the delimiter for pathnames in the output, and print\n+\tthem verbatim. Without this option, pathnames with \"unusual\"\n+\tcharacters are quoted as explained for the configuration\n+\tvariable core.quotePath (see git-config(1)).\n \n -o::\n --only-matching::\ndiff --git a/builtin/grep.c b/builtin/grep.c\nindex 99e2685090..bdf1a4bbc9 100644\n--- a/builtin/grep.c\n+++ b/builtin/grep.c\n@@ -295,6 +295,38 @@ static int grep_cmd_config(const char *var, const char *value, void *cb)\n \treturn st;\n }\n \n+static void grep_source_name(struct grep_opt *opt, const char *filename,\n+\t\t\t int tree_name_len, struct strbuf *out)\n+{\n+\tstrbuf_reset(out);\n+\n+\tif (opt->null_following_name) {\n+\t\tif (opt->relative && opt->prefix_length) {\n+\t\t\tstruct strbuf rel_buf = STRBUF_INIT;\n+\t\t\tconst char *rel_name =\n+\t\t\t\trelative_path(filename + tree_name_len,\n+\t\t\t\t\t opt->prefix, &rel_buf);\n+\n+\t\t\tif (tree_name_len)\n+\t\t\t\tstrbuf_add(out, filename, tree_name_len);\n+\n+\t\t\tstrbuf_addstr(out, rel_name);\n+\t\t\tstrbuf_release(&rel_buf);\n+\t\t} else {\n+\t\t\tstrbuf_addstr(out, filename);\n+\t\t}\n+\t\treturn;\n+\t}\n+\n+\tif (opt->relative && opt->prefix_length)\n+\t\tquote_path_relative(filename + tree_name_len, opt->prefix, out);\n+\telse\n+\t\tquote_c_style(filename + tree_name_len, out, NULL, 0);\n+\n+\tif (tree_name_len)\n+\t\tstrbuf_insert(out, 0, filename, tree_name_len);\n+}\n+\n static int grep_oid(struct grep_opt *opt, const struct object_id *oid,\n \t\t const char *filename, int tree_name_len,\n \t\t const char *path)\n@@ -302,13 +334,7 @@ static int grep_oid(struct grep_opt *opt, const struct object_id *oid,\n \tstruct strbuf pathbuf = STRBUF_INIT;\n \tstruct grep_source gs;\n \n-\tif (opt->relative && opt->prefix_length) {\n-\t\tquote_path_relative(filename + tree_name_len, opt->prefix, &pathbuf);\n-\t\tstrbuf_insert(&pathbuf, 0, filename, tree_name_len);\n-\t} else {\n-\t\tstrbuf_addstr(&pathbuf, filename);\n-\t}\n-\n+\tgrep_source_name(opt, filename, tree_name_len, &pathbuf);\n \tgrep_source_init(&gs, GREP_SOURCE_OID, pathbuf.buf, path, oid);\n \tstrbuf_release(&pathbuf);\n \n@@ -334,11 +360,7 @@ static int grep_file(struct grep_opt *opt, const char *filename)\n \tstruct strbuf buf = STRBUF_INIT;\n \tstruct grep_source gs;\n \n-\tif (opt->relative && opt->prefix_length)\n-\t\tquote_path_relative(filename, opt->prefix, &buf);\n-\telse\n-\t\tstrbuf_addstr(&buf, filename);\n-\n+\tgrep_source_name(opt, filename, 0, &buf);\n \tgrep_source_init(&gs, GREP_SOURCE_FILE, buf.buf, filename, filename);\n \tstrbuf_release(&buf);\n \ndiff --git a/t/t7810-grep.sh b/t/t7810-grep.sh\nindex 7d7b396c23..991d5bd9c0 100755\n--- a/t/t7810-grep.sh\n+++ b/t/t7810-grep.sh\n@@ -72,6 +72,11 @@ test_expect_success setup '\n \t# Still a no-op.\n \tfunction dummy() {}\n \tEOF\n+\tif test_have_prereq FUNNYNAMES\n+\tthen\n+\t\techo unusual >\"\\\"unusual\\\" pathname\" &&\n+\t\techo unusual >\"t/nested \\\"unusual\\\" pathname\"\n+\tfi &&\n \tgit add . &&\n \ttest_tick &&\n \tgit commit -m initial\n@@ -481,6 +486,48 @@ do\n \t\tgit grep --count -h -e b $H -- ab >actual &&\n \t\ttest_cmp expected actual\n \t'\n+\n+\ttest_expect_success FUNNYNAMES \"grep $L should quote unusual pathnames\" '\n+\t\tcat >expected <<-EOF &&\n+\t\t${HC}\"\\\"unusual\\\" pathname\":unusual\n+\t\t${HC}\"t/nested \\\"unusual\\\" pathname\":unusual\n+\t\tEOF\n+\t\tgit grep unusual $H >actual &&\n+\t\ttest_cmp expected actual\n+\t'\n+\n+\ttest_expect_success FUNNYNAMES \"grep $L in subdir should quote unusual relative pathnames\" '\n+\t\tcat >expected <<-EOF &&\n+\t\t${HC}\"nested \\\"unusual\\\" pathname\":unusual\n+\t\tEOF\n+\t\t(\n+\t\t\tcd t &&\n+\t\t\tgit grep unusual $H\n+\t\t) >actual &&\n+\t\ttest_cmp expected actual\n+\t'\n+\n+\ttest_expect_success FUNNYNAMES \"grep -z $L with unusual pathnames\" '\n+\t\tcat >expected <<-EOF &&\n+\t\t${HC}\"unusual\" pathname:unusual\n+\t\t${HC}t/nested \"unusual\" pathname:unusual\n+\t\tEOF\n+\t\tgit grep -z unusual $H >actual &&\n+\t\ttr \"\\0\" \":\" actual-replace-null &&\n+\t\ttest_cmp expected actual-replace-null\n+\t'\n+\n+\ttest_expect_success FUNNYNAMES \"grep -z $L in subdir with unusual relative pathnames\" '\n+\t\tcat >expected <<-EOF &&\n+\t\t${HC}nested \"unusual\" pathname:unusual\n+\t\tEOF\n+\t\t(\n+\t\t\tcd t &&\n+\t\t\tgit grep -z unusual $H\n+\t\t) >actual &&\n+\t\ttr \"\\0\" \":\" actual-replace-null &&\n+\t\ttest_cmp expected actual-replace-null\n+\t'\n done\n \n cat >expected <