Re: [RFC PATCH 3/3] grep: add support for grepping in submodules

From: Chris Packham <judge.packham@gmail.com>
Date: Sep 29, 2010, 23:47 UTC
Message-ID: <4CA3D01B.6060600@gmail.com>
In-Reply-To: <7v4od8ma0j.fsf@alter.siamese.dyndns.org>

On 29/09/10 15:59, Junio C Hamano wrote:

Show 7 quoted lines

> Jens Lehmann <Jens.Lehmann@web.de> writes:
> 
>> Hm, at a quick glance it might be much easier to copy argc & argv
>> in cmd_grep() before parse_options() starts manipulating it.
> 
> Yes, I think that is a much saner direction to go.  Otherwise, you would
> need to unparse grep boolean expressions as well.

I've got some of that working in my quest to not use saved_argv[0]. If we follow some of your points below about handling ref names then rebuilding the args actually starts to make life easier. I guess the same is true for just making these manipulations on the grep_opts. The only thing that is really clear is that saving the original argv is not going to help us.

Show 33 quoted lines

> A few more things to think about.
> 
> 1. What does this mean:
> 
>     $ git grep --recursive -e frotz master next
> 
> It recurses into the submodule commits recorded in 'master' and 'next'
> commits in the superproject, right?
> 
> How do the lines output from the above look like?  From the superproject,
> we will get lines like these:
> 
>     master:t/README:  test_description='xxx test (option --frotz)
>     master:t/README:  and tries to run git-ls-files with option --frotz.'
> 
> What if we have a submodule at git-gui in the superproject, and its README
> has string frotz in it?  Should we label the submodule commit we find in
> 'master' of superproject as 'master' as well, even if it is not at the tip
> of 'master' branch of the submodule?  Or do we get abbreviated hexadecimal
> SHA-1 name?  IOW, would we see:
> 
>     master:git-gui/README: git-gui also knows frotz
> 
> or
> 
>     deadbeef:git-gui/README: git-gui also knows frotz
> 
> where "deadbeaf...." is what "git rev-parse master:git-gui" would give us
> in the superproject?
> 
> I tend to think the former is preferable, but then most likely you would
> need to pass not just submodule-prefix but the original ref name
> (i.e. 'master') you started from down to the recursive one.

Passing the ref name is doable. There is a little potential for confusion between who's "master" that actually is (the same confusion is in theory possible with an abbreviated SHA-1). Maybe we should color the submodule ref's differently

Show 21 quoted lines

> 2. Now how would this work with pathspecs?
> 
>     $ git grep --recursive -e frotz -- dir/
> 
> This should look for the string in the named directory in the superproject
> and if there are submodules in that directory, recurse into them as well,
> right?
> 
> What pathspec, if any, will be in effect when we recurse into the
> submodule at dir/sub?  Limiting to dir/ feels wrong, no?
> 
> 3. Corollary.
> 
> Is it reasonable to expect that we will look into all shell scripts, both
> in the superproject and in submodules, with this:
> 
>     $ git grep --recursive -e frotz -- '*.sh'
> 
> Oops?  What happened to the "we restrict the recursion using pathspec, and
> we do not pass down the pathspec" that was suggested in 2.?
>

This is a bit of a grey area, I'm not sure what is the sensible thing to do.

Maybe we could pop a directory level per recursion e.g.
  user enters 'dir/sub/subsub/*.sh'
  first level recursion is passed 'sub/subsub/*.sh'
  second level recursion is passed 'subsub/*.sh'
  subsequent levels of recursion are passed '*.sh'

But that's not quite what the user thought they asked for (i.e. they will end up with dir/sub/subsub/subsubsub/file.sh).

Or we could alter the behaviour based on whether their original pathspec had an explicit trailing /.