git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Jeff King <peff@peff.net>
Cc: James Pickens <jepicken@gmail.com>, git@vger.kernel.org
Subject: Re: [PATCH] grep: --full-tree
Date: Fri, 27 Nov 2009 00:18:37 -0800	[thread overview]
Message-ID: <7vd434v0rm.fsf@alter.siamese.dyndns.org> (raw)
In-Reply-To: <20091127062013.GA20844@coredump.intra.peff.net> (Jeff King's message of "Fri\, 27 Nov 2009 01\:20\:13 -0500")

Jeff King <peff@peff.net> writes:

> ... And I don't want to speak for Junio,
> but he seemed to agree that what you most want would depend on the repo
> organization (though I think he may disagree that it is important enough
> to merit the hassle of a config option).

Oh, I totally agree with you that what's convenient is different per
<project, the role I play in the project> pair.

In my day-job project, I almost always work in a directory four levels
down from the toplevel ("src/lib/u/<something>") and almost always what to
run grep from near the top ("/src" in the strawman syntax to grep from
that directory).  I have similar preference when playing with projects I
am not very familiar with---hack in a limited area somewhere deeper, but
grep a lot more widely.  So it is very tempting to say that I would want
"full-tree" configured as default in these repositories.

But the conclusion I draw from that observation is that it should be
equally easy for me to invoke both behaviours from the command line, and
not "I want to freeze which default is used for the project by setting a
configuration variable", because I know "the role I play" part changes
from time to time (note that I didn't say "changes over time"---that can
be addressed by "Then you should flip the config when the day comes").

At first sight, git.git is too shallow for "full-tree" vs "current
directory" distinction to make any meaningful difference, especially
because it is very top heavy and I almost always am at the top level
directory.  But even there, I can clearly see that I have need for easy
access to both modes.  I sometimes play a contributor who is interested in
and is very familiar with a specific area (hence I know in which files to
find strings without resorting to full-tree grep), and other times play a
reviewer role who tries to follow what an area expert, who is much more
familiar than me in some parts of the system, did in his patch.  In the
latter case, I would not know in which files to grep offhand, and would
benefit from "tree-wide" option, in order to find out what is done by that
obscure function the area expert used in his patch.

A repository-wide configuration would not help me at all, but a way to
invoke either mode from the command line that is short-and-sweet would
consistently give me the desired result without having to remember what
the default-of-the-day (or default-in-the-repo) is.

But the above is mostly about "Peff wants config, Junio thinks it won't
help him", and is not really a disagreement.  As long as we don't use the
existence of configurable defaults as an excuse for making/leaving it very
cumbersome to invoke the non-default mode from command line, "it won't
help some users" is not a reason to block it.

I suspect a per-repo configuration _might_ confuse new people (and people
who help them), and I brought it up as a potential issue myself.  That
could be a more valid reason to object to configurable default, but I
haven't formed an opinion how serious a problem it would be in real life;
I should sleep on this one and wait for others' opinions.

Regardless of the above, there are unresolved issues in the --full-tree
patch as posted.

We've been saying that even if the default were changed to grep in the
full tree, limiting to the current directory is easy with a single "." at
the end, but I do not think it is entirely true.  I often want to grep in
only "*.h" files but they are spread across the directories.  If you do

    $ git grep -e frotz -- "*.h"

it finds in paths that match the pathspec in the current directory with
today's default, but after we flip the default (either globally or with
your configuration per-repo), it won't be possible to limit the search to
header files in the current directory and under.  It will find in header
files spread over in the whole tree.

Also the --full-tree option I did at the beginning of this thread doesn't
work with the above command line example, as it only kicks in when there
is no pathspec.

Maybe these are not worth solving, and we should keep the current default
and just tell ourselves to go up to the level we want to grep from.  That
would be simple, robust and the easiest to explain.

  reply	other threads:[~2009-11-27  8:18 UTC|newest]

Thread overview: 68+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-11-24  8:56 [PATCH] grep: --full-tree Junio C Hamano
2009-11-25 13:16 ` Michael J Gruber
2009-11-25 19:32   ` Junio C Hamano
2009-11-25 14:56 ` Sverre Rabbelier
2009-11-25 19:32   ` Junio C Hamano
2009-11-25 20:19     ` Sverre Rabbelier
2009-11-25 20:23       ` Junio C Hamano
2009-11-25 20:46         ` Sverre Rabbelier
2009-11-25 23:31           ` Johannes Schindelin
2009-11-25 23:29             ` Sverre Rabbelier
2009-11-25 20:52     ` Jeff King
2009-11-25 20:39 ` Jeff King
2009-11-25 20:52   ` Junio C Hamano
2009-11-25 21:00     ` Jeff King
2009-11-25 21:33       ` Junio C Hamano
2009-11-25 21:49         ` Jeff King
2009-11-25 22:12           ` James Pickens
2009-11-25 22:20             ` Jeff King
2009-11-26 17:56               ` James Pickens
2009-11-27  6:20                 ` Jeff King
2009-11-27  8:18                   ` Junio C Hamano [this message]
2009-11-27  9:31                   ` Johannes Schindelin
2009-11-27  9:59                     ` Jeff King
2009-11-27 10:53                       ` Johannes Schindelin
2009-11-27 16:27                         ` Uri Okrent
2009-11-27 18:29                           ` Junio C Hamano
2009-11-27 18:47                             ` Uri Okrent
2009-11-27 20:53                               ` Jeff King
2009-11-29 19:50                                 ` Uri Okrent
2009-11-29 11:48                             ` Felipe Contreras
2009-11-27 18:02                         ` Jeff King
2009-11-27 20:07                           ` Johannes Schindelin
2009-11-27 21:05                             ` Jeff King
2009-11-29 10:28                               ` Johannes Schindelin
2009-11-29 18:32                                 ` Jeff King
2009-11-29 19:49                                   ` Junio C Hamano
2009-11-29 12:13                             ` Felipe Contreras
2009-11-27 18:29                       ` Junio C Hamano
2009-11-27 20:50                         ` Jeff King
2009-11-29 10:21                           ` Johannes Schindelin
2009-11-29 18:24                             ` Jeff King
2009-11-27 10:37                     ` Matthieu Moy
2009-11-27 10:56                       ` Johannes Schindelin
2009-11-25 22:26             ` Wincent Colaiuta
2009-11-26  0:00               ` Junio C Hamano
2009-11-26  0:16                 ` A Large Angry SCM
2009-11-26  0:20                   ` Junio C Hamano
2009-11-26  0:30                     ` A Large Angry SCM
2009-11-26  0:36                       ` Junio C Hamano
2009-11-26 18:14                 ` James Pickens
2009-11-25 22:19           ` Junio C Hamano
2009-11-25 22:26             ` Jeff King
2009-11-25 22:41               ` A Large Angry SCM
2009-11-25 22:53                 ` Jeff King
2009-11-25 23:07                   ` A Large Angry SCM
2009-11-25 23:22                     ` Jeff King
2009-11-29 11:38                       ` Felipe Contreras
2009-11-29 19:45                         ` Uri Okrent
2009-11-26  0:02               ` Junio C Hamano
2009-11-27  6:22                 ` Jeff King
2009-11-25 22:15         ` A Large Angry SCM
2009-11-25 22:21           ` Junio C Hamano
2009-11-25 22:31             ` A Large Angry SCM
2009-11-25 22:43               ` A Large Angry SCM
2009-11-25 23:34         ` Johannes Schindelin
2009-11-25 23:41           ` Junio C Hamano
2009-11-26  0:04             ` Johannes Schindelin
2009-11-26  0:13               ` Junio C Hamano

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=7vd434v0rm.fsf@alter.siamese.dyndns.org \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=jepicken@gmail.com \
    --cc=peff@peff.net \
    /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).