{"thread":{"id":"34768","subject":"[PATCHv2] grep: use slash for path delimiter, not colon","startedAt":"2013-08-26T14:46:12Z","lastAt":"2013-08-27T03:37:37Z","messageCount":11,"participants":["Phil Hord","Jeff King","Johannes Sixt","Junio C Hamano"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"225902","messageId":"1377528372-31206-1-git-send-email-hordp@cisco.com","threadId":"34768","inReplyTo":null,"subject":"[PATCHv2] grep: use slash for path delimiter, not colon","fromName":"Phil Hord","fromEmail":"hordp@cisco.com","sentAt":"2013-08-26T14:46:12Z","receivedAt":"2013-08-26T14:46:12Z","isPatch":false,"sender":{"key":"phil.hord@gmail.com","avatar":"https://avatars.githubusercontent.com/u/123908?v=4"},"body":"When a commit is grepped and matching filenames are printed, grep-objects\ncreates the filename by prefixing the original cmdline argument to the\nmatched path separated by a colon. Normally this forms a valid blob\nreference to the filename, like this:\n\n git grep -l foo HEAD\n HEAD:some/path/to/foo.txt\n ^\n\nBut a tree path may be given to grep instead; in this case the colon is\nnot a valid delimiter to use since it is placed inside a path.\n\n git grep -l foo HEAD:some\n HEAD:some:path/to/foo.txt\n ^\n\nThe slash path delimiter should be used instead. Fix git grep to\ndiscern the correct delimiter so it can report valid object names.\n\n git grep -l foo HEAD:some\n HEAD:some/path/to/foo.txt\n ^\n\nAlso, prevent the delimiter being added twice, as happens now in these\nexamples:\n\n git grep -l foo HEAD:\n HEAD::some/path/to/foo.txt\n ^\n git grep -l foo HEAD:some/\n HEAD:some/:path/to/foo.txt\n ^\n\nAdd a test to confirm correct path forming.\n---\nThis version is a bit more deterministic and also adds a test.\n\nIt accepts the expense of examining the path argument again to \ndetermine if it is a tree-ish + path rather than just a tree (commit).\nThe get_sha1 call occurs one extra time for each tree-ish argument,\nso it's not expensive. We avoid mucking with the object_array API this\nway, and also do not rely on the object-type to tell us anything about\nthe way the object name was spelled.\n\nThis one also adds a check to avoid duplicating an extant delimiter.\n\n builtin/grep.c | 9 ++++++++-\n t/t7810-grep.sh | 15 +++++++++++++++\n 2 files changed, 23 insertions(+), 1 deletion(-)\n\ndiff --git a/builtin/grep.c b/builtin/grep.c\nindex 03bc442..6fc418f 100644\n--- a/builtin/grep.c\n+++ b/builtin/grep.c\n@@ -480,8 +480,15 @@ static int grep_object(struct grep_opt *opt, const struct pathspec *pathspec,\n \t\tlen = name ? strlen(name) : 0;\n \t\tstrbuf_init(&base, PATH_MAX + len + 1);\n \t\tif (len) {\n+\t\t\tstruct object_context ctx;\n+\t\t\tunsigned char sha1[20];\n+\t\t\tchar delimiter = ':';\n+\t\t\tif (!get_sha1_with_context(name, 0, sha1, &ctx) &&\n+\t\t\t\tctx.path[0]!=0)\n+\t\t\t\tdelimiter='/';\n \t\t\tstrbuf_add(&base, name, len);\n-\t\t\tstrbuf_addch(&base, ':');\n+\t\t\tif (name[len-1] != delimiter)\n+\t\t\t\tstrbuf_addch(&base, delimiter);\n \t\t}\n \t\tinit_tree_desc(&tree, data, size);\n \t\thit = grep_tree(opt, pathspec, &tree, &base, base.len,\ndiff --git a/t/t7810-grep.sh b/t/t7810-grep.sh\nindex f698001..2494bfc 100755\n--- a/t/t7810-grep.sh\n+++ b/t/t7810-grep.sh\n@@ -886,6 +886,21 @@ test_expect_success 'grep -e -- -- path' '\n '\n \n cat >expected <actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success \"grep HEAD:path\" '\n+\tgit grep -n -e vvv HEAD:t/ >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+cat >expected < This version is a bit more deterministic and also adds a test.\n> \n> It accepts the expense of examining the path argument again to \n> determine if it is a tree-ish + path rather than just a tree (commit).\n> The get_sha1 call occurs one extra time for each tree-ish argument,\n> so it's not expensive.\n\nI don't like this approach in general because it lacks atomicity. IOW,\nthe thing you are looking up may change between the two get_sha1 calls.\nYou're _almost_ good here because you don't actually care what the\nsecond call returns, but only which features it _would_ have used. But\nyou may see the second call fail because the ref doesn't exist anymore,\nor points to a different tree, and you will erroneously use \":\" instead\nof \"/\".\n\nI admit this is not that likely, but I'd really rather avoid introducing\nsuch races if we can.\n\n> We avoid mucking with the object_array API this way, and also do not\n> rely on the object-type to tell us anything about the way the object\n> name was spelled.\n\nChanging the object_array API would be hard, but I don't think we need\nto do it here. Can we simply stop using object_array to pass the list,\nand instead just have a custom list?\n\nI'll see how painful that is.\n\n-Peff\n"},{"id":"225922","messageId":"20130826195331.GA21051@sigill.intra.peff.net","threadId":"34768","inReplyTo":"20130826192826.GA23598@sigill.intra.peff.net","subject":"Re: [PATCHv2] grep: use slash for path delimiter, not colon","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-08-26T19:53:31Z","receivedAt":"2013-08-26T19:53:31Z","isPatch":false,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Aug 26, 2013 at 03:28:26PM -0400, Jeff King wrote:\n\n> Changing the object_array API would be hard, but I don't think we need\n> to do it here. Can we simply stop using object_array to pass the list,\n> and instead just have a custom list?\n> \n> I'll see how painful that is.\n\nNot very, I think. Here's the series.\n\n [1/2]: grep: stop using object_array\n [2/2]: grep: use slash for path delimiter, not colon\n\n-Peff\n"},{"id":"225923","messageId":"20130826195434.GA21074@sigill.intra.peff.net","threadId":"34768","inReplyTo":"20130826195331.GA21051@sigill.intra.peff.net","subject":"[PATCH 1/2] grep: stop using object_array","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-08-26T19:54:34Z","receivedAt":"2013-08-26T19:54:34Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"We use an object_array to store the set of objects to grep\nthat we received on the command-line. There is no particular\nreason to use object_array here except that its code was\nalready written, and it contained the elements we needed\n(though we did not care about mode at all).\n\nHowever, future patches will need to remember more about the\narguments than object_array can provide. Let's use our own\ncustom struct. Thanks to the ALLOC_GROW macro, this is really\nonly a few lines longer (and we even save a few bytes of\nmemory as we do not care about the mode, and we know that we\ndo not have to copy the name strings, as they come from\nargv).\n\nSigned-off-by: Jeff King \n---\n builtin/grep.c | 31 ++++++++++++++++++++-----------\n 1 file changed, 20 insertions(+), 11 deletions(-)\n\ndiff --git a/builtin/grep.c b/builtin/grep.c\nindex d3b3b1d..ee47d49 100644\n--- a/builtin/grep.c\n+++ b/builtin/grep.c\n@@ -23,6 +23,11 @@ static char const * const grep_usage[] = {\n \tNULL\n };\n \n+struct object_to_grep {\n+\tstruct object *item;\n+\tconst char *name;\n+};\n+\n static int use_threads = 1;\n \n #ifndef NO_PTHREADS\n@@ -493,16 +498,15 @@ static int grep_objects(struct grep_opt *opt, const struct pathspec *pathspec,\n }\n \n static int grep_objects(struct grep_opt *opt, const struct pathspec *pathspec,\n-\t\t\tconst struct object_array *list)\n+\t\t\tconst struct object_to_grep *list, int nr)\n {\n \tunsigned int i;\n \tint hit = 0;\n-\tconst unsigned int nr = list->nr;\n \n \tfor (i = 0; i < nr; i++) {\n \t\tstruct object *real_obj;\n-\t\treal_obj = deref_tag(list->objects[i].item, NULL, 0);\n-\t\tif (grep_object(opt, pathspec, real_obj, list->objects[i].name)) {\n+\t\treal_obj = deref_tag(list[i].item, NULL, 0);\n+\t\tif (grep_object(opt, pathspec, real_obj, list[i].name)) {\n \t\t\thit = 1;\n \t\t\tif (opt->status_only)\n \t\t\t\tbreak;\n@@ -628,7 +632,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \tint external_grep_allowed__ignored;\n \tconst char *show_in_pager = NULL, *default_pager = \"dummy\";\n \tstruct grep_opt opt;\n-\tstruct object_array list = OBJECT_ARRAY_INIT;\n+\tstruct object_to_grep *list = NULL;\n+\tint list_nr = 0, list_alloc = 0;\n \tconst char **paths = NULL;\n \tstruct pathspec pathspec;\n \tstruct string_list path_list = STRING_LIST_INIT_NODUP;\n@@ -822,7 +827,11 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \t\t\tstruct object *object = parse_object_or_die(sha1, arg);\n \t\t\tif (!seen_dashdash)\n \t\t\t\tverify_non_filename(prefix, arg);\n-\t\t\tadd_object_array(object, arg, &list);\n+\n+\t\t\tALLOC_GROW(list, list_nr+1, list_alloc);\n+\t\t\tlist[list_nr].item = object;\n+\t\t\tlist[list_nr].name = arg;\n+\t\t\tlist_nr++;\n \t\t\tcontinue;\n \t\t}\n \t\tif (!strcmp(arg, \"--\")) {\n@@ -833,7 +842,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \t}\n \n #ifndef NO_PTHREADS\n-\tif (list.nr || cached || online_cpus() == 1)\n+\tif (list_nr || cached || online_cpus() == 1)\n \t\tuse_threads = 0;\n #else\n \tuse_threads = 0;\n@@ -861,7 +870,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \tpathspec.max_depth = opt.max_depth;\n \tpathspec.recursive = 1;\n \n-\tif (show_in_pager && (cached || list.nr))\n+\tif (show_in_pager && (cached || list_nr))\n \t\tdie(_(\"--open-files-in-pager only works on the worktree\"));\n \n \tif (show_in_pager && opt.pattern_list && !opt.pattern_list->next) {\n@@ -889,12 +898,12 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \n \tif (!use_index || untracked) {\n \t\tint use_exclude = (opt_exclude < 0) ? use_index : !!opt_exclude;\n-\t\tif (list.nr)\n+\t\tif (list_nr)\n \t\t\tdie(_(\"--no-index or --untracked cannot be used with revs.\"));\n \t\thit = grep_directory(&opt, &pathspec, use_exclude);\n \t} else if (0 <= opt_exclude) {\n \t\tdie(_(\"--[no-]exclude-standard cannot be used for tracked contents.\"));\n-\t} else if (!list.nr) {\n+\t} else if (!list_nr) {\n \t\tif (!cached)\n \t\t\tsetup_work_tree();\n \n@@ -902,7 +911,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \t} else {\n \t\tif (cached)\n \t\t\tdie(_(\"both --cached and trees are given.\"));\n-\t\thit = grep_objects(&opt, &pathspec, &list);\n+\t\thit = grep_objects(&opt, &pathspec, list, list_nr);\n \t}\n \n \tif (use_threads)\n-- \n1.8.4.2.g87d4a77\n"},{"id":"225924","messageId":"20130826195616.GB21074@sigill.intra.peff.net","threadId":"34768","inReplyTo":"20130826195331.GA21051@sigill.intra.peff.net","subject":"[PATCH 2/2] grep: use slash for path delimiter, not colon","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-08-26T19:56:16Z","receivedAt":"2013-08-26T19:56:16Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"From: Phil Hord \n\nWhen a commit is grepped and matching filenames are printed, grep-objects\ncreates the filename by prefixing the original cmdline argument to the\nmatched path separated by a colon. Normally this forms a valid blob\nreference to the filename, like this:\n\n git grep -l foo HEAD\n HEAD:some/path/to/foo.txt\n ^\n\nBut a tree path may be given to grep instead; in this case the colon is\nnot a valid delimiter to use since it is placed inside a path.\n\n git grep -l foo HEAD:some\n HEAD:some:path/to/foo.txt\n ^\n\nThe slash path delimiter should be used instead. Fix git grep to\ndiscern the correct delimiter so it can report valid object names.\n\n git grep -l foo HEAD:some\n HEAD:some/path/to/foo.txt\n ^\n\nAlso, prevent the delimiter being added twice, as happens now in these\nexamples:\n\n git grep -l foo HEAD:\n HEAD::some/path/to/foo.txt\n ^\n git grep -l foo HEAD:some/\n HEAD:some/:path/to/foo.txt\n ^\n\nAdd a test to confirm correct path forming.\n\nSigned-off-by: Jeff King \n---\nI left the author as you, since you have done all the hard work; this is\nreally just me rebasing your patch on top of mine. But note that you did\nnot signoff the original.\n\n builtin/grep.c | 13 +++++++++----\n t/t7810-grep.sh | 15 +++++++++++++++\n 2 files changed, 24 insertions(+), 4 deletions(-)\n\ndiff --git a/builtin/grep.c b/builtin/grep.c\nindex ee47d49..2df7986 100644\n--- a/builtin/grep.c\n+++ b/builtin/grep.c\n@@ -26,6 +26,7 @@ struct object_to_grep {\n struct object_to_grep {\n \tstruct object *item;\n \tconst char *name;\n+\tunsigned has_path:1;\n };\n \n static int use_threads = 1;\n@@ -462,7 +463,7 @@ static int grep_object(struct grep_opt *opt, const struct pathspec *pathspec,\n }\n \n static int grep_object(struct grep_opt *opt, const struct pathspec *pathspec,\n-\t\t struct object *obj, const char *name)\n+\t\t struct object *obj, const char *name, char delimiter)\n {\n \tif (obj->type == OBJ_BLOB)\n \t\treturn grep_sha1(opt, obj->sha1, name, 0, NULL);\n@@ -485,7 +486,8 @@ static int grep_object(struct grep_opt *opt, const struct pathspec *pathspec,\n \t\tstrbuf_init(&base, PATH_MAX + len + 1);\n \t\tif (len) {\n \t\t\tstrbuf_add(&base, name, len);\n-\t\t\tstrbuf_addch(&base, ':');\n+\t\t\tif (name[len-1] != delimiter)\n+\t\t\t\tstrbuf_addch(&base, delimiter);\n \t\t}\n \t\tinit_tree_desc(&tree, data, size);\n \t\thit = grep_tree(opt, pathspec, &tree, &base, base.len,\n@@ -506,7 +508,8 @@ static int grep_objects(struct grep_opt *opt, const struct pathspec *pathspec,\n \tfor (i = 0; i < nr; i++) {\n \t\tstruct object *real_obj;\n \t\treal_obj = deref_tag(list[i].item, NULL, 0);\n-\t\tif (grep_object(opt, pathspec, real_obj, list[i].name)) {\n+\t\tif (grep_object(opt, pathspec, real_obj, list[i].name,\n+\t\t\t\tlist[i].has_path ? '/' : ':')) {\n \t\t\thit = 1;\n \t\t\tif (opt->status_only)\n \t\t\t\tbreak;\n@@ -822,8 +825,9 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \tfor (i = 0; i < argc; i++) {\n \t\tconst char *arg = argv[i];\n \t\tunsigned char sha1[20];\n+\t\tstruct object_context oc;\n \t\t/* Is it a rev? */\n-\t\tif (!get_sha1(arg, sha1)) {\n+\t\tif (!get_sha1_with_context(arg, 0, sha1, &oc)) {\n \t\t\tstruct object *object = parse_object_or_die(sha1, arg);\n \t\t\tif (!seen_dashdash)\n \t\t\t\tverify_non_filename(prefix, arg);\n@@ -831,6 +835,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)\n \t\t\tALLOC_GROW(list, list_nr+1, list_alloc);\n \t\t\tlist[list_nr].item = object;\n \t\t\tlist[list_nr].name = arg;\n+\t\t\tlist[list_nr].has_path = !!oc.path[0];\n \t\t\tlist_nr++;\n \t\t\tcontinue;\n \t\t}\ndiff --git a/t/t7810-grep.sh b/t/t7810-grep.sh\nindex f698001..2494bfc 100755\n--- a/t/t7810-grep.sh\n+++ b/t/t7810-grep.sh\n@@ -886,6 +886,21 @@ cat >expected <expected <actual &&\n+\ttest_cmp expected actual\n+'\n+\n+test_expect_success \"grep HEAD:path\" '\n+\tgit grep -n -e vvv HEAD:t/ >actual &&\n+\ttest_cmp expected actual\n+'\n+\n+cat >expected < Also, prevent the delimiter being added twice, as happens now in these\n> examples:\n> \n> git grep -l foo HEAD:\n> HEAD::some/path/to/foo.txt\n> ^\n\nWhich one of these two does it print then?\n\n HEAD:/some/path/to/foo.txt\n HEAD:some/path/to/foo.txt\n\n-- Hannes\n"},{"id":"225933","messageId":"CABURp0pELT70EtnsJvWYgOTK5aoGUyB8ONhrNBtAOyLHODf6gw@mail.gmail.com","threadId":"34768","inReplyTo":"521BB6DA.5050807@kdbg.org","subject":"Re: [PATCH 2/2] grep: use slash for path delimiter, not colon","fromName":"Phil Hord","fromEmail":"phil.hord@gmail.com","sentAt":"2013-08-26T20:52:05Z","receivedAt":"2013-08-26T20:52:05Z","isPatch":true,"sender":{"key":"phil.hord@gmail.com","avatar":"https://avatars.githubusercontent.com/u/123908?v=4"},"body":"On Mon, Aug 26, 2013 at 4:13 PM, Johannes Sixt wrote:\n> Am 26.08.2013 21:56, schrieb Jeff King:\n>> Also, prevent the delimiter being added twice, as happens now in these\n>> examples:\n>>\n>> git grep -l foo HEAD:\n>> HEAD::some/path/to/foo.txt\n>> ^\n>\n> Which one of these two does it print then?\n>\n> HEAD:/some/path/to/foo.txt\n> HEAD:some/path/to/foo.txt\n\n\nWith my patch it prints the latter.\n\nThis is because get_sha1_with_context(\"HEAD:\"...) returns an empty\n'path' string. The code decides to use ':' as the delimiter in that\ncase, but it sees there already is one at the end of \"HEAD:\".\n\nPhil\n"},{"id":"225932","messageId":"20130826205215.GB23598@sigill.intra.peff.net","threadId":"34768","inReplyTo":"521BB6DA.5050807@kdbg.org","subject":"Re: [PATCH 2/2] grep: use slash for path delimiter, not colon","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-08-26T20:52:15Z","receivedAt":"2013-08-26T20:52:15Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Aug 26, 2013 at 10:13:14PM +0200, Johannes Sixt wrote:\n\n> Am 26.08.2013 21:56, schrieb Jeff King:\n> > Also, prevent the delimiter being added twice, as happens now in these\n> > examples:\n> > \n> > git grep -l foo HEAD:\n> > HEAD::some/path/to/foo.txt\n> > ^\n> \n> Which one of these two does it print then?\n> \n> HEAD:/some/path/to/foo.txt\n> HEAD:some/path/to/foo.txt\n\nIt should (and does) print the latter.\n\nBut I do note that our pathspec handling for subdirectories seems buggy.\nIf you do:\n\n $ cd Documentation\n $ git grep -l foo | head -1\n RelNotes/1.5.1.5.txt\n\nthat's fine; we limit to the current directory. But then if you do:\n\n $ git grep -l foo HEAD | head -1\n HEAD:RelNotes/1.5.1.5.txt\n\nwe still limit to the current directory, but the output does note note\nthis (it should be \"HEAD:./RelNotes/1.5.1.5.txt\"). I think this bug is\northogonal to Phil's patch, though.\n\n-Peff\n"},{"id":"225935","messageId":"CABURp0pPzRH5JdPEooyZw0iccD891E1i_CbTqDfVJnx+drfQEw@mail.gmail.com","threadId":"34768","inReplyTo":"20130826205215.GB23598@sigill.intra.peff.net","subject":"Re: [PATCH 2/2] grep: use slash for path delimiter, not colon","fromName":"Phil Hord","fromEmail":"phil.hord@gmail.com","sentAt":"2013-08-26T21:03:04Z","receivedAt":"2013-08-26T21:03:04Z","isPatch":true,"sender":{"key":"phil.hord@gmail.com","avatar":"https://avatars.githubusercontent.com/u/123908?v=4"},"body":"On Mon, Aug 26, 2013 at 4:52 PM, Jeff King wrote:\n> On Mon, Aug 26, 2013 at 10:13:14PM +0200, Johannes Sixt wrote:\n>\n>> Am 26.08.2013 21:56, schrieb Jeff King:\n>> > Also, prevent the delimiter being added twice, as happens now in these\n>> > examples:\n>> >\n>> > git grep -l foo HEAD:\n>> > HEAD::some/path/to/foo.txt\n>> > ^\n>>\n>> Which one of these two does it print then?\n>>\n>> HEAD:/some/path/to/foo.txt\n>> HEAD:some/path/to/foo.txt\n>\n> It should (and does) print the latter.\n>\n> But I do note that our pathspec handling for subdirectories seems buggy.\n> If you do:\n>\n> $ cd Documentation\n> $ git grep -l foo | head -1\n> RelNotes/1.5.1.5.txt\n>\n> that's fine; we limit to the current directory. But then if you do:\n>\n> $ git grep -l foo HEAD | head -1\n> HEAD:RelNotes/1.5.1.5.txt\n>\n> we still limit to the current directory, but the output does not note\n> this (it should be \"HEAD:./RelNotes/1.5.1.5.txt\"). I think this bug is\n> orthogonal to Phil's patch, though.\n\nMaybe not. My path completes the assumption that the L:R value\nreturned by grep is an object ref; but Junio still thought it wasn't.\nI think this is another case where his view was correct.\n\nThere's more bad news on this front.\n\n $ cd Documentation\n $ git grep -l foo HEAD .. | head -1\n HEAD:../.gitignore\n\nThat's not a valid ref, either (though maybe it could be).\n\nPhil\n"},{"id":"225937","messageId":"20130826211320.GA26417@sigill.intra.peff.net","threadId":"34768","inReplyTo":"CABURp0pPzRH5JdPEooyZw0iccD891E1i_CbTqDfVJnx+drfQEw@mail.gmail.com","subject":"Re: [PATCH 2/2] grep: use slash for path delimiter, not colon","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2013-08-26T21:13:20Z","receivedAt":"2013-08-26T21:13:20Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Aug 26, 2013 at 05:03:04PM -0400, Phil Hord wrote:\n\n> > $ git grep -l foo HEAD | head -1\n> > HEAD:RelNotes/1.5.1.5.txt\n> >\n> > we still limit to the current directory, but the output does not note\n> > this (it should be \"HEAD:./RelNotes/1.5.1.5.txt\"). I think this bug is\n> > orthogonal to Phil's patch, though.\n> \n> Maybe not. My path completes the assumption that the L:R value\n> returned by grep is an object ref; but Junio still thought it wasn't.\n> I think this is another case where his view was correct.\n\nI certainly assumed it was, because it is in most cases it is. And something\nlike \"HEAD:RelNotes/1.5.1.5.txt\" certainly _looks_ like one, and is\ngenerated by the current git. And what is the point of coming up with a\nfile listing if the names you return do not actually exist?\n\n> There's more bad news on this front.\n> \n> $ cd Documentation\n> $ git grep -l foo HEAD .. | head -1\n> HEAD:../.gitignore\n> \n> That's not a valid ref, either (though maybe it could be).\n\nYes, though we seem to normalize paths already. So the other entries\nfrom that command are (in git.git):\n\n HEAD:../.mailmap\n HEAD:RelNotes/1.5.1.5.txt\n\nSo we could either:\n\n 1. Prepend the current path before normalizing to yield:\n\n HEAD:.mailmap\n HEAD:Documentation/RelNotes/1.5.1.5.txt\n\n 2. Teach the get_sha1 path parser about \"..\", and prepend \"./\" when we\n are in a prefixed subdir.\n\n HEAD:./../.mailmap\n HEAD:./RelNotes/1.5.1.5.txt\n\n-Peff\n"},{"id":"225948","messageId":"xmqq7gf7iz26.fsf@gitster.dls.corp.google.com","threadId":"34768","inReplyTo":"20130826195331.GA21051@sigill.intra.peff.net","subject":"Re: [PATCHv2] grep: use slash for path delimiter, not colon","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2013-08-27T03:37:37Z","receivedAt":"2013-08-27T03:37:37Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Jeff King writes:\n\n> On Mon, Aug 26, 2013 at 03:28:26PM -0400, Jeff King wrote:\n>\n>> Changing the object_array API would be hard, but I don't think we need\n>> to do it here. Can we simply stop using object_array to pass the list,\n>> and instead just have a custom list?\n>> \n>> I'll see how painful that is.\n>\n> Not very, I think. Here's the series.\n>\n> [1/2]: grep: stop using object_array\n> [2/2]: grep: use slash for path delimiter, not colon\n\nI agree that if we were to do this, these patches show a reasonable\napproach to do so.\n\nI however am not yet convinced if its output is necessarily better\nX-<.\n"}]}