On 11/15, Stefan Beller wrote: > On Fri, Nov 11, 2016 at 3:51 PM, Brandon Williams <bmwill@xxxxxxxxxx> wrote: > > + /* > > + * Add basename of parent project > > + * When performing grep on a <tree> object the filename is prefixed > > + * with the object's name: '<tree-name>:filename'. > > This comment is hard to read as it's unclear what the <angle brackets> mean. > (Are the supposed to indicate a variable? If so why is file name not marked up?) Yeah you're right, the angle brackets don't really add anything to the comment. I'll drop them. > > In order to > > + * provide uniformity of output we want to pass the name of the > > + * parent project's object name to the submodule so the submodule can > > + * prefix its output with the parent's name and not its own SHA1. > > + */ > > + if (end_of_base) > > + argv_array_pushf(&cp.args, "--parent-basename=%.*s", > > + (int) (end_of_base - gs->name), > > + gs->name); > > Do we pass this only with the tree-ish? > What if we are grepping the working tree and the file name contains a colon? Actually you're right, this would only happen if we are passing a tree-ish, which has a tree-name prefixed to the filename. I'll add that as an additional check to ensure that this handles file names with a colon correctly....though why you have a colon in a filename is beyond me :P > > +test_expect_success 'grep tree HEAD^' ' > > + cat >expect <<-\EOF && > > + HEAD^:a:foobar > > + HEAD^:b/b:bar > > + HEAD^:submodule/a:foobar > > + EOF > > + > > + git grep -e "bar" --recurse-submodules HEAD^ > actual && > > + test_cmp expect actual > > +' > > + > > +test_expect_success 'grep tree HEAD^^' ' > > + cat >expect <<-\EOF && > > + HEAD^^:a:foobar > > + HEAD^^:b/b:bar > > + EOF > > + > > + git grep -e "bar" --recurse-submodules HEAD^^ > actual && > > + test_cmp expect actual > > +' > > + > > +test_expect_success 'grep tree and pathspecs' ' > > + cat >expect <<-\EOF && > > + HEAD:submodule/a:foobar > > + HEAD:submodule/sub/a:foobar > > + EOF > > + > > + git grep -e "bar" --recurse-submodules HEAD -- submodule > actual && > > + test_cmp expect actual > > +' > > Mind to add tests for > * recursive submodules (say 2 levels), preferrably not having the > gitlink at the root each, i.e. root has a sub1 at path subs/sub1 and > sub1 has a sub2 > at path subs/sub2, such that recursing would produce a path like > HEAD:subs/sub1/subs/sub2/dir/file ? > * file names with a colon in it > * instead of just HEAD referencing trees, maybe a sha1 referenced test as well > (though it is not immediately clear what the benefit would be) > * what if the submodule doesn't have the commit referenced in the given sha1 I'll add more tests too! -- Brandon Williams