From: Jens Lehmann <Jens.Lehmann@web.de>
To: Chris Packham <judge.packham@gmail.com>
Cc: git@vger.kernel.org
Subject: Re: [RFC PATCH 3/3] grep: add support for grepping in submodules
Date: Thu, 30 Sep 2010 13:24:58 +0200 [thread overview]
Message-ID: <4CA4738A.9040503@web.de> (raw)
In-Reply-To: <4CA3C569.4020309@gmail.com>
Am 30.09.2010 01:02, schrieb Chris Packham:
> I actually started with --recursive and switched to
> --recurse-submodules. One thing with this is the standard grep
> --recursive option which may cause some confusion if people expect git
> grep to behave like normal grep.
Guess how I came to use "--recurse-submodules" for recursive checkout
in the first place ;-) But the fact that clone already uses it weighs
stronger here I suppose ...
> One more thought on this that has been hanging around in my mind. I
> sometimes want to do something on all but one submodule, in this case
> with grep I'm fairly likely to want to skip a linux repository because I
> already know the thing I'm looking for is in userland. Maybe in the
> future we can make --recursive take an argument that allows us to
> specify/restrict which submodules get included in the command invocation.
Hmm, maybe adding an option to "git grep" to exclude a pathspec would
make more sense?
>> It seems you want to use strbuf_detach() here so that this argv[]
>> stays valid after the strbuf_release() at the end of this function.
> I'll look into strbuf_detatch. The tricky thing will be keeping track of
> what to free at the end of grep_submodule.
Right, but if you push the strbuf operations into one of the calling
functions you can achieve that more easily.
> Yeah this is the part I was struggling with a little. It would be easy
> to save argv before any option processing but I wondered if that would
> be frowned upon as an overhead for non-submodule usages.
Yup, but as you are only copying a pointer array the overhead is very
small. And if the code gets much easier that way (as I would expect)
that price is well paid.
next prev parent reply other threads:[~2010-09-30 11:25 UTC|newest]
Thread overview: 22+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-09-29 20:28 [RFC PATCH 0/3] grep: submodule support Chris Packham
2010-09-29 20:28 ` [RFC PATCH 1/3] add test for git grep --recursive Chris Packham
2010-09-29 20:35 ` Ævar Arnfjörð Bjarmason
2010-09-29 20:48 ` Chris Packham
2010-09-29 21:34 ` Kevin Ballard
2010-09-29 20:28 ` [RFC PATCH 2/3] grep: prepare grep for submodules Chris Packham
2010-09-30 1:10 ` Nguyen Thai Ngoc Duy
2010-09-30 18:34 ` Chris Packham
2010-10-01 14:37 ` Nguyen Thai Ngoc Duy
2010-10-01 16:26 ` Chris Packham
2010-09-29 20:28 ` [RFC PATCH 3/3] grep: add support for grepping in submodules Chris Packham
2010-09-29 22:21 ` Jens Lehmann
2010-09-29 22:59 ` Junio C Hamano
2010-09-29 23:47 ` Chris Packham
2010-09-30 11:09 ` Jens Lehmann
2010-09-30 11:28 ` Johannes Sixt
2010-09-30 15:07 ` Jens Lehmann
2010-09-29 23:02 ` Chris Packham
2010-09-30 11:24 ` Jens Lehmann [this message]
2010-09-30 16:48 ` Chris Packham
2010-09-30 18:59 ` Heiko Voigt
2010-09-30 19:48 ` Jens Lehmann
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=4CA4738A.9040503@web.de \
--to=jens.lehmann@web.de \
--cc=git@vger.kernel.org \
--cc=judge.packham@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).