{"thread":{"id":"52689","subject":"Bug or unexpected behaviour in git show <rev>:a\\b","startedAt":"2020-01-24T12:45:41Z","lastAt":"2020-01-31T00:15:38Z","messageCount":15,"participants":["David Burström","Jeff King","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"390358","messageId":"CAA-mk2_jB2X1J-NCXQ92spa-fmEct9G9f1iQo7t8=rvD5coFDA@mail.gmail.com","threadId":"52689","inReplyTo":null,"subject":"Bug or unexpected behaviour in git show <rev>:a\\b","fromName":"David Burström","fromEmail":"davidburstrom@spotify.com","sentAt":"2020-01-24T12:45:28Z","receivedAt":"2020-01-24T12:45:41Z","isPatch":false,"sender":{"key":"davidburstrom@spotify.com","avatar":null},"body":"Hi!\n\nAs part of some experiments on running git on Windows, I found\nsomething unexpected. If I use e.g. bash to run the following in a\nrepository:\n\ngit show HEAD:a\\\\b\n\nthe command stalls for a short while and then exits with 0 and prints\nnothing on stdout. I've verified this on git 2.25.0 and 2.21.0. I\nwould have expected either \"fatal: Path 'a\\b' does not exist in\n'HEAD'\" or something of that nature. Nothing in \"man git revisions\"\nhint that backslash is treated specially. Is this a bug or is it an\nundocumented feature?\n\nBest regards,\n  David Burström\n"},{"id":"390380","messageId":"20200124190119.GB1672342@coredump.intra.peff.net","threadId":"52689","inReplyTo":"CAA-mk2_jB2X1J-NCXQ92spa-fmEct9G9f1iQo7t8=rvD5coFDA@mail.gmail.com","subject":"Re: Bug or unexpected behaviour in git show <rev>:a\\b","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-01-24T19:01:19Z","receivedAt":"2020-01-24T19:01:21Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jan 24, 2020 at 01:45:28PM +0100, David Burström wrote:\n\n> As part of some experiments on running git on Windows, I found\n> something unexpected. If I use e.g. bash to run the following in a\n> repository:\n> \n> git show HEAD:a\\\\b\n> \n> the command stalls for a short while and then exits with 0 and prints\n> nothing on stdout. I've verified this on git 2.25.0 and 2.21.0. I\n> would have expected either \"fatal: Path 'a\\b' does not exist in\n> 'HEAD'\" or something of that nature. Nothing in \"man git revisions\"\n> hint that backslash is treated specially. Is this a bug or is it an\n> undocumented feature?\n\nMaybe both. :) This is an interesting case.\n\nThe arguments to git-show (like anything that invokes the revision\nmachinery, like git-log) can be _either_ an object identifier or a\npathspec. Since you didn't provide a \"--\" separator to disambiguate,\nwe'll try both.\n\nFor your case \"HEAD:a\\b\", as well as the more mundane \"HEAD:a-b\",\nget_oid() realizes they are not valid object identifiers (because the\nfiles don't exist in HEAD^{tree}). So then we consider it as a pathspec.\n\nIn the case of \"HEAD:a-b\", no such file exists in the working tree\neither, so we complain:\n\n $ git show HEAD:a-b\n fatal: Path 'a-b' does not exist in 'HEAD'\n\nand here verify_filename(), which produces the error, is smart enough to\ndiagnose that the bogus argument looks a lot like a request for a\npath-in-tree.\n\nBut since 28fcc0b71a (pathspec: avoid the need of \"--\" when wildcard is\nused, 2015-05-02), we allow non-files as pathspecs if they use magic\nglob meta-characters (to make it more convenient to use \"*.c\" or\nsimilar). And \"\\\" is such a meta-character. So we actually diff HEAD\nusing the pathspec \"HEAD:a\\b\", which of course matches nothing.\n\nYou can defeat this by using \"--\". I.e.:\n\n  [mundane name, dwim logic complains because file doesn't exist]\n  $ git show HEAD:a-b\n  fatal: Path 'a-b' does not exist in 'HEAD'\n\n  [mundane name, explicit pathspec does diff with no matches]\n  $ git show -- HEAD:a-b\n\n  [mundane name, explicit revision complains that it can't be resolved]\n  $ git show HEAD:a-b --\n  fatal: bad revision 'HEAD:a-b'\n\n  [exotic name, dwim considers it a pathspec due to metacharacter]\n  $ git show HEAD:a\\\\b\n\n  [exotic name, explicit pathspec does the same thing]\n  $ git show -- HEAD:a\\\\b\n\n  [exotic name, explicit revision is just like the mundane case]\n  $ git show HEAD:a\\\\b --\n  fatal: bad revision 'HEAD:a\\b'\n\nSo everything is working as designed, or at least explainable. But I\nthink there is some room for improvement. A backslash that isn't\nfollowed by a glob meta-character _is_ still a meta character (your\n\"a\\b\" would be globbing for \"ab\"). But it's useless enough that I think\nit shouldn't be enough to trigger the \"oh, you probably meant this as a\npathspec\" DWIM rule.\n\nWe _could_ also say \"even though this could technically be a pathspec\nbecause of its metacharacter, it looks vaguely enough like a\npath-in-tree revision that we shouldn't guess\". That I'm less\ncomfortable with, just because it makes the heuristics even more\nmagical.\n\nAlso, unrelated to your problem, but I find it interesting in the output\nabove that \"git show HEAD:foo --\" produces a less useful error message\nthan \"git show HEAD:foo\" without the separator, even though the user has\ngiven us even more information about their intent. I think the DWIM\nverify_filename() has grown a lot more diagnosis code over the years\nthat could also be used in the other code path.\n\n-Peff\n"},{"id":"390384","messageId":"xmqqk15gzmc8.fsf@gitster-ct.c.googlers.com","threadId":"52689","inReplyTo":"20200124190119.GB1672342@coredump.intra.peff.net","subject":"Re: Bug or unexpected behaviour in git show <rev>:a\\b","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-01-24T19:27:35Z","receivedAt":"2020-01-24T19:27:45Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> So everything is working as designed, or at least explainable. But I\n> think there is some room for improvement. A backslash that isn't\n> followed by a glob meta-character _is_ still a meta character (your\n> \"a\\b\" would be globbing for \"ab\"). But it's useless enough that I think\n> it shouldn't be enough to trigger the \"oh, you probably meant this as a\n> pathspec\" DWIM rule.\n\nThis sounds sensible.\n\n> We _could_ also say \"even though this could technically be a pathspec\n> because of its metacharacter, it looks vaguely enough like a\n> path-in-tree revision that we shouldn't guess\". That I'm less\n> comfortable with, just because it makes the heuristics even more\n> magical.\n\nNot just it becomes more magical, I am afraid that the code to\nimplement such a heuristics would be fragile and become a source of\nunnecessary bugs.  Let's not go there.\n\nI should learn to use \"working as designed or at least explainable\"\nmore often in my responses, by the way.  That's quite a useful and\ngood phrase ;-)\n\n"},{"id":"390426","messageId":"20200125000051.GA566074@coredump.intra.peff.net","threadId":"52689","inReplyTo":"xmqqk15gzmc8.fsf@gitster-ct.c.googlers.com","subject":"Re: Bug or unexpected behaviour in git show <rev>:a\\b","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-01-25T00:00:51Z","receivedAt":"2020-01-25T00:00:54Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jan 24, 2020 at 11:27:35AM -0800, Junio C Hamano wrote:\n\n> Jeff King <peff@peff.net> writes:\n> \n> > So everything is working as designed, or at least explainable. But I\n> > think there is some room for improvement. A backslash that isn't\n> > followed by a glob meta-character _is_ still a meta character (your\n> > \"a\\b\" would be globbing for \"ab\"). But it's useless enough that I think\n> > it shouldn't be enough to trigger the \"oh, you probably meant this as a\n> > pathspec\" DWIM rule.\n> \n> This sounds sensible.\n\nOK, the patch I came up with is below.\n\n> > We _could_ also say \"even though this could technically be a pathspec\n> > because of its metacharacter, it looks vaguely enough like a\n> > path-in-tree revision that we shouldn't guess\". That I'm less\n> > comfortable with, just because it makes the heuristics even more\n> > magical.\n> \n> Not just it becomes more magical, I am afraid that the code to\n> implement such a heuristics would be fragile and become a source of\n> unnecessary bugs.  Let's not go there.\n\nOK. It does mean that:\n\n  git show HEAD:a*\n\nwill still quietly produce no output instead of saying \"hey, there is no\na* in HEAD\". But I think given the lack of bug reports over the years\nthat this case (and the backslash one I'm fixing) are probably\nrelatively rare.  The backslash one seems a lot more likely, just\nbecause Windows folks may treat it like a path separator (I'm not sure\nif that even works, considering its meaning in a glob, but certainly I\ncan imagine somebody doing so as an experiment and getting confused by\nthe result).\n\n> I should learn to use \"working as designed or at least explainable\"\n> more often in my responses, by the way.  That's quite a useful and\n> good phrase ;-)\n\nPerhaps that can be Git's motto. ;)\n\nAnyway, here's the patch. Even though this is rare, I think it's worth\ndoing. The code is simple and I don't anticipate anybody complaining\nabout the tightening.\n\n-- >8 --\nSubject: verify_filename(): handle backslashes in \"wildcards are pathspecs\" rule\n\nCommit 28fcc0b71a (pathspec: avoid the need of \"--\" when wildcard is\nused, 2015-05-02) allowed:\n\n  git rev-parse '*.c'\n\nwithout the double-dash. But the rule it uses to check for wildcards\nactually looks for any glob special. This is overly liberal, as it means\nthat a pattern that doesn't actually do any wildcard matching, like\n\"a\\b\", will be considered a pathspec.\n\nIf you do have such a file on disk, that's presumably what you wanted.\nBut if you don't, the results are confusing: rather than say \"there's no\nsuch path a\\b\", we'll quietly accept it as a pathspec which very likely\nmatches nothing (or at least not what you intended). Likewise, looking\nfor path \"a\\*b\" doesn't expand the search at all; it would only find a\nsingle entry, \"a*b\".\n\nThis commit switches the rule to trigger only when glob metacharacters\nwould expand the search, meaning both of those cases will now report an\nerror (you can still disambiguate using \"--\", of course; we're just\ntightening the DWIM heuristic).\n\nNote that we didn't test the original feature in 28fcc0b71a at all. So\nthis patch not only tests for these corner cases, but also adds a\nregression test for the existing behavior.\n\nReported-by: David Burström <davidburstrom@spotify.com>\nSigned-off-by: Jeff King <peff@peff.net>\n---\n setup.c                        | 23 ++++++++++++++++++++---\n t/t1506-rev-parse-diagnosis.sh | 14 ++++++++++++++\n 2 files changed, 34 insertions(+), 3 deletions(-)\n\ndiff --git a/setup.c b/setup.c\nindex e2a479a64f..12228c0d9c 100644\n--- a/setup.c\n+++ b/setup.c\n@@ -197,9 +197,26 @@ static void NORETURN die_verify_filename(struct repository *r,\n  */\n static int looks_like_pathspec(const char *arg)\n {\n-\t/* anything with a wildcard character */\n-\tif (!no_wildcard(arg))\n-\t\treturn 1;\n+\tconst char *p;\n+\tint escaped = 0;\n+\n+\t/*\n+\t * Wildcard characters imply the user is looking to match pathspecs\n+\t * that aren't in the filesystem. Note that this doesn't include\n+\t * backslash even though it's a glob special; by itself it doesn't\n+\t * cause any increase in the match. Likewise ignore backslash-escaped\n+\t * wildcard characters.\n+\t */\n+\tfor (p = arg; *p; p++) {\n+\t\tif (escaped) {\n+\t\t\tescaped = 0;\n+\t\t} else if (is_glob_special(*p)) {\n+\t\t\tif (*p == '\\\\')\n+\t\t\t\tescaped = 1;\n+\t\t\telse\n+\t\t\t\treturn 1;\n+\t\t}\n+\t}\n \n \t/* long-form pathspec magic */\n \tif (starts_with(arg, \":(\"))\ndiff --git a/t/t1506-rev-parse-diagnosis.sh b/t/t1506-rev-parse-diagnosis.sh\nindex 6d951ca015..8a75f37a11 100755\n--- a/t/t1506-rev-parse-diagnosis.sh\n+++ b/t/t1506-rev-parse-diagnosis.sh\n@@ -222,4 +222,18 @@ test_expect_success 'reject Nth ancestor if N is too high' '\n \ttest_must_fail git rev-parse HEAD~100000000000000000000000000000000\n '\n \n+test_expect_success 'pathspecs with wildcards are not ambiguous' '\n+\techo \"*.c\" >expect &&\n+\tgit rev-parse \"*.c\" >actual &&\n+\ttest_cmp expect actual\n+'\n+\n+test_expect_success 'backslash does not trigger wildcard rule' '\n+\ttest_must_fail git rev-parse \"foo\\\\bar\"\n+'\n+\n+test_expect_success 'escaped char does not trigger wildcard rule' '\n+\ttest_must_fail git rev-parse \"foo\\\\*bar\"\n+'\n+\n test_done\n-- \n2.25.0.421.gb74d19af79\n\n"},{"id":"390427","messageId":"20200125000542.GA566779@coredump.intra.peff.net","threadId":"52689","inReplyTo":"20200124190119.GB1672342@coredump.intra.peff.net","subject":"Re: Bug or unexpected behaviour in git show <rev>:a\\b","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-01-25T00:05:42Z","receivedAt":"2020-01-25T00:05:44Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Fri, Jan 24, 2020 at 02:01:19PM -0500, Jeff King wrote:\n\n> Also, unrelated to your problem, but I find it interesting in the output\n> above that \"git show HEAD:foo --\" produces a less useful error message\n> than \"git show HEAD:foo\" without the separator, even though the user has\n> given us even more information about their intent. I think the DWIM\n> verify_filename() has grown a lot more diagnosis code over the years\n> that could also be used in the other code path.\n\nI took a look at this, but it gets ugly pretty quickly, so I gave up.\nThe problem is that all of the good errors are produced by get_oid()\nwith GET_OID_ONLY_TO_DIE, but the moment where we decide to produce the\nbad error is sometimes removed from the get_oid() call. E.g., in\nsomething like \"a..b\", we'd want to report on whichever of \"a\" or \"b\"\ncaused a problem, but at the point of reporting the error, we only have\nthe unparsed \"a..b\".\n\nCertainly this can be overcome, but it seemed like a rabbit-hole that I\ndidn't feel like going down today. Anybody is welcome to give it a shot.\nI suspect the whole thing would be simpler if get_oid() could optionally\nreturn an error string to the caller, instead of forcing us to call it a\nsecond time with the \"only to die\" flag.\n\nSpeaking of which, I did go part-way down a different rabbit-hole in\npreparation, and ended up with these patches. So it seemed worth posting\nthem.\n\n  [1/3]: t1400: avoid \"test\" string comparisons\n  [2/3]: t1506: drop space after redirection operator\n  [3/3]: sha1-name: mark get_oid() error messages for translation\n\n sha1-name.c                    | 30 +++++++++++------------\n t/t1400-update-ref.sh          | 36 ++++++++++++++++++----------\n t/t1506-rev-parse-diagnosis.sh | 44 +++++++++++++++++-----------------\n 3 files changed, 61 insertions(+), 49 deletions(-)\n\n-Peff\n"},{"id":"390428","messageId":"20200125000624.GA567109@coredump.intra.peff.net","threadId":"52689","inReplyTo":"20200125000542.GA566779@coredump.intra.peff.net","subject":"[PATCH 1/3] t1400: avoid \"test\" string comparisons","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-01-25T00:06:24Z","receivedAt":"2020-01-25T00:06:27Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Using the shell \"test\" here is inflexible, because we can't easily swap\nit out for an i18n-aware version like we can with test_cmp and\ntest_i18ncmp. And it's not even saving us any processes, since we have\nto use \"cat\" to get the output. So let's switch to using test_cmp, which\nhas the added bonus that it will produce better output if there's a\nfailure.\n\nNote that not all of the changed outputs here are candidates for\ntranslation, but I've converted all of them for consistency and to\nbenefit from the better output.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n t/t1400-update-ref.sh | 36 ++++++++++++++++++++++++------------\n 1 file changed, 24 insertions(+), 12 deletions(-)\n\ndiff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh\nindex b815cdd1b8..a86dd2fbd9 100755\n--- a/t/t1400-update-ref.sh\n+++ b/t/t1400-update-ref.sh\n@@ -361,55 +361,67 @@ ld=\"Thu, 26 May 2005 18:43:00 -0500\"\n test_expect_success 'Query \"master@{May 25 2005}\" (before history)' '\n \ttest_when_finished \"rm -f o e\" &&\n \tgit rev-parse --verify \"master@{May 25 2005}\" >o 2>e &&\n-\ttest $C = $(cat o) &&\n-\ttest \"warning: Log for '\\''master'\\'' only goes back to $ed.\" = \"$(cat e)\"\n+\techo \"$C\" >expect &&\n+\ttest_cmp expect o &&\n+\techo \"warning: Log for '\\''master'\\'' only goes back to $ed.\" >expect &&\n+\ttest_cmp expect e\n '\n test_expect_success 'Query master@{2005-05-25} (before history)' '\n \ttest_when_finished \"rm -f o e\" &&\n \tgit rev-parse --verify master@{2005-05-25} >o 2>e &&\n-\ttest $C = $(cat o) &&\n-\ttest \"warning: Log for '\\''master'\\'' only goes back to $ed.\" = \"$(cat e)\"\n+\techo \"$C\" >expect &&\n+\ttest_cmp expect o &&\n+\techo \"warning: Log for '\\''master'\\'' only goes back to $ed.\" >expect &&\n+\ttest_cmp expect e\n '\n test_expect_success 'Query \"master@{May 26 2005 23:31:59}\" (1 second before history)' '\n \ttest_when_finished \"rm -f o e\" &&\n \tgit rev-parse --verify \"master@{May 26 2005 23:31:59}\" >o 2>e &&\n-\ttest $C = $(cat o) &&\n-\ttest \"warning: Log for '\\''master'\\'' only goes back to $ed.\" = \"$(cat e)\"\n+\techo \"$C\" >expect &&\n+\ttest_cmp expect o &&\n+\techo \"warning: Log for '\\''master'\\'' only goes back to $ed.\" >expect &&\n+\ttest_cmp expect e\n '\n test_expect_success 'Query \"master@{May 26 2005 23:32:00}\" (exactly history start)' '\n \ttest_when_finished \"rm -f o e\" &&\n \tgit rev-parse --verify \"master@{May 26 2005 23:32:00}\" >o 2>e &&\n-\ttest $C = $(cat o) &&\n+\techo \"$C\" >expect &&\n+\ttest_cmp expect o &&\n \ttest_must_be_empty e\n '\n test_expect_success 'Query \"master@{May 26 2005 23:32:30}\" (first non-creation change)' '\n \ttest_when_finished \"rm -f o e\" &&\n \tgit rev-parse --verify \"master@{May 26 2005 23:32:30}\" >o 2>e &&\n-\ttest $A = $(cat o) &&\n+\techo \"$A\" >expect &&\n+\ttest_cmp expect o &&\n \ttest_must_be_empty e\n '\n test_expect_success 'Query \"master@{2005-05-26 23:33:01}\" (middle of history with gap)' '\n \ttest_when_finished \"rm -f o e\" &&\n \tgit rev-parse --verify \"master@{2005-05-26 23:33:01}\" >o 2>e &&\n-\ttest $B = $(cat o) &&\n+\techo \"$B\" >expect &&\n+\ttest_cmp expect o &&\n \ttest_i18ngrep -F \"warning: log for ref $m has gap after $gd\" e\n '\n test_expect_success 'Query \"master@{2005-05-26 23:38:00}\" (middle of history)' '\n \ttest_when_finished \"rm -f o e\" &&\n \tgit rev-parse --verify \"master@{2005-05-26 23:38:00}\" >o 2>e &&\n-\ttest $Z = $(cat o) &&\n+\techo \"$Z\" >expect &&\n+\ttest_cmp expect o &&\n \ttest_must_be_empty e\n '\n test_expect_success 'Query \"master@{2005-05-26 23:43:00}\" (exact end of history)' '\n \ttest_when_finished \"rm -f o e\" &&\n \tgit rev-parse --verify \"master@{2005-05-26 23:43:00}\" >o 2>e &&\n-\ttest $E = $(cat o) &&\n+\techo \"$E\" >expect &&\n+\ttest_cmp expect o &&\n \ttest_must_be_empty e\n '\n test_expect_success 'Query \"master@{2005-05-28}\" (past end of history)' '\n \ttest_when_finished \"rm -f o e\" &&\n \tgit rev-parse --verify \"master@{2005-05-28}\" >o 2>e &&\n-\ttest $D = $(cat o) &&\n+\techo \"$D\" >expect &&\n+\ttest_cmp expect o &&\n \ttest_i18ngrep -F \"warning: log for ref $m unexpectedly ended on $ld\" e\n '\n \n-- \n2.25.0.421.gb74d19af79\n\n"},{"id":"390429","messageId":"20200125000631.GB567109@coredump.intra.peff.net","threadId":"52689","inReplyTo":"20200125000542.GA566779@coredump.intra.peff.net","subject":"[PATCH 2/3] t1506: drop space after redirection operator","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-01-25T00:06:31Z","receivedAt":"2020-01-25T00:06:33Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"Some (but not all!) redirections in this file are spelled \"2> error\".\nLet's switch them to our usual style.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\n t/t1506-rev-parse-diagnosis.sh | 22 +++++++++++-----------\n 1 file changed, 11 insertions(+), 11 deletions(-)\n\ndiff --git a/t/t1506-rev-parse-diagnosis.sh b/t/t1506-rev-parse-diagnosis.sh\nindex 8a75f37a11..f49fc770d6 100755\n--- a/t/t1506-rev-parse-diagnosis.sh\n+++ b/t/t1506-rev-parse-diagnosis.sh\n@@ -104,35 +104,35 @@ test_expect_success 'correct relative file objects (6)' '\n test_expect_success 'incorrect revision id' '\n \ttest_must_fail git rev-parse foobar:file.txt 2>error &&\n \tgrep \"Invalid object name '\"'\"'foobar'\"'\"'.\" error &&\n-\ttest_must_fail git rev-parse foobar 2> error &&\n+\ttest_must_fail git rev-parse foobar 2>error &&\n \ttest_i18ngrep \"unknown revision or path not in the working tree.\" error\n '\n \n test_expect_success 'incorrect file in sha1:path' '\n-\ttest_must_fail git rev-parse HEAD:nothing.txt 2> error &&\n+\ttest_must_fail git rev-parse HEAD:nothing.txt 2>error &&\n \tgrep \"fatal: Path '\"'\"'nothing.txt'\"'\"' does not exist in '\"'\"'HEAD'\"'\"'\" error &&\n-\ttest_must_fail git rev-parse HEAD:index-only.txt 2> error &&\n+\ttest_must_fail git rev-parse HEAD:index-only.txt 2>error &&\n \tgrep \"fatal: Path '\"'\"'index-only.txt'\"'\"' exists on disk, but not in '\"'\"'HEAD'\"'\"'.\" error &&\n \t(cd subdir &&\n-\t test_must_fail git rev-parse HEAD:file2.txt 2> error &&\n+\t test_must_fail git rev-parse HEAD:file2.txt 2>error &&\n \t test_did_you_mean HEAD subdir/ file2.txt exists )\n '\n \n test_expect_success 'incorrect file in :path and :N:path' '\n-\ttest_must_fail git rev-parse :nothing.txt 2> error &&\n+\ttest_must_fail git rev-parse :nothing.txt 2>error &&\n \tgrep \"fatal: Path '\"'\"'nothing.txt'\"'\"' does not exist (neither on disk nor in the index).\" error &&\n-\ttest_must_fail git rev-parse :1:nothing.txt 2> error &&\n+\ttest_must_fail git rev-parse :1:nothing.txt 2>error &&\n \tgrep \"Path '\"'\"'nothing.txt'\"'\"' does not exist (neither on disk nor in the index).\" error &&\n-\ttest_must_fail git rev-parse :1:file.txt 2> error &&\n+\ttest_must_fail git rev-parse :1:file.txt 2>error &&\n \ttest_did_you_mean \":0\" \"\" file.txt \"is in the index\" \"at stage 1\" &&\n \t(cd subdir &&\n-\t test_must_fail git rev-parse :1:file.txt 2> error &&\n+\t test_must_fail git rev-parse :1:file.txt 2>error &&\n \t test_did_you_mean \":0\" \"\" file.txt \"is in the index\" \"at stage 1\" &&\n-\t test_must_fail git rev-parse :file2.txt 2> error &&\n+\t test_must_fail git rev-parse :file2.txt 2>error &&\n \t test_did_you_mean \":0\" subdir/ file2.txt \"is in the index\" &&\n-\t test_must_fail git rev-parse :2:file2.txt 2> error &&\n+\t test_must_fail git rev-parse :2:file2.txt 2>error &&\n \t test_did_you_mean :0 subdir/ file2.txt \"is in the index\") &&\n-\ttest_must_fail git rev-parse :disk-only.txt 2> error &&\n+\ttest_must_fail git rev-parse :disk-only.txt 2>error &&\n \tgrep \"fatal: Path '\"'\"'disk-only.txt'\"'\"' exists on disk, but not in the index.\" error\n '\n \n-- \n2.25.0.421.gb74d19af79\n\n"},{"id":"390431","messageId":"20200125001301.GC567109@coredump.intra.peff.net","threadId":"52689","inReplyTo":"20200125000542.GA566779@coredump.intra.peff.net","subject":"[PATCH 3/3] sha1-name: mark get_oid() error messages for translation","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-01-25T00:13:01Z","receivedAt":"2020-01-25T00:13:05Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"There are several error messages in get_oid() and its children that are\nclearly intended for humans, but aren't marked for translation. E.g.:\n\n  $ git show :1:foo\n  fatal: Path 'foo' is in the index, but not at stage 1.\n  Did you mean ':0:foo'?\n\nLet's mark these for translation. While we're at it, let's switch the\nstyle to be more like our usual error messages: start with a lowercase\nletter and omit a period at the end of the line.\n\nThis does mean that multi-line messages like the one above don't have\nany punctuation between the two sentences. I solved that by adding a\n\"hint\" marker like we'd see from advise(). So the result is:\n\n  $ git show :1:foo\n  fatal: path 'foo' is in the index, but not at stage 1\n  hint: Did you mean ':0:foo'?\n\nA few tests had to be switched to test_i18ngrep and test_i18ncmp. Since\nwe were touching them anyway, I also simplified the ones using i18ngrep\na bit for readability.\n\nSigned-off-by: Jeff King <peff@peff.net>\n---\nI'm on the fence about whether the resulting message is clear enough.\nCertainly we could just mark them for translation but leave the\ncapitalization and punctuation as-is.\n\nI actually think _both_ of those lines could be considered hints. And\nsomething like:\n\n  hint: path 'foo' is in the index, but not at stage 1;\n  hint: did you mean ':0:foo'?\n  fatal: unable to resolve object name ':1:foo'\n\nmight be better still. But I suspect the text in that final \"fatal\"\ncould be better if done by the caller of get_oid(). So that takes me\nback to my earlier suggestion that get_oid() should actually be\ncollecting this hints as it runs and making them available to the\ncaller. I punted on that for now, though.\n\n sha1-name.c                    | 30 +++++++++++++++---------------\n t/t1400-update-ref.sh          | 12 ++++++------\n t/t1506-rev-parse-diagnosis.sh | 22 +++++++++++-----------\n 3 files changed, 32 insertions(+), 32 deletions(-)\n\ndiff --git a/sha1-name.c b/sha1-name.c\nindex 200eb373ad..75235cb490 100644\n--- a/sha1-name.c\n+++ b/sha1-name.c\n@@ -919,15 +919,15 @@ static int get_oid_basic(struct repository *r, const char *str, int len,\n \t\t\t}\n \t\t\tif (at_time) {\n \t\t\t\tif (!(flags & GET_OID_QUIETLY)) {\n-\t\t\t\t\twarning(\"Log for '%.*s' only goes \"\n-\t\t\t\t\t\t\"back to %s.\", len, str,\n+\t\t\t\t\twarning(_(\"log for '%.*s' only goes back to %s\"),\n+\t\t\t\t\t\tlen, str,\n \t\t\t\t\t\tshow_date(co_time, co_tz, DATE_MODE(RFC2822)));\n \t\t\t\t}\n \t\t\t} else {\n \t\t\t\tif (flags & GET_OID_QUIETLY) {\n \t\t\t\t\texit(128);\n \t\t\t\t}\n-\t\t\t\tdie(\"Log for '%.*s' only has %d entries.\",\n+\t\t\t\tdie(_(\"log for '%.*s' only has %d entries\"),\n \t\t\t\t    len, str, co_cnt);\n \t\t\t}\n \t\t}\n@@ -1692,22 +1692,22 @@ static void diagnose_invalid_oid_path(struct repository *r,\n \t\tprefix = \"\";\n \n \tif (file_exists(filename))\n-\t\tdie(\"Path '%s' exists on disk, but not in '%.*s'.\",\n+\t\tdie(_(\"path '%s' exists on disk, but not in '%.*s'\"),\n \t\t    filename, object_name_len, object_name);\n \tif (is_missing_file_error(errno)) {\n \t\tchar *fullname = xstrfmt(\"%s%s\", prefix, filename);\n \n \t\tif (!get_tree_entry(r, tree_oid, fullname, &oid, &mode)) {\n-\t\t\tdie(\"Path '%s' exists, but not '%s'.\\n\"\n-\t\t\t    \"Did you mean '%.*s:%s' aka '%.*s:./%s'?\",\n+\t\t\tdie(_(\"path '%s' exists, but not '%s'\\n\"\n+\t\t\t    \"hint: Did you mean '%.*s:%s' aka '%.*s:./%s'?\"),\n \t\t\t    fullname,\n \t\t\t    filename,\n \t\t\t    object_name_len, object_name,\n \t\t\t    fullname,\n \t\t\t    object_name_len, object_name,\n \t\t\t    filename);\n \t\t}\n-\t\tdie(\"Path '%s' does not exist in '%.*s'\",\n+\t\tdie(_(\"path '%s' does not exist in '%.*s'\"),\n \t\t    filename, object_name_len, object_name);\n \t}\n }\n@@ -1735,8 +1735,8 @@ static void diagnose_invalid_index_path(struct repository *r,\n \t\tce = istate->cache[pos];\n \t\tif (ce_namelen(ce) == namelen &&\n \t\t    !memcmp(ce->name, filename, namelen))\n-\t\t\tdie(\"Path '%s' is in the index, but not at stage %d.\\n\"\n-\t\t\t    \"Did you mean ':%d:%s'?\",\n+\t\t\tdie(_(\"path '%s' is in the index, but not at stage %d\\n\"\n+\t\t\t    \"hint: Did you mean ':%d:%s'?\"),\n \t\t\t    filename, stage,\n \t\t\t    ce_stage(ce), filename);\n \t}\n@@ -1751,17 +1751,17 @@ static void diagnose_invalid_index_path(struct repository *r,\n \t\tce = istate->cache[pos];\n \t\tif (ce_namelen(ce) == fullname.len &&\n \t\t    !memcmp(ce->name, fullname.buf, fullname.len))\n-\t\t\tdie(\"Path '%s' is in the index, but not '%s'.\\n\"\n-\t\t\t    \"Did you mean ':%d:%s' aka ':%d:./%s'?\",\n+\t\t\tdie(_(\"path '%s' is in the index, but not '%s'\\n\"\n+\t\t\t    \"hint: Did you mean ':%d:%s' aka ':%d:./%s'?\"),\n \t\t\t    fullname.buf, filename,\n \t\t\t    ce_stage(ce), fullname.buf,\n \t\t\t    ce_stage(ce), filename);\n \t}\n \n \tif (repo_file_exists(r, filename))\n-\t\tdie(\"Path '%s' exists on disk, but not in the index.\", filename);\n+\t\tdie(_(\"path '%s' exists on disk, but not in the index\"), filename);\n \tif (is_missing_file_error(errno))\n-\t\tdie(\"Path '%s' does not exist (neither on disk nor in the index).\",\n+\t\tdie(_(\"path '%s' does not exist (neither on disk nor in the index)\"),\n \t\t    filename);\n \n \tstrbuf_release(&fullname);\n@@ -1774,7 +1774,7 @@ static char *resolve_relative_path(struct repository *r, const char *rel)\n \t\treturn NULL;\n \n \tif (r != the_repository || !is_inside_work_tree())\n-\t\tdie(\"relative path syntax can't be used outside working tree.\");\n+\t\tdie(_(\"relative path syntax can't be used outside working tree\"));\n \n \t/* die() inside prefix_path() if resolved path is outside worktree */\n \treturn prefix_path(startup_info->prefix,\n@@ -1912,7 +1912,7 @@ static enum get_oid_result get_oid_with_context_1(struct repository *repo,\n \t\t\treturn ret;\n \t\t} else {\n \t\t\tif (only_to_die)\n-\t\t\t\tdie(\"Invalid object name '%.*s'.\", len, name);\n+\t\t\t\tdie(_(\"invalid object name '%.*s'.\"), len, name);\n \t\t}\n \t}\n \treturn ret;\ndiff --git a/t/t1400-update-ref.sh b/t/t1400-update-ref.sh\nindex a86dd2fbd9..a6224ef65f 100755\n--- a/t/t1400-update-ref.sh\n+++ b/t/t1400-update-ref.sh\n@@ -363,24 +363,24 @@ test_expect_success 'Query \"master@{May 25 2005}\" (before history)' '\n \tgit rev-parse --verify \"master@{May 25 2005}\" >o 2>e &&\n \techo \"$C\" >expect &&\n \ttest_cmp expect o &&\n-\techo \"warning: Log for '\\''master'\\'' only goes back to $ed.\" >expect &&\n-\ttest_cmp expect e\n+\techo \"warning: log for '\\''master'\\'' only goes back to $ed\" >expect &&\n+\ttest_i18ncmp expect e\n '\n test_expect_success 'Query master@{2005-05-25} (before history)' '\n \ttest_when_finished \"rm -f o e\" &&\n \tgit rev-parse --verify master@{2005-05-25} >o 2>e &&\n \techo \"$C\" >expect &&\n \ttest_cmp expect o &&\n-\techo \"warning: Log for '\\''master'\\'' only goes back to $ed.\" >expect &&\n-\ttest_cmp expect e\n+\techo \"warning: log for '\\''master'\\'' only goes back to $ed\" >expect &&\n+\ttest_i18ncmp expect e\n '\n test_expect_success 'Query \"master@{May 26 2005 23:31:59}\" (1 second before history)' '\n \ttest_when_finished \"rm -f o e\" &&\n \tgit rev-parse --verify \"master@{May 26 2005 23:31:59}\" >o 2>e &&\n \techo \"$C\" >expect &&\n \ttest_cmp expect o &&\n-\techo \"warning: Log for '\\''master'\\'' only goes back to $ed.\" >expect &&\n-\ttest_cmp expect e\n+\techo \"warning: log for '\\''master'\\'' only goes back to $ed\" >expect &&\n+\ttest_i18ncmp expect e\n '\n test_expect_success 'Query \"master@{May 26 2005 23:32:00}\" (exactly history start)' '\n \ttest_when_finished \"rm -f o e\" &&\ndiff --git a/t/t1506-rev-parse-diagnosis.sh b/t/t1506-rev-parse-diagnosis.sh\nindex f49fc770d6..c2b5125c12 100755\n--- a/t/t1506-rev-parse-diagnosis.sh\n+++ b/t/t1506-rev-parse-diagnosis.sh\n@@ -9,8 +9,8 @@ exec </dev/null\n test_did_you_mean ()\n {\n \tcat >expected <<-EOF &&\n-\tfatal: Path '$2$3' $4, but not ${5:-$SQ$3$SQ}.\n-\tDid you mean '$1:$2$3'${2:+ aka $SQ$1:./$3$SQ}?\n+\tfatal: path '$2$3' $4, but not ${5:-$SQ$3$SQ}\n+\thint: Did you mean '$1:$2$3'${2:+ aka $SQ$1:./$3$SQ}?\n \tEOF\n \ttest_cmp expected error\n }\n@@ -103,26 +103,26 @@ test_expect_success 'correct relative file objects (6)' '\n \n test_expect_success 'incorrect revision id' '\n \ttest_must_fail git rev-parse foobar:file.txt 2>error &&\n-\tgrep \"Invalid object name '\"'\"'foobar'\"'\"'.\" error &&\n+\ttest_i18ngrep \"invalid object name .foobar.\" error &&\n \ttest_must_fail git rev-parse foobar 2>error &&\n \ttest_i18ngrep \"unknown revision or path not in the working tree.\" error\n '\n \n test_expect_success 'incorrect file in sha1:path' '\n \ttest_must_fail git rev-parse HEAD:nothing.txt 2>error &&\n-\tgrep \"fatal: Path '\"'\"'nothing.txt'\"'\"' does not exist in '\"'\"'HEAD'\"'\"'\" error &&\n+\ttest_i18ngrep \"path .nothing.txt. does not exist in .HEAD.\" error &&\n \ttest_must_fail git rev-parse HEAD:index-only.txt 2>error &&\n-\tgrep \"fatal: Path '\"'\"'index-only.txt'\"'\"' exists on disk, but not in '\"'\"'HEAD'\"'\"'.\" error &&\n+\ttest_i18ngrep \"path .index-only.txt. exists on disk, but not in .HEAD.\" error &&\n \t(cd subdir &&\n \t test_must_fail git rev-parse HEAD:file2.txt 2>error &&\n \t test_did_you_mean HEAD subdir/ file2.txt exists )\n '\n \n test_expect_success 'incorrect file in :path and :N:path' '\n \ttest_must_fail git rev-parse :nothing.txt 2>error &&\n-\tgrep \"fatal: Path '\"'\"'nothing.txt'\"'\"' does not exist (neither on disk nor in the index).\" error &&\n+\ttest_i18ngrep \"path .nothing.txt. does not exist (neither on disk nor in the index)\" error &&\n \ttest_must_fail git rev-parse :1:nothing.txt 2>error &&\n-\tgrep \"Path '\"'\"'nothing.txt'\"'\"' does not exist (neither on disk nor in the index).\" error &&\n+\ttest_i18ngrep \"path .nothing.txt. does not exist (neither on disk nor in the index)\" error &&\n \ttest_must_fail git rev-parse :1:file.txt 2>error &&\n \ttest_did_you_mean \":0\" \"\" file.txt \"is in the index\" \"at stage 1\" &&\n \t(cd subdir &&\n@@ -133,16 +133,16 @@ test_expect_success 'incorrect file in :path and :N:path' '\n \t test_must_fail git rev-parse :2:file2.txt 2>error &&\n \t test_did_you_mean :0 subdir/ file2.txt \"is in the index\") &&\n \ttest_must_fail git rev-parse :disk-only.txt 2>error &&\n-\tgrep \"fatal: Path '\"'\"'disk-only.txt'\"'\"' exists on disk, but not in the index.\" error\n+\ttest_i18ngrep \"path .disk-only.txt. exists on disk, but not in the index\" error\n '\n \n test_expect_success 'invalid @{n} reference' '\n \ttest_must_fail git rev-parse master@{99999} >output 2>error &&\n \ttest_must_be_empty output &&\n-\tgrep \"fatal: Log for [^ ]* only has [0-9][0-9]* entries.\" error  &&\n+\ttest_i18ngrep \"log for [^ ]* only has [0-9][0-9]* entries\" error  &&\n \ttest_must_fail git rev-parse --verify master@{99999} >output 2>error &&\n \ttest_must_be_empty output &&\n-\tgrep \"fatal: Log for [^ ]* only has [0-9][0-9]* entries.\" error\n+\ttest_i18ngrep \"log for [^ ]* only has [0-9][0-9]* entries\" error\n '\n \n test_expect_success 'relative path not found' '\n@@ -162,7 +162,7 @@ test_expect_success 'relative path outside worktree' '\n test_expect_success 'relative path when cwd is outside worktree' '\n \ttest_must_fail git --git-dir=.git --work-tree=subdir rev-parse HEAD:./file.txt >output 2>error &&\n \ttest_must_be_empty output &&\n-\tgrep \"relative path syntax can.t be used outside working tree.\" error\n+\ttest_i18ngrep \"relative path syntax can.t be used outside working tree\" error\n '\n \n test_expect_success '<commit>:file correctly diagnosed after a pathname' '\n-- \n2.25.0.421.gb74d19af79\n"},{"id":"390455","messageId":"CAA-mk2-X9kdS1h+6P0Y5wASHjFPJBKK-Dw9C1+OJL5U-nouVRg@mail.gmail.com","threadId":"52689","inReplyTo":"20200125000051.GA566074@coredump.intra.peff.net","subject":"Re: Bug or unexpected behaviour in git show <rev>:a\\b","fromName":"David Burström","fromEmail":"davidburstrom@spotify.com","sentAt":"2020-01-25T13:21:25Z","receivedAt":"2020-01-25T13:21:38Z","isPatch":false,"sender":{"key":"davidburstrom@spotify.com","avatar":null},"body":"On Sat, 25 Jan 2020 at 01:00, Jeff King <peff@peff.net> wrote:\n> > > We _could_ also say \"even though this could technically be a pathspec\n> > > because of its metacharacter, it looks vaguely enough like a\n> > > path-in-tree revision that we shouldn't guess\". That I'm less\n> > > comfortable with, just because it makes the heuristics even more\n> > > magical.\n> >\n> > Not just it becomes more magical, I am afraid that the code to\n> > implement such a heuristics would be fragile and become a source of\n> > unnecessary bugs.  Let's not go there.\n>\n> OK. It does mean that:\n>\n>   git show HEAD:a*\n>\n> will still quietly produce no output instead of saying \"hey, there is no\n> a* in HEAD\". But I think given the lack of bug reports over the years\n> that this case (and the backslash one I'm fixing) are probably\n> relatively rare.  The backslash one seems a lot more likely, just\n> because Windows folks may treat it like a path separator (I'm not sure\n> if that even works, considering its meaning in a glob, but certainly I\n> can imagine somebody doing so as an experiment and getting confused by\n> the result).\n\nI'm normally in a Unix environment but needed to address a potential\nissue in Windows, which indeed\ngot me confused about the results.\n\nUpon experimenting in said Unix environment with what you outlined\nabove on making revision and pathspec explicit,\nI figured that it becomes a little odd in the case where one would use\na glob meta character in a filename (why anybody would want\nthat except for academic reasons is beyond me), but it could still be\na source of bugs in tooling interacting with git:\n\nGiven a repository initialized with\n\n$ git init && echo a > a && echo '*' > '*' && git add a '*' && git\ncommit -m \"first\" && git rm a '*' && git commit -m \"second\"\n\nthere's a bit of a discrepancy when using git show to dump the\ncontents per revision. In the beginning, everything is normal for\nHEAD^ but for HEAD, it's a requirement to use disambiguation to avoid\nbelieving that '*' was an empty file. At the same time, the\ndisambiguation causes the error message change its nature, for obvious\nreasons:\n\n$ git show 'HEAD^:a'\na\n$ git show 'HEAD^:a' --\na\n$ git show 'HEAD^:*' # presumably get_oid finds the object\n*\n$ git show 'HEAD^:*' --\n*\n$ git show 'HEAD:a'\nfatal: Path 'a' does not exist in 'HEAD'\n$ git show 'HEAD:a' --\nfatal: bad revision 'HEAD:a'\n$ git show 'HEAD:*'\n$ git show 'HEAD:*' --\nfatal: bad revision 'HEAD:*'\n\nIndeed, it seems to be working as designed or at least explainably.\nI'll make sure to add '--' in my tool integration to deal with this\ncorner case.\n"},{"id":"390573","messageId":"xmqqmua8bust.fsf@gitster-ct.c.googlers.com","threadId":"52689","inReplyTo":"20200125000051.GA566074@coredump.intra.peff.net","subject":"Re: Bug or unexpected behaviour in git show <rev>:a\\b","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-01-27T18:47:46Z","receivedAt":"2020-01-27T18:47:51Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> Subject: verify_filename(): handle backslashes in \"wildcards are pathspecs\" rule\n>\n> Commit 28fcc0b71a (pathspec: avoid the need of \"--\" when wildcard is\n> used, 2015-05-02) allowed:\n>\n>   git rev-parse '*.c'\n>\n> without the double-dash. But the rule it uses to check for wildcards\n> actually looks for any glob special. This is overly liberal, as it means\n> that a pattern that doesn't actually do any wildcard matching, like\n> \"a\\b\", will be considered a pathspec.\n>\n> If you do have such a file on disk, that's presumably what you wanted.\n> But if you don't, the results are confusing: rather than say \"there's no\n> such path a\\b\", we'll quietly accept it as a pathspec which very likely\n> matches nothing (or at least not what you intended). Likewise, looking\n> for path \"a\\*b\" doesn't expand the search at all; it would only find a\n> single entry, \"a*b\".\n>\n> This commit switches the rule to trigger only when glob metacharacters\n> would expand the search, meaning both of those cases will now report an\n> error (you can still disambiguate using \"--\", of course; we're just\n> tightening the DWIM heuristic).\n\nMakes sense.  Thanks.\n"},{"id":"390748","messageId":"xmqqpnf2dk6m.fsf@gitster-ct.c.googlers.com","threadId":"52689","inReplyTo":"20200125001301.GC567109@coredump.intra.peff.net","subject":"Re: [PATCH 3/3] sha1-name: mark get_oid() error messages for translation","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-01-29T21:30:57Z","receivedAt":"2020-01-29T21:31:05Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> +\t\t\tdie(_(\"path '%s' exists, but not '%s'\\n\"\n> +\t\t\t    \"hint: Did you mean '%.*s:%s' aka '%.*s:./%s'?\"),\n> ...\n\nThe above is meant to be localizable, but...\n\n> diff --git a/t/t1506-rev-parse-diagnosis.sh b/t/t1506-rev-parse-diagnosis.sh\n> index f49fc770d6..c2b5125c12 100755\n> --- a/t/t1506-rev-parse-diagnosis.sh\n> +++ b/t/t1506-rev-parse-diagnosis.sh\n> @@ -9,8 +9,8 @@ exec </dev/null\n>  test_did_you_mean ()\n>  {\n>  \tcat >expected <<-EOF &&\n> -\tfatal: Path '$2$3' $4, but not ${5:-$SQ$3$SQ}.\n> -\tDid you mean '$1:$2$3'${2:+ aka $SQ$1:./$3$SQ}?\n> +\tfatal: path '$2$3' $4, but not ${5:-$SQ$3$SQ}\n> +\thint: Did you mean '$1:$2$3'${2:+ aka $SQ$1:./$3$SQ}?\n>  \tEOF\n>  \ttest_cmp expected error\n\n...this obviously would not allow GIT_TEST_GETTEXT_POISON test to\npass.  And ...\n\n>  test_expect_success 'incorrect file in sha1:path' '\n>  \ttest_must_fail git rev-parse HEAD:nothing.txt 2>error &&\n> -\tgrep \"fatal: Path '\"'\"'nothing.txt'\"'\"' does not exist in '\"'\"'HEAD'\"'\"'\" error &&\n> +\ttest_i18ngrep \"path .nothing.txt. does not exist in .HEAD.\" error &&\n>  \ttest_must_fail git rev-parse HEAD:index-only.txt 2>error &&\n> -\tgrep \"fatal: Path '\"'\"'index-only.txt'\"'\"' exists on disk, but not in '\"'\"'HEAD'\"'\"'.\" error &&\n> +\ttest_i18ngrep \"path .index-only.txt. exists on disk, but not in .HEAD.\" error &&\n>  \t(cd subdir &&\n>  \t test_must_fail git rev-parse HEAD:file2.txt 2>error &&\n>  \t test_did_you_mean HEAD subdir/ file2.txt exists )\n>  '\n\n... the last step executed in \"subdir\" does not seem to pass.\n"},{"id":"390749","messageId":"xmqqimkudjmt.fsf@gitster-ct.c.googlers.com","threadId":"52689","inReplyTo":"xmqqpnf2dk6m.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 3/3] sha1-name: mark get_oid() error messages for translation","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-01-29T21:42:50Z","receivedAt":"2020-01-29T21:42:55Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Junio C Hamano <gitster@pobox.com> writes:\n\n> Jeff King <peff@peff.net> writes:\n>\n>> +\t\t\tdie(_(\"path '%s' exists, but not '%s'\\n\"\n>> +\t\t\t    \"hint: Did you mean '%.*s:%s' aka '%.*s:./%s'?\"),\n>> ...\n>\n> The above is meant to be localizable, but...\n>\n>> diff --git a/t/t1506-rev-parse-diagnosis.sh b/t/t1506-rev-parse-diagnosis.sh\n>> index f49fc770d6..c2b5125c12 100755\n>> --- a/t/t1506-rev-parse-diagnosis.sh\n>> +++ b/t/t1506-rev-parse-diagnosis.sh\n>> @@ -9,8 +9,8 @@ exec </dev/null\n>>  test_did_you_mean ()\n>>  {\n>>  \tcat >expected <<-EOF &&\n>> -\tfatal: Path '$2$3' $4, but not ${5:-$SQ$3$SQ}.\n>> -\tDid you mean '$1:$2$3'${2:+ aka $SQ$1:./$3$SQ}?\n>> +\tfatal: path '$2$3' $4, but not ${5:-$SQ$3$SQ}\n>> +\thint: Did you mean '$1:$2$3'${2:+ aka $SQ$1:./$3$SQ}?\n>>  \tEOF\n>>  \ttest_cmp expected error\n>\n> ...this obviously would not allow GIT_TEST_GETTEXT_POISON test to\n> pass.  And ...\n>\n>>  test_expect_success 'incorrect file in sha1:path' '\n>>  \ttest_must_fail git rev-parse HEAD:nothing.txt 2>error &&\n>> -\tgrep \"fatal: Path '\"'\"'nothing.txt'\"'\"' does not exist in '\"'\"'HEAD'\"'\"'\" error &&\n>> +\ttest_i18ngrep \"path .nothing.txt. does not exist in .HEAD.\" error &&\n>>  \ttest_must_fail git rev-parse HEAD:index-only.txt 2>error &&\n>> -\tgrep \"fatal: Path '\"'\"'index-only.txt'\"'\"' exists on disk, but not in '\"'\"'HEAD'\"'\"'.\" error &&\n>> +\ttest_i18ngrep \"path .index-only.txt. exists on disk, but not in .HEAD.\" error &&\n>>  \t(cd subdir &&\n>>  \t test_must_fail git rev-parse HEAD:file2.txt 2>error &&\n>>  \t test_did_you_mean HEAD subdir/ file2.txt exists )\n>>  '\n>\n> ... the last step executed in \"subdir\" does not seem to pass.\n\nI'll queue this band-aid on top before making my last pushout for\nthe day.  Even with poisoned i18n/l10n, die(_(msg)) gives \"fatal:\"\nprefix at the beginning, so that is what test_did_you_mean would\nexpect to see from a passing test under GIT_TEST_GETTEXT_POISON.\n\nThe other hunk is about a test that greps in \"error\".\n\n t/t1506-rev-parse-diagnosis.sh | 17 +++++++++++------\n 1 file changed, 11 insertions(+), 6 deletions(-)\n\ndiff --git a/t/t1506-rev-parse-diagnosis.sh b/t/t1506-rev-parse-diagnosis.sh\nindex 2d26d2267e..f28a35c84e 100755\n--- a/t/t1506-rev-parse-diagnosis.sh\n+++ b/t/t1506-rev-parse-diagnosis.sh\n@@ -8,11 +8,16 @@ exec </dev/null\n \n test_did_you_mean ()\n {\n-\tcat >expected <<-EOF &&\n-\tfatal: path '$2$3' $4, but not ${5:-$SQ$3$SQ}\n-\thint: Did you mean '$1:$2$3'${2:+ aka $SQ$1:./$3$SQ}?\n-\tEOF\n-\ttest_cmp expected error\n+\tif ! test_have_prereq C_LOCALE_OUTPUT\n+\tthen\n+\t\tgrep \"^fatal: \" error\n+\telse\n+\t\tcat >expected <<-EOF &&\n+\t\tfatal: path '$2$3' $4, but not ${5:-$SQ$3$SQ}\n+\t\thint: Did you mean '$1:$2$3'${2:+ aka $SQ$1:./$3$SQ}?\n+\t\tEOF\n+\t\ttest_cmp expected error\n+\tfi\n }\n \n HASH_file=\n@@ -149,7 +154,7 @@ test_expect_success 'relative path not found' '\n \t(\n \t\tcd subdir &&\n \t\ttest_must_fail git rev-parse HEAD:./nonexistent.txt 2>error &&\n-\t\tgrep subdir/nonexistent.txt error\n+\t\ttest_i18ngrep subdir/nonexistent.txt error\n \t)\n '\n \n"},{"id":"390769","messageId":"20200130071700.GB2189233@coredump.intra.peff.net","threadId":"52689","inReplyTo":"xmqqimkudjmt.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 3/3] sha1-name: mark get_oid() error messages for translation","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-01-30T07:17:00Z","receivedAt":"2020-01-30T07:17:06Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, Jan 29, 2020 at 01:42:50PM -0800, Junio C Hamano wrote:\n\n> >> -\tfatal: Path '$2$3' $4, but not ${5:-$SQ$3$SQ}.\n> >> -\tDid you mean '$1:$2$3'${2:+ aka $SQ$1:./$3$SQ}?\n> >> +\tfatal: path '$2$3' $4, but not ${5:-$SQ$3$SQ}\n> >> +\thint: Did you mean '$1:$2$3'${2:+ aka $SQ$1:./$3$SQ}?\n> >>  \tEOF\n> >>  \ttest_cmp expected error\n> >\n> > ...this obviously would not allow GIT_TEST_GETTEXT_POISON test to\n> > pass.  And ...\n\nHrmph. I know I tested with GETTEXT_POISON, but you are obviously right\nthat this doesn't pass. I must have botched something in one of my\nrebase passes at the end.\n\nThanks for catching it, but...\n\n> I'll queue this band-aid on top before making my last pushout for\n> the day.  Even with poisoned i18n/l10n, die(_(msg)) gives \"fatal:\"\n> prefix at the beginning, so that is what test_did_you_mean would\n> expect to see from a passing test under GIT_TEST_GETTEXT_POISON.\n> \n> The other hunk is about a test that greps in \"error\".\n\nI think we can do this much more simply, by just using i18ncmp:\n\ndiff --git a/t/t1506-rev-parse-diagnosis.sh b/t/t1506-rev-parse-diagnosis.sh\nindex c2b5125c12..62085a89e3 100755\n--- a/t/t1506-rev-parse-diagnosis.sh\n+++ b/t/t1506-rev-parse-diagnosis.sh\n@@ -12,7 +12,7 @@ test_did_you_mean ()\n \tfatal: path '$2$3' $4, but not ${5:-$SQ$3$SQ}\n \thint: Did you mean '$1:$2$3'${2:+ aka $SQ$1:./$3$SQ}?\n \tEOF\n-\ttest_cmp expected error\n+\ttest_i18ncmp expected error\n }\n \n HASH_file=\n\n(we'd still need the s/grep/test_i18ngrep/ in your second hunk).\n\n-Peff\n"},{"id":"390829","messageId":"xmqqeevgdabm.fsf@gitster-ct.c.googlers.com","threadId":"52689","inReplyTo":"20200130071700.GB2189233@coredump.intra.peff.net","subject":"Re: [PATCH 3/3] sha1-name: mark get_oid() error messages for translation","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2020-01-30T19:16:13Z","receivedAt":"2020-01-30T19:16:21Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King <peff@peff.net> writes:\n\n> I think we can do this much more simply, by just using i18ncmp:\n>\n> diff --git a/t/t1506-rev-parse-diagnosis.sh b/t/t1506-rev-parse-diagnosis.sh\n> index c2b5125c12..62085a89e3 100755\n> --- a/t/t1506-rev-parse-diagnosis.sh\n> +++ b/t/t1506-rev-parse-diagnosis.sh\n> @@ -12,7 +12,7 @@ test_did_you_mean ()\n>  \tfatal: path '$2$3' $4, but not ${5:-$SQ$3$SQ}\n>  \thint: Did you mean '$1:$2$3'${2:+ aka $SQ$1:./$3$SQ}?\n>  \tEOF\n> -\ttest_cmp expected error\n> +\ttest_i18ncmp expected error\n>  }\n\nYeah, \"what die(_(...)) under POISON says matches /^fatal:/\"\nprobably is not worth checking and more importantly not something we\nwould want to rely on.\n\n>  HASH_file=\n>\n> (we'd still need the s/grep/test_i18ngrep/ in your second hunk).\n\nYes.  The final amend to 3/3 has become like this.\n\n$ git range-diff bc3f657f71..83252ba6f1 HEAD^..HEAD\n1:  83252ba6f1 ! 1:  b0418303b1 sha1-name: mark get_oid() error messages for translation\n    @@ t/t1506-rev-parse-diagnosis.sh: exec </dev/null\n     +\tfatal: path '$2$3' $4, but not ${5:-$SQ$3$SQ}\n     +\thint: Did you mean '$1:$2$3'${2:+ aka $SQ$1:./$3$SQ}?\n      \tEOF\n    - \ttest_cmp expected error\n    +-\ttest_cmp expected error\n    ++\ttest_i18ncmp expected error\n      }\n    + \n    + HASH_file=\n     @@ t/t1506-rev-parse-diagnosis.sh: test_expect_success 'correct relative file objects (6)' '\n      \n      test_expect_success 'incorrect revision id' '\n    @@ t/t1506-rev-parse-diagnosis.sh: test_expect_success 'incorrect file in :path and\n      '\n      \n      test_expect_success 'relative path not found' '\n    + \t(\n    + \t\tcd subdir &&\n    + \t\ttest_must_fail git rev-parse HEAD:./nonexistent.txt 2>error &&\n    +-\t\tgrep subdir/nonexistent.txt error\n    ++\t\ttest_i18ngrep subdir/nonexistent.txt error\n    + \t)\n    + '\n    + \n     @@ t/t1506-rev-parse-diagnosis.sh: test_expect_success 'relative path outside worktree' '\n      test_expect_success 'relative path when cwd is outside worktree' '\n      \ttest_must_fail git --git-dir=.git --work-tree=subdir rev-parse HEAD:./file.txt >output 2>error &&\n\n"},{"id":"390877","messageId":"20200131001535.GC2394563@coredump.intra.peff.net","threadId":"52689","inReplyTo":"xmqqeevgdabm.fsf@gitster-ct.c.googlers.com","subject":"Re: [PATCH 3/3] sha1-name: mark get_oid() error messages for translation","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2020-01-31T00:15:35Z","receivedAt":"2020-01-31T00:15:38Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Thu, Jan 30, 2020 at 11:16:13AM -0800, Junio C Hamano wrote:\n\n> Yes.  The final amend to 3/3 has become like this.\n> [...]\n\nPerfect, thanks for fixing my mistake.\n\n-Peff\n"}]}