[PATCH] grep: --full-tree

[PATCH] grep: --full-tree

[Junio C Hamano]

While working inside a deep subdirectory, it sometimes is necessary to
find a string you see in a file you are working on from the files in the
entire project.  This is especially true when you are dipping your toe
into an unfamiliar project.

By default, "git grep" limits its search space to the current directory
and below (i.e. as if "-r ." is specified), and it is rather cumbersome to
repeat ../ as many times as necessary.  This new option tells "git grep"
not to limit the search space to the current directory.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 * In http://article.gmane.org/gmane.comp.version-control.git/111717, I
   once argued in the opposite way, but I think it is Ok to aim for making
   the default --full-tree in the longer run (cf. $gmane/127885).  This is
   the first step in that direction.

   I am not sure if there can be a sane way to flip the default without
   hurting existing scripts and users.  Backward compatibility always is
   a pain.

 builtin-grep.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/builtin-grep.c b/builtin-grep.c
index 761799d..5787f35 100644
--- a/builtin-grep.c
+++ b/builtin-grep.c
@@ -693,6 +693,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 {
 	int hit = 0;
 	int cached = 0;
+	int full_tree = 0;
 	int external_grep_allowed = 1;
 	int seen_dashdash = 0;
 	struct grep_opt opt;
@@ -732,6 +733,8 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 		OPT_BIT('H', NULL, &opt.pathname, "show filenames", 1),
 		OPT_NEGBIT(0, "full-name", &opt.relative,
 			"show filenames relative to top directory", 1),
+		OPT_BIT(0, "full-tree", &full_tree,
+			"search from the top of the tree", 1),
 		OPT_BOOLEAN('l', "files-with-matches", &opt.name_only,
 			"show only filenames instead of matching lines"),
 		OPT_BOOLEAN(0, "name-only", &opt.name_only,
@@ -862,7 +865,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix)
 
 	if (i < argc)
 		paths = get_pathspec(prefix, argv + i);
-	else if (prefix) {
+	else if (prefix && !full_tree) {
 		paths = xcalloc(2, sizeof(const char *));
 		paths[0] = prefix;
 		paths[1] = NULL;
-- 
1.6.6.rc0.47.g1fdffa.dirty

Re: [PATCH] grep: --full-tree

[Michael J Gruber]

Junio C Hamano venit, vidit, dixit 24.11.2009 09:56:
> While working inside a deep subdirectory, it sometimes is necessary to
> find a string you see in a file you are working on from the files in the
> entire project.  This is especially true when you are dipping your toe
> into an unfamiliar project.
> 
> By default, "git grep" limits its search space to the current directory
> and below (i.e. as if "-r ." is specified), and it is rather cumbersome to
> repeat ../ as many times as necessary.  This new option tells "git grep"
> not to limit the search space to the current directory.
> 
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
> 
>  * In http://article.gmane.org/gmane.comp.version-control.git/111717, I
>    once argued in the opposite way, but I think it is Ok to aim for making
>    the default --full-tree in the longer run (cf. $gmane/127885).  This is
>    the first step in that direction.
> 
>    I am not sure if there can be a sane way to flip the default without
>    hurting existing scripts and users.  Backward compatibility always is
>    a pain.

On a related note, I had planned for a while now to go through the
commands and check for inconsistencies w.r.t. to subdir default. For
example, ls-files behaves like grep, whereas status is different. We
already had discussions about the commit:path notation from a subdir. (I
don't remember the outcome.) Of course, defaulting status differently
could be dangerous. Having --full-tree as default for all commands and
requiring an explicit "." sounds safer for all commands and not overly
inconvenient. (I remember once wondering where my committed files are,
looking at git ls-files output from a subdir.)

I think we should make this behavior as uniform across commands as
possible. Do we have a time frame for 1.7.0 within which one should
achieve such incompatible changes?

Michael

Re: [PATCH] grep: --full-tree

[Sverre Rabbelier]

Heya,

On Tue, Nov 24, 2009 at 09:56, Junio C Hamano <gitster@pobox.com> wrote:
>   I am not sure if there can be a sane way to flip the default without
>   hurting existing scripts and users.  Backward compatibility always is
>   a pain.

I regularly rely on this behavior in my usage of git grep. For
example, the Melange project has this layout:
-- app
-- app/soc
-- app/django
-- app/... etc
-- scripts
-- tests
-- thirdparty

I almost always want only results from "app/soc", so when I run git
grep I do so from within "app/soc" to make sure I don't get false
positives from the many external sources we have.

Just chiming in for the "want to keep the current behavior" camp :).

PS: I don't mind having to set a config variable to keep the current
behavior though.

-- 
Cheers,

Sverre Rabbelier

Re: [PATCH] grep: --full-tree

[Junio C Hamano]

Michael J Gruber <git@drmicha.warpmail.net> writes:

> Junio C Hamano venit, vidit, dixit 24.11.2009 09:56:
>> While working inside a deep subdirectory, it sometimes is necessary to
>> find a string you see in a file you are working on from the files in the
>> entire project.  This is especially true when you are dipping your toe
>> into an unfamiliar project.
>> 
>> By default, "git grep" limits its search space to the current directory
>> and below (i.e. as if "-r ." is specified), and it is rather cumbersome to
>> repeat ../ as many times as necessary.  This new option tells "git grep"
>> not to limit the search space to the current directory.
>> 
>> Signed-off-by: Junio C Hamano <gitster@pobox.com>
>> ---
>> 
>>  * In http://article.gmane.org/gmane.comp.version-control.git/111717, I
>>    once argued in the opposite way, but I think it is Ok to aim for making
>>    the default --full-tree in the longer run (cf. $gmane/127885).  This is
>>    the first step in that direction.
>> 
>>    I am not sure if there can be a sane way to flip the default without
>>    hurting existing scripts and users.  Backward compatibility always is
>>    a pain.
>
> On a related note, I had planned for a while now to go through the
> commands and check for inconsistencies w.r.t. to subdir default. For
> example, ls-files behaves like grep, whereas status is different. We
> already had discussions about the commit:path notation from a subdir. (I
> don't remember the outcome.) Of course, defaulting status differently
> could be dangerous. Having --full-tree as default for all commands and
> requiring an explicit "." sounds safer for all commands and not overly
> inconvenient. (I remember once wondering where my committed files are,
> looking at git ls-files output from a subdir.)
>
> I think we should make this behavior as uniform across commands as
> possible. Do we have a time frame for 1.7.0 within which one should
> achieve such incompatible changes?

I do not think there is such a consensus for a blanket change like that.

If you are starting a discussion to build one for a particular change (not
necessarily the one you mentioned above) now, you are way too late for
1.7.0.  The changes scheduled for 1.7.0 were glitches we have known for
quite some time, and more importantly had a concensus on _how_ they should
be handled long before 1.6.3 (May 6, 2009), and the most importantly, the
steps in the transition plan since then have already been executing.

 - The plan for "git push" changes were already announced in 1.6.3, and
   the first step of transition was implemented there.

 - We already had consensus for changing the default "send-email"
   threading behaviour before 1.6.2 and it was scheduled to happen in
   1.6.3 but has been deferred until now.

 - For a long time, it has been known that it is confusing and unexpected
   to users that "git status" is a synonym for "git commit --dry-run".
   The plan to make "git status" different from "git commit --dry-run" has
   been done in mid August this year.

 - For a long time, "git diff" considered -b/-w options are only for
   controlling generation of patch text, and these options didn't affect
   the exit status (when run with --exit-code) nor suppress the patch
   header lines (i.e. "diff --git").  This could be argued as a bug (the
   same way as "some commands are relative to cwd by default and others
   are relative to the whole tree" can be), but it doesn't mean we can
   blame user's scripts for relying on the bug and change the semantics
   all of sudden.  We had been cooking the change since May 2009 and
   announcements were in all issues of "What's cooking" since Aug 2009 for
   this change.

Also, please do not confuse 1.7.0 with a license for "I do not like this
and that, screw backward compatibility, and change things as if we were
building git from scratch without any existing users".  We need a solid
transition plan to ease the pain for existing scripts and users.

As to ls-files, I haven't seen any good proposal of a smooth transition
plan (like what we laid out for a few semantic changes for "git push" for
1.7.0), if we were to eventually change it, and I personally do not think
there can be a smooth transition for that particular command.  It is used
as a very low level building block for people's scripts, and I don't think
of a way to change its fundamental behaviour without causing people a lot
of extra work.  I doubt you can easily build a concensus that the benefit
of "consistency" is worth it for such a change.

    Side note.  What we _could_ do is to make ls-files less (much less)
    necessary at the UI level for you to _type_ from the command line.
    Enumerate in what situations you used the command, think about the
    reason for each of occasions why you used it (e.g. "after a conflicted
    merge I wanted to find out which paths are still unresolved and
    'ls-files -u' was the most convenient way"), and eliminate the reason
    (e.g. "add a new (option to 'merge'|command) that reports the needed
    information in much more readable way than 'ls-files -u' does).

The same applies to "$treeish:$path" syntax.

It may be convenient if there were to specify "I want to name the path in
HEAD~47 that corresponds to this file in the directory I am currently in."
But that does not necessarily mean we should change the semantics and
break existing users.  One way to satisfy the wish without breaking
existing users would be to start accepting "$treeish:./$relative".

Re: [PATCH] grep: --full-tree

[Junio C Hamano]

Sverre Rabbelier <srabbelier@gmail.com> writes:

> I almost always want only results from "app/soc", so when I run git
> grep I do so from within "app/soc" to make sure I don't get false
> positives from the many external sources we have.

The standard answer given by others has been "you can always say '.' at
the end; having to remember/count number of ../ necessary is much much
more inconvenient".

> PS: I don't mind having to set a config variable to keep the current
> behavior though.

I've thought about it for five seconds before sending my patch, but
discarded it, because I do not see it as a good transition plan.

If it were something like "git-push", that is a purely Porcelain for
causing _effect_ to outside world, the customizable behaviour of the
command depending on which repository it is run is excusable and may even
be beneficial.

But if a command like "grep" that "does one small thing and do it well"
changes its behaviour drastically depending on a config variable or an
environment variable, it won't be a command that you can rely upon any
more in your scripts and hooks.  It's the same insanity as GREP_OPTIONS
environment variable.

So this change, if we were to do it, unfortunately has to be "we do it
once and for everybody" a flag day event, I think.  That is what I am not
enthused about this patch.

Re: [PATCH] grep: --full-tree

[Sverre Rabbelier]

Heya,

On Wed, Nov 25, 2009 at 20:32, Junio C Hamano <gitster@pobox.com> wrote:
> The standard answer given by others has been "you can always say '.' at
> the end; having to remember/count number of ../ necessary is much much
> more inconvenient".

A commandline flag to keep the old behavior then perhaps? "git config
alias.gr 'grep --no-full-tree'" is not that hard to write either.

-- 
Cheers,

Sverre Rabbelier

Re: [PATCH] grep: --full-tree

[Junio C Hamano]

Sverre Rabbelier <srabbelier@gmail.com> writes:

> On Wed, Nov 25, 2009 at 20:32, Junio C Hamano <gitster@pobox.com> wrote:
>> The standard answer given by others has been "you can always say '.' at
>> the end; having to remember/count number of ../ necessary is much much
>> more inconvenient".
>
> A commandline flag to keep the old behavior then perhaps? "git config
> alias.gr 'grep --no-full-tree'" is not that hard to write either.

But then you can alias "gr 'grep --full-tree'" with the same ease and
there is no reason to change the default.

Re: [PATCH] grep: --full-tree

[Jeff King]

On Tue, Nov 24, 2009 at 12:56:32AM -0800, Junio C Hamano wrote:

>  * In http://article.gmane.org/gmane.comp.version-control.git/111717, I
>    once argued in the opposite way, but I think it is Ok to aim for making
>    the default --full-tree in the longer run (cf. $gmane/127885).  This is
>    the first step in that direction.

Ironically, I argued for --full-tree behavior in the same thread, but
have since softened my view. What I have come to realize is that (for
me, anyway) it is very dependent on the organization of the project you
are working on.

For git.git, and most of my small-ish other projects, I want "git grep"
to search the full tree. But recently, I have been working on a
large-ish project imported from svn where the parts of interest to me
are rooted two directories deep (i.e., I am working in
"linux/subproject/", and I don't care at all what's happening in
"windows/otherproject"). I don't want grep hits from the other area to
clutter my output, and I especially don't want to waste time hitting the
disk for those pages, which are an order of magnitude larger than the
working set of files that are actually of interest to me.

On top of that, I think there are two ways within a logical subproject
to use subdirectories. In git.git, I tend to actually chdir into
Documentation/ or t/, because they have their own Makefiles. But for a
project that organizes its code into a bunch of module subdirectories
all driven by a top-level non-recursive Makefile, I tend to stay at the
root and actually do "vi module/foo.c; make".

So what that tells me is:

  1. It is not necessarily the _developer_, but a combination of
     developer and project that decides which behavior (of --full-tree
     and the current behavior) is more useful. Which to me really points
     to the utility of a config option.

  2. It would be useful to have a "partial-tree" middle ground. In other
     words, if I am in "linux/subproject/t", I would find it most
     useful if "git grep" searched all of "linux/subproject".
     Implementing that would become much more complex, though. Probably
     the user would specify a list of rooted subprojects, and we would
     prefix-match our path to find which one we were in, and then
     do a full-tree grep on that subtree.

     And yes, this is somewhat an argument in favor of splitting the
     project into submodules. But I'd really rather not do that. They
     introduce significant complexity, and the rest of git is so _good_
     at ignoring uninteresting parts.

-Peff

Re: [PATCH] grep: --full-tree

[Sverre Rabbelier]

Heya,

On Wed, Nov 25, 2009 at 21:23, Junio C Hamano <gitster@pobox.com> wrote:
> But then you can alias "gr 'grep --full-tree'" with the same ease and
> there is no reason to change the default.

I agree, but then again I'm somewhat biased, as I want the current behavior :P.

-- 
Cheers,

Sverre Rabbelier

Re: [PATCH] grep: --full-tree

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Tue, Nov 24, 2009 at 12:56:32AM -0800, Junio C Hamano wrote:
>
>>  * In http://article.gmane.org/gmane.comp.version-control.git/111717, I
>>    once argued in the opposite way, but I think it is Ok to aim for making
>>    the default --full-tree in the longer run (cf. $gmane/127885).  This is
>>    the first step in that direction.
>
> Ironically, I argued for --full-tree behavior in the same thread, but
> have since softened my view. What I have come to realize is that (for
> me, anyway) it is very dependent on the organization of the project you
> are working on.

I was against --full-tree but wished for it when I dipped my toe into
somebody else's project and my itch lived in a directory a few levels
deep, while the infrastructure the files in the directory uses were spread
across global include directory and platform implementations [*1*].

And I agree that the preferred behaviour depends largely on both the
project and what kind of change you are currently scratching.

So I think the posted patch alone without changing anything else would be
the approach to give the most benefit with the least impact to existing
users, at least for now.

>   2. It would be useful to have a "partial-tree" middle ground. In other
>      words, if I am in "linux/subproject/t", I would find it most
>      useful if "git grep" searched all of "linux/subproject".

"git grep -e frotz .." will work in your "from linux/subproject/t look for
everywhere in linux/subproject", but if "/t" part were much longer and
variable (iow you need to chdir around inside linux/subproject to scratch
your itch) compared to "linux/subproject" part that is much shorter and
static (to your work), it may make sense to give us a mode to specify
pathspec from the top of the tree.

    $ cd linux/subproject
    $ cd foo
    $ cd bar
    $ cd baz
    $ git grep --absolute-pathspec -e frotz -- linux/subproject

As "git grep" never takes absolute paths, we _might_ be able to also do

    $ git grep -e frotz -- /linux/subproject

to achieve the same.


[Footnote]

*1* It's rockbox, if you need to know.

Re: [PATCH] grep: --full-tree

[Jeff King]

On Wed, Nov 25, 2009 at 11:32:44AM -0800, Junio C Hamano wrote:

> But if a command like "grep" that "does one small thing and do it well"
> changes its behaviour drastically depending on a config variable or an
> environment variable, it won't be a command that you can rely upon any
> more in your scripts and hooks.  It's the same insanity as GREP_OPTIONS
> environment variable.

I know this is the attitude we have taken in the past, and I am worried
it is part of what hurts the usability of git. Just consider for a
moment: git grows some feature with a default behavior X. Time passes.
Some people like behavior Y instead. How can we help the people who like
Y?

  1. Declare Y better than X, and default to it. This hurts people who
     like X. It also hurts scripts built around X.

  2. Add a config option to switch the behavior to Y. This hurts people
     or scripts unexpectedly using somebody's configuration with Y.

  3. Add a --Y command line option. Now the Y people have to remember to
     use that option. Every single time they invoke the command.

  4. Tell them to alias "git foo-y" to "git foo --Y". IMHO, this is
     completely unscalable. They can't just call it "foo", so they have
     to remember to invoke "foo-y" each time. And when they forget,
     instead of getting an error, they get the X behavior. Furthermore,
     as time goes on, they basically develop a vocabulary of git
     commands that is totally unlike anybody else's, making their
     scripts and git knowledge unportable to other people's setups (sort
     of like in (2) above).

So as a Y user, what is the impression of git that I am left with? It
doesn't do what I want unless I remember an option every time, or create
an arcane pseudo-porcelain interface through my set of aliases. Patches
to fix the situation are blocked by compatibility issues. Y users remain
frustrated indefinitely.

I know that (1) and (2) have their problems. But I think by not giving a
little on those compatibility issues, we end up with an equally bad or
worse outcome. In other words, I think in this case that (2) may be the
lesser of many evils.

-Peff

Re: [PATCH] grep: --full-tree

[Jeff King]

On Wed, Nov 25, 2009 at 12:52:11PM -0800, Junio C Hamano wrote:

> So I think the posted patch alone without changing anything else would be
> the approach to give the most benefit with the least impact to existing
> users, at least for now.

Yes, I meant to say in my original message but forgot to: I think
--full-tree is an important first step, no matter what happens next. It
gives people a way to do what they want without typing the right number
of ".."s, and it opens up --no-full-tree if the default changes later.

But I do worry about it being a command-line option. You are asking the
user to remember to type --full-tree every time. I can't count the
number of times I have been in a subdirectory and done "git grep foo",
spent some time analyzing and doing something with the results, only for
my palm to hit my forehead when I realize that I was missing half of the
results I wanted. In other words, I not only have to remember to use the
option, but when I forget, I may get punished very annoyingly by results
which are subtly different from what I want.

So I am in favor of taking it further, but even if we do, the
command-line option is the right thing to be doing _now_.

> "git grep -e frotz .." will work in your "from linux/subproject/t look for
> everywhere in linux/subproject", but if "/t" part were much longer and
> variable (iow you need to chdir around inside linux/subproject to scratch
> your itch) compared to "linux/subproject" part that is much shorter and
> static (to your work), it may make sense to give us a mode to specify
> pathspec from the top of the tree.
> 
>     $ cd linux/subproject
>     $ cd foo
>     $ cd bar
>     $ cd baz
>     $ git grep --absolute-pathspec -e frotz -- linux/subproject
> 
> As "git grep" never takes absolute paths, we _might_ be able to also do
> 
>     $ git grep -e frotz -- /linux/subproject
> 
> to achieve the same.

Certainly I think that would be an improvement. But again, it suffers
from the "you must remember to do this" as above. I really want "git
grep" to Do What I Mean.

I have to wonder: is "git grep" really plumbing or porcelain? It is
really just a wrapper for

  git ls-files | xargs grep

Do people actually use it in their scripts? Should they be?

-Peff

Re: [PATCH] grep: --full-tree

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Wed, Nov 25, 2009 at 12:52:11PM -0800, Junio C Hamano wrote:
>
>> So I think the posted patch alone without changing anything else would be
>> the approach to give the most benefit with the least impact to existing
>> users, at least for now.
>
> Yes, I meant to say in my original message but forgot to: I think
> --full-tree is an important first step, no matter what happens next. It
> gives people a way to do what they want without typing the right number
> of ".."s, and it opens up --no-full-tree if the default changes later.
>
> But I do worry about it being a command-line option. You are asking the
> user to remember to type --full-tree every time.

We could redefine get_pathspec() to treat a pathspec that begins with a
slash to be anchored at the top, i.e.

	$ git grep -e frotz /

would be a nicer way to spell

	$ git grep --full-tree -e frotz

and allows you more than what you can do with --full-tree, e.g.

	$ cd linux/subtree/some/very/deep/subdir/you/do/not/remember/exactly
	$ git grep -e frotz /linux/subtree

If we do that, it will not be limited to "grep" but would bring uniformity
to the command set [*1*].  Of course, you can keep doing

	$ cd t
	$ git grep -e frotz .

to look inside only the current directory, and once this new convention is
accepted and widely used, it would become possible to flip the default
without causing too much pain (yes, I am agreeing with you that this is an
important first step).

Once there is a convenient and uniform way to ask for either behaviour, no
matter what the default is, the scripts that want specific behaviour can
be updated to choose whichever they want, given enough time (say, 2.0.0).

> Certainly I think that would be an improvement. But again, it suffers
> from the "you must remember to do this" as above. I really want "git
> grep" to Do What I Mean.

And /this-is-absolute is one way to tell "grep" What You Mean.  I do not
claim it would be the _best_ way (I just concocted it up a few minutes ago
without giving it deep thought).  Do you have a better alternative in
mind?

> I have to wonder: is "git grep" really plumbing or porcelain? It is
> really just a wrapper for
>
>   git ls-files | xargs grep
>
> Do people actually use it in their scripts? Should they be?

The issue is not necessarily "scripts", but "what people use the output
for".

My earlier "push is excusable" was primarily because "push" tends to be
the _final_ action in the chain of events, as opposed to "ls-files" and
"grep" output that are meant to be used by the user to _decide_ what to
do next depending on what they find, and as such, the latter has more
problem if they changed behaviour based on the configuration.


[Footnote]

*1* It won't be only get_pathspec(), but we would also need to teach
verify_filename() and verify_non_filename() about the new convention.

Re: [PATCH] grep: --full-tree

[Jeff King]

On Wed, Nov 25, 2009 at 01:33:22PM -0800, Junio C Hamano wrote:

> We could redefine get_pathspec() to treat a pathspec that begins with a
> slash to be anchored at the top, i.e.
> 
> 	$ git grep -e frotz /
> 
> would be a nicer way to spell
> 
> 	$ git grep --full-tree -e frotz

I do like that idea (and I cannot see any obvious flaw in it, though I
have only been think for a few minutes). I am not sure how useful it
will be for other commands. Conceptually I might use it for "diff" and
"status" (the new version that uses pathspecs sanely :) ), but those
commands generally aren't a big deal. I haven't touched anything in the
uninteresting subtree, so there is nothing to report.

Hmm. Actually, after having considered that, don't we actually allow
absolute paths in diff to do out-of-tree diffs? I haven't looked at how
that code interacts with get_pathspec.

> > Certainly I think that would be an improvement. But again, it suffers
> > from the "you must remember to do this" as above. I really want "git
> > grep" to Do What I Mean.
> 
> And /this-is-absolute is one way to tell "grep" What You Mean.  I do not
> claim it would be the _best_ way (I just concocted it up a few minutes ago
> without giving it deep thought).  Do you have a better alternative in
> mind?

Well, what I meant is that I shouldn't have to tell it each time what I
mean. I should be able to set up configuration so that it does what I
want (well, ideally, it would just read my mind, but I am willing to
concede that point). That is, I don't want to have to remember "git grep
--full-tree" or "git grep /" every time, because I am not likely to
notice when I forget. I want to set up "when I am in this directory,
this is probably what I want".

> My earlier "push is excusable" was primarily because "push" tends to be
> the _final_ action in the chain of events, as opposed to "ls-files" and
> "grep" output that are meant to be used by the user to _decide_ what to
> do next depending on what they find, and as such, the latter has more
> problem if they changed behaviour based on the configuration.

I'm not sure I really understand. "git grep" is routinely producing
wrong results for me _now_. I'd like to configure it so that it produces
results more sensible to me. If I am the one who sets the configuration
variable to something more sensible for my workflow, who am I hurting?

-Peff

Re: [PATCH] grep: --full-tree

[James Pickens]

On Wed, Nov 25, 2009 at 2:49 PM, Jeff King <peff@peff.net> wrote:
> I'm not sure I really understand. "git grep" is routinely producing
> wrong results for me _now_. I'd like to configure it so that it produces
> results more sensible to me. If I am the one who sets the configuration
> variable to something more sensible for my workflow, who am I hurting?

Config options are not free - they add code bloat, increase the maintenance
and testing burden, make it harder to explain how Git works if you have to
say things like "if config X is true, then Git does ..., otherwise Git does
..., unless config Y is false, in which case Git does ...", make it harder
to debug when Git doesn't do what you expected if you have to check a bunch
of configs to figure out what the behavior should be, and make it harder to
develop new features since you have to consider how they might interact with
lots of config options.  So I think the bar for adding config options,
especially ones that fundamentally change user visible behavior, should be
set pretty high, and this one doesn't even come close to getting over the
bar.

I like Junio's suggestion to make paths starting with / anchored to the
top.  If that were added then it would be easy for users to tell Git what
they want; they just have to use the right pathspec, which I think is a
very reasonable requirement.

That's my 2 cents....

James

Re: [PATCH] grep: --full-tree

[A Large Angry SCM]

Junio C Hamano wrote:
 > Jeff King <peff@peff.net> writes:
 >
 >> On Wed, Nov 25, 2009 at 12:52:11PM -0800, Junio C Hamano wrote:
 >>
 >>> So I think the posted patch alone without changing anything else 
would be
 >>> the approach to give the most benefit with the least impact to existing
 >>> users, at least for now.
 >> Yes, I meant to say in my original message but forgot to: I think
 >> --full-tree is an important first step, no matter what happens next. It
 >> gives people a way to do what they want without typing the right number
 >> of ".."s, and it opens up --no-full-tree if the default changes later.
 >>
 >> But I do worry about it being a command-line option. You are asking the
 >> user to remember to type --full-tree every time.
 >
 > We could redefine get_pathspec() to treat a pathspec that begins with a
 > slash to be anchored at the top, i.e.
 >
 > 	$ git grep -e frotz /
 >
 > would be a nicer way to spell
 >
 > 	$ git grep --full-tree -e frotz
 >
 > and allows you more than what you can do with --full-tree, e.g.
 >
 > 	$ cd linux/subtree/some/very/deep/subdir/you/do/not/remember/exactly
 > 	$ git grep -e frotz /linux/subtree
 >
 > If we do that, it will not be limited to "grep" but would bring 
uniformity
 > to the command set [*1*].  Of course, you can keep doing
 >
 > 	$ cd t
 > 	$ git grep -e frotz .
 >
 > to look inside only the current directory, and once this new 
convention is
 > accepted and widely used, it would become possible to flip the default
 > without causing too much pain (yes, I am agreeing with you that this 
is an
 > important first step).
 >
 > Once there is a convenient and uniform way to ask for either 
behaviour, no
 > matter what the default is, the scripts that want specific behaviour can
 > be updated to choose whichever they want, given enough time (say, 2.0.0).
 >

Speaking as a `grep' user: having git-grep behave radically different 
than normal grep would be/is very annoying [*1*].

Speaking as a `git' user: having the different git commands use 
radically different path conventions, relative to other git commands, 
would be/is very annoying [*1*].

To make the reconciliation  even more difficult, some git commands will 
also work on out-of-tree paths.

Footnotes:
[*1*] And surprising to new/occasional git users.

Re: [PATCH] grep: --full-tree

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> ... That is, I don't want to have to remember "git grep
> --full-tree" or "git grep /" every time

But that cuts both ways.  If you change the default to full-tree,
people will forget to put "." every time when asking to limit to the
current directory.

> If I am the one who sets the configuration variable to something more
> sensible for my workflow, who am I hurting?

Somebody more clueful in git than you who is called to help you in your
repository when you have trouble.  Obviously "you" in this sentence is not
Jeff King, but I think you get the point.

And re-read what I wrote in its entirety and notice I am not disagreeing
with you that the long term goal should be to have the default changed
consistently for all command to do the full tree.  The important first
step is to make sure we are capable of doing both full tree and limit to
current directory and "grep" is one example that cannot do both, and be it
the --full-tree option or new /rooted-pathspec, we need some change _now_
that is backward compatible to pave the way for later changes.  We give
people convenient way to choose between the two, and _train_ them to
express which way they want _without_ having to think.  After that is
achieved, the default does not matter much and we can safely change the
default.

Think of it as a way to force existing users _unlearn_ the command
specific default we currently have.  Because any change of default will
hurt them until that is done, we should start training them as early as
possible.

Re: [PATCH] grep: --full-tree

[Jeff King]

On Wed, Nov 25, 2009 at 03:12:26PM -0700, James Pickens wrote:

> Config options are not free - they add code bloat, increase the maintenance
> and testing burden, make it harder to explain how Git works if you have to
> say things like "if config X is true, then Git does ..., otherwise Git does
> ..., unless config Y is false, in which case Git does ...", make it harder
> to debug when Git doesn't do what you expected if you have to check a bunch
> of configs to figure out what the behavior should be, and make it harder to
> develop new features since you have to consider how they might interact with
> lots of config options.  So I think the bar for adding config options,
> especially ones that fundamentally change user visible behavior, should be
> set pretty high, and this one doesn't even come close to getting over the
> bar.

Sure, there are all those downsides. But what is the other option?
Making me use the command line option (or pathspec magic) every single
time I invoke git grep? That is a huge downside to me.

I started to try to write an argument against this, but I really don't
know how to. You don't think this particular option gets over the bar.
Probably because it is not something that has been annoying you
personally. But is _is_ something that has been annoying me. Now we are
both making claims from our gut. How do we proceed with a rational
analysis?

-Peff

Re: [PATCH] grep: --full-tree

[Junio C Hamano]

A Large Angry SCM <gitzilla@gmail.com> writes:

> To make the reconciliation  even more difficult, some git commands
> will also work on out-of-tree paths.

I think people know my feeling towards "diff --no-index".  It makes many
nice features invented in git to people who do not use "git" (e.g, to
compare files outside of control of git using --color-words).  If it
happened in an ideal world, it wouldn't have been done as a patch to "git"
but to something like "GNU diff" to benefit people who do not even want to
install "git".  But unfortunately that wasn't the way it was done.

It has turned out to be an unnecessary maintenance burden ever since it
was applied and every time we needed to change small things to "git diff".
I would not be very unhappy if we need to ejected it from "git diff" if it
turns out to hinder the necessary UI changes too much.

Re: [PATCH] grep: --full-tree

[Jeff King]

On Wed, Nov 25, 2009 at 02:19:35PM -0800, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > ... That is, I don't want to have to remember "git grep
> > --full-tree" or "git grep /" every time
> 
> But that cuts both ways.  If you change the default to full-tree,
> people will forget to put "." every time when asking to limit to the
> current directory.

I know. Which is why I am arguing for a configuration option.

Though as a side note, I think if you are going to err, it is probably
better to err in showing _too much_ data. It is easier for the user to
notice the situation and re-issue the command with more limits than it
is for them to notice that some results are missing and re-issue the
command with fewer limits.

> > If I am the one who sets the configuration variable to something more
> > sensible for my workflow, who am I hurting?
> 
> Somebody more clueful in git than you who is called to help you in your
> repository when you have trouble.  Obviously "you" in this sentence is not
> Jeff King, but I think you get the point.

Clearly, because there _isn't_ anybody more clueful than me in git. ;)

But that is what my meta-rant elsewhere in the thread was about. Sure,
it hurts when more clueful people are called in (and that includes
asking for help on the list). But that evil to me is much less than a
user who is frustrated because they have to specify the same thing to
git over and over again.

> And re-read what I wrote in its entirety and notice I am not disagreeing
> with you that the long term goal should be to have the default changed
> consistently for all command to do the full tree.  The important first
> step is to make sure we are capable of doing both full tree and limit to
> current directory and "grep" is one example that cannot do both, and be it
> the --full-tree option or new /rooted-pathspec, we need some change _now_
> that is backward compatible to pave the way for later changes.  We give
> people convenient way to choose between the two, and _train_ them to
> express which way they want _without_ having to think.  After that is
> achieved, the default does not matter much and we can safely change the
> default.
> 
> Think of it as a way to force existing users _unlearn_ the command
> specific default we currently have.  Because any change of default will
> hurt them until that is done, we should start training them as early as
> possible.

I agree with all of this as far as changing the default goes. But the
point of my earlier messages was that I don't think there _is_ one sane
default. I really do want it different per-project. And that means a
configuration option.

-Peff

Re: [PATCH] grep: --full-tree

[Wincent Colaiuta]

El 25/11/2009, a las 23:12, James Pickens escribió:

> I like Junio's suggestion to make paths starting with / anchored to  
> the
> top.

Oh, I wouldn't like that at all. I think it would be a very ugly UI  
wart, because it would basically make Git behave differently than  
every other command line tool that accepts paths. If it is to deviate  
from the extremely widespread convention that paths starting with /  
refer to absolute paths rooted at the root of the filesystem, then the  
justification for it would need to be very strong indeed.

Cheers,
Wincent

Re: [PATCH] grep: --full-tree

[A Large Angry SCM]

Junio C Hamano wrote:
> A Large Angry SCM <gitzilla@gmail.com> writes:
> 
>> To make the reconciliation  even more difficult, some git commands
>> will also work on out-of-tree paths.
> 
> I think people know my feeling towards "diff --no-index".  It makes many
> nice features invented in git to people who do not use "git" (e.g, to
> compare files outside of control of git using --color-words).  If it
> happened in an ideal world, it wouldn't have been done as a patch to "git"
> but to something like "GNU diff" to benefit people who do not even want to
> install "git".  But unfortunately that wasn't the way it was done.
> 
> It has turned out to be an unnecessary maintenance burden ever since it
> was applied and every time we needed to change small things to "git diff".
> I would not be very unhappy if we need to ejected it from "git diff" if it
> turns out to hinder the necessary UI changes too much.
> 

git-diff aside, I would be much happier with git's interface, and I 
think new/occasional users would be also, if it was consistent across 
all of git commands. And even more so if git's conventions matched most 
other commands on my system of choice.

Re: [PATCH] grep: --full-tree

[A Large Angry SCM]

Jeff King wrote:
> On Wed, Nov 25, 2009 at 02:19:35PM -0800, Junio C Hamano wrote:
> 
>> Jeff King <peff@peff.net> writes:
>>
>>> ... That is, I don't want to have to remember "git grep
>>> --full-tree" or "git grep /" every time
>> But that cuts both ways.  If you change the default to full-tree,
>> people will forget to put "." every time when asking to limit to the
>> current directory.
> 
> I know. Which is why I am arguing for a configuration option.
> 
> Though as a side note, I think if you are going to err, it is probably
> better to err in showing _too much_ data. It is easier for the user to
> notice the situation and re-issue the command with more limits than it
> is for them to notice that some results are missing and re-issue the
> command with fewer limits.
> 
>>> If I am the one who sets the configuration variable to something more
>>> sensible for my workflow, who am I hurting?
>> Somebody more clueful in git than you who is called to help you in your
>> repository when you have trouble.  Obviously "you" in this sentence is not
>> Jeff King, but I think you get the point.
> 
> Clearly, because there _isn't_ anybody more clueful than me in git. ;)
> 
> But that is what my meta-rant elsewhere in the thread was about. Sure,
> it hurts when more clueful people are called in (and that includes
> asking for help on the list). But that evil to me is much less than a
> user who is frustrated because they have to specify the same thing to
> git over and over again.
> 
>> And re-read what I wrote in its entirety and notice I am not disagreeing
>> with you that the long term goal should be to have the default changed
>> consistently for all command to do the full tree.  The important first
>> step is to make sure we are capable of doing both full tree and limit to
>> current directory and "grep" is one example that cannot do both, and be it
>> the --full-tree option or new /rooted-pathspec, we need some change _now_
>> that is backward compatible to pave the way for later changes.  We give
>> people convenient way to choose between the two, and _train_ them to
>> express which way they want _without_ having to think.  After that is
>> achieved, the default does not matter much and we can safely change the
>> default.
>>
>> Think of it as a way to force existing users _unlearn_ the command
>> specific default we currently have.  Because any change of default will
>> hurt them until that is done, we should start training them as early as
>> possible.
> 
> I agree with all of this as far as changing the default goes. But the
> point of my earlier messages was that I don't think there _is_ one sane
> default. I really do want it different per-project. And that means a
> configuration option.

Since grep is so useful, both interactively and scripted, outside of 
git, this is a pretty convincing argument that git-grep, and all other 
git commands with configurable behavior or defaults that change over 
time, need a both a scripting form and an interactive form.

Re: [PATCH] grep: --full-tree

[A Large Angry SCM]

A Large Angry SCM wrote:
> git-diff aside, I would be much happier with git's interface, and I 
> think new/occasional users would be also, if it was consistent across 
> all of git commands. And even more so if git's conventions matched most 
> other commands on my system of choice.

s/my system of choice/the user's system of choice/

Re: [PATCH] grep: --full-tree

[Jeff King]

On Wed, Nov 25, 2009 at 05:41:33PM -0500, A Large Angry SCM wrote:

> >I agree with all of this as far as changing the default goes. But the
> >point of my earlier messages was that I don't think there _is_ one sane
> >default. I really do want it different per-project. And that means a
> >configuration option.
> 
> Since grep is so useful, both interactively and scripted, outside of
> git, this is a pretty convincing argument that git-grep, and all
> other git commands with configurable behavior or defaults that change
> over time, need a both a scripting form and an interactive form.

It is tempting to have scripts simply set a GIT_VANILLA environment
variable to ignore config options. But I think it is not quite so
simple. As a script, if I am calling "git log", do I want it to respect
the user's colorization config or not? It depends on _how_ I am calling
it. Is the output to be shown to the user, or am I going to process it
myself?

Similarly, why is the script calling "git grep"? If it is because the
script is a convenience wrapper (e.g., let's say to colorize the output
in a particular way), then it probably wants to respect my configured
choice of which files to grep. But if the script is just using "git
grep" to get data to perform some other calculation, then it probably
does care deeply about which set of files to grep.

So I think you have situations where scripts do want to invoke the
porcelain version of a command versus the plumbing. But much harder, you
have ones where they want to respect some options but not others.

-Peff

Re: [PATCH] grep: --full-tree

[A Large Angry SCM]

Jeff King wrote:
> On Wed, Nov 25, 2009 at 05:41:33PM -0500, A Large Angry SCM wrote:
> 
>>> I agree with all of this as far as changing the default goes. But the
>>> point of my earlier messages was that I don't think there _is_ one sane
>>> default. I really do want it different per-project. And that means a
>>> configuration option.
>> Since grep is so useful, both interactively and scripted, outside of
>> git, this is a pretty convincing argument that git-grep, and all
>> other git commands with configurable behavior or defaults that change
>> over time, need a both a scripting form and an interactive form.
> 
> It is tempting to have scripts simply set a GIT_VANILLA environment
> variable to ignore config options. But I think it is not quite so
> simple. As a script, if I am calling "git log", do I want it to respect
> the user's colorization config or not? It depends on _how_ I am calling
> it. Is the output to be shown to the user, or am I going to process it
> myself?
> 
> Similarly, why is the script calling "git grep"? If it is because the
> script is a convenience wrapper (e.g., let's say to colorize the output
> in a particular way), then it probably wants to respect my configured
> choice of which files to grep. But if the script is just using "git
> grep" to get data to perform some other calculation, then it probably
> does care deeply about which set of files to grep.
> 
> So I think you have situations where scripts do want to invoke the
> porcelain version of a command versus the plumbing. But much harder, you
> have ones where they want to respect some options but not others.

<semi rhetorical>
So, what's the solution?

Have every command command take a list of configuration options to 
ignore/respect?

Have every command take an option to ignore/respect _all_ configuration 
options?

Have inconsistency between commands, like we have now

Have commands have all kinds of hidden/undocumented default settings?
</semi rhetorical>

Re: [PATCH] grep: --full-tree

[Jeff King]

On Wed, Nov 25, 2009 at 06:07:00PM -0500, A Large Angry SCM wrote:

> <semi rhetorical>
> So, what's the solution?
> 
> Have every command command take a list of configuration options to
> ignore/respect?
> 
> Have every command take an option to ignore/respect _all_
> configuration options?
> 
> Have inconsistency between commands, like we have now
> 
> Have commands have all kinds of hidden/undocumented default settings?
> </semi rhetorical>

I don't know. All of those options suck. ;

Probably we would want something flexible, but with sane defaults. Like
an environment variable to ignore all (or most) config options, but then
the ability to opt into specific ones. Something like:

  GIT_PLUMBING=1; export GIT_PLUMBING
  git log ;# does not respect any non-plumbing config
  git --respect='log.showroot' ;# respect just the one variable
  git --respect='color.*' log ;# you get all color

But there are two big obstacles (besides the obvious issue that
introducing this in itself needs a gentle transition plan):

  1. We need to annotate every config option with whether it is
     potentially problematic. For example, core.filemode should probably
     be respected no matter what (but I'm not sure if it is simply true
     for core.*).

  2. Script writers need to actually use the system, which is somewhat
     more verbose and annoying than what they have to do now. But at
     least it defaults to safety when they are lazy, and then they can
     re-add options. Of course, they are stuck on an upgrade treadmill
     of analyzing and approving each new option that appears in git.

-Peff

Re: [PATCH] grep: --full-tree

[Sverre Rabbelier]

Heya,

On Thu, Nov 26, 2009 at 00:31, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> You mean like the many people who wanted to keep the dashed commands, but
> unlike who you do speak up _before_ your expectations are broken?

Yes, kinda like that, but with the note that I do think we should not
lock down the UI in favor or not breaking expectations. I mean, if
people want to retain a specific behavior forever, maybe they should
just keep using that version of git forever? Nah, that doens't work of
course, but I do think a balance is needed between the two (improving
the UI and not breaking people's expectations).

-- 
Cheers,

Sverre Rabbelier

Re: [PATCH] grep: --full-tree

[Johannes Schindelin]

Hi,

On Wed, 25 Nov 2009, Sverre Rabbelier wrote:

> On Wed, Nov 25, 2009 at 21:23, Junio C Hamano <gitster@pobox.com> wrote:
> > But then you can alias "gr 'grep --full-tree'" with the same ease and 
> > there is no reason to change the default.
> 
> I agree, but then again I'm somewhat biased, as I want the current 
> behavior :P.

You mean like the many people who wanted to keep the dashed commands, but 
unlike who you do speak up _before_ your expectations are broken?

Ciao,
Dscho

Re: [PATCH] grep: --full-tree

[Johannes Schindelin]

Hi,

On Wed, 25 Nov 2009, Junio C Hamano wrote:

> Jeff King <peff@peff.net> writes:
> 
> > On Wed, Nov 25, 2009 at 12:52:11PM -0800, Junio C Hamano wrote:
> >
> >> So I think the posted patch alone without changing anything else would be
> >> the approach to give the most benefit with the least impact to existing
> >> users, at least for now.
> >
> > Yes, I meant to say in my original message but forgot to: I think
> > --full-tree is an important first step, no matter what happens next. It
> > gives people a way to do what they want without typing the right number
> > of ".."s, and it opens up --no-full-tree if the default changes later.
> >
> > But I do worry about it being a command-line option. You are asking the
> > user to remember to type --full-tree every time.
> 
> We could redefine get_pathspec() to treat a pathspec that begins with a
> slash to be anchored at the top,

This would break spectacularly in MSys.  And this is just one reason not 
to do this "magic".

Clearly, a command line option is the only unambiguous way to do what you 
want to do (and not changing the default all of a sudden).

Ciao,
Dscho

Re: [PATCH] grep: --full-tree

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> This would break spectacularly in MSys.

How?  If that is the case wouldn't --full-tree break Msys the same way?

Re: [PATCH] grep: --full-tree

[Junio C Hamano]

Wincent Colaiuta <win@wincent.com> writes:

> Oh, I wouldn't like that at all. I think it would be a very ugly UI
> wart, because it would basically make Git behave differently than
> every other command line tool that accepts paths. If it is to deviate
> from the extremely widespread convention that paths starting with /
> refer to absolute paths rooted at the root of the filesystem, then the
> justification for it would need to be very strong indeed.

There are at least two flaws in that argument.

 - git does not accept paths (it lets you specify patterns that match,
   e.g. t/ to name ptahs under t/ directory).

 - "/pathspec" does follow the widespread convention that a string that
   begin with a "/" refer to a path rooted at the root _in the context_;
   the definition of root may or may not match the filesystem root.

   Think of things like <a href="/$path">Top</a>.  Does "/$path" mean at
   the root of filesystem?  No.

I am not married to the "git grep -e frotz /Documentation" notation, by
the way.  I just didn't think of a different notation that is equally
short, sweet and logical.  We could do //Documentation if it makes it more
distinct, but I do think it is worse than a single slash.

Re: [PATCH] grep: --full-tree

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Wed, Nov 25, 2009 at 02:19:35PM -0800, Junio C Hamano wrote:
>
>> Jeff King <peff@peff.net> writes:
>> 
>> > ... That is, I don't want to have to remember "git grep
>> > --full-tree" or "git grep /" every time
>> 
>> But that cuts both ways.  If you change the default to full-tree,
>> people will forget to put "." every time when asking to limit to the
>> current directory.
>
> I know. Which is why I am arguing for a configuration option.

Yeah; what is your take on tr/reset-checkout-patch topic, by the way?  I
do not particularly like a configuration that changes the behaviour of a
command in a drastic way---it will make helping others much harder, but I
guess it should be Ok?

This may sound like an OffTopic, but because we _are_ discussing
consistency, it matters.

Re: [PATCH] grep: --full-tree

[Johannes Schindelin]

Hi,

On Wed, 25 Nov 2009, Junio C Hamano wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > This would break spectacularly in MSys.
> 
> How?  If that is the case wouldn't --full-tree break Msys the same way?

If you want to specify an argument on MSys that starts with a slash, you 
have to provide double slashes, otherwise it gets expanded to the Windows 
path (prefixing with the absolute path of the MSys root).

But this introduces yet another inconcistency: using double slashes 
everywhere else does not work.

Hopefully you see the real reason why it breaks down?  The reason is that 
you try to re-interpret something in a special way that means something 
different.  A path starting with a slash simply does not mean "the root of 
the project".  It means "the root of the file system".

Ciao,
Dscho

Re: [PATCH] grep: --full-tree

[Junio C Hamano]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> If you want to specify an argument on MSys that starts with a slash, you 
> have to provide double slashes, otherwise it gets expanded to the Windows 
> path (prefixing with the absolute path of the MSys root).

How well does the command line hack handle things like

    $ git archive --output=/var/tmp/foo.tar

I have to wonder.

> Hopefully you see the real reason why it breaks down?

Yes, Windows' braindamage.  It doesn't invalidate anything I said so far,
but only proves that emulation can go only so far.  I hope msys port can
grok real "path from root" notation on Windows, e.g.

    $ git diff --no-index "C:\My Documents/hello.txt" "D:\goodbye.txt"

(or perhaps "//c/my documents/...", but I do not care about the details).

Re: [PATCH] grep: --full-tree

[A Large Angry SCM]

Junio C Hamano wrote:

>  - git does not accept paths (it lets you specify patterns that match,
>    e.g. t/ to name ptahs under t/ directory).

Here is where it get interesting!

Our users, new and old alike, are wanting consistency. Consistency 
amongst the git commands. Consistency with their platform of choice. 
Consistency with what they are familiar with, Consistency with their 
expectations.

Declaring that git commands (all?!) do not take paths but patterns does 
not help the situation; however technically correct it may be.

>  - "/pathspec" does follow the widespread convention that a string that
>    begin with a "/" refer to a path rooted at the root _in the context_;
>    the definition of root may or may not match the filesystem root.

But the users are almost always dealing with things (objects) that 
started as files, act like files and may be files again. Why should they 
not expect filesystem semantics.

Re: [PATCH] grep: --full-tree

[Junio C Hamano]

A Large Angry SCM <gitzilla@gmail.com> writes:

> But the users are almost always dealing with things (objects) that
> started as files, act like files and may be files again. Why should
> they not expect filesystem semantics.

Do you truly want to see this?

    diff --git a/var/tmp/git/Makefile b/var/tmp/git/Makefile
    index 5a0b3d4..e9b03a8 100644
    --- a/var/tmp/git/Makefile
    +++ b/var/tmp/git/Makefile
    @@ -1985,3 +1985,4 @@ coverage-report:
    ...

As long as you are talking about paths you communicate with git, your
root _is_ the root of the work tree, and it shouldn't matter where you
have your work tree.

That is what I meant by "the root _in the context_".

Re: [PATCH] grep: --full-tree

[A Large Angry SCM]

Junio C Hamano wrote:
> A Large Angry SCM <gitzilla@gmail.com> writes:
> 
>> But the users are almost always dealing with things (objects) that
>> started as files, act like files and may be files again. Why should
>> they not expect filesystem semantics.
> 
> Do you truly want to see this?
> 
>     diff --git a/var/tmp/git/Makefile b/var/tmp/git/Makefile
>     index 5a0b3d4..e9b03a8 100644
>     --- a/var/tmp/git/Makefile
>     +++ b/var/tmp/git/Makefile
>     @@ -1985,3 +1985,4 @@ coverage-report:
>     ...
> 
> As long as you are talking about paths you communicate with git, your
> root _is_ the root of the work tree, and it shouldn't matter where you
> have your work tree.
> 
> That is what I meant by "the root _in the context_".
> 

Nice example for a command output. But what did the user specify on the 
command line?

Re: [PATCH] grep: --full-tree

[Junio C Hamano]

A Large Angry SCM <gitzilla@gmail.com> writes:

> Junio C Hamano wrote:
>> A Large Angry SCM <gitzilla@gmail.com> writes:
>>
>>> But the users are almost always dealing with things (objects) that
>>> started as files, act like files and may be files again. Why should
>>> they not expect filesystem semantics.
>>
>> Do you truly want to see this?
>>
>>     diff --git a/var/tmp/git/Makefile b/var/tmp/git/Makefile
>>     index 5a0b3d4..e9b03a8 100644
>>     --- a/var/tmp/git/Makefile
>>     +++ b/var/tmp/git/Makefile
>>     @@ -1985,3 +1985,4 @@ coverage-report:
>>     ...
>>
>> As long as you are talking about paths you communicate with git, your
>> root _is_ the root of the work tree, and it shouldn't matter where you
>> have your work tree.
>>
>> That is what I meant by "the root _in the context_".
>
> Nice example for a command output. But what did the user specify on
> the command line?

Either relative path from $(cwd) or full pathname from the root of the
filesystem in your world (because you do not understand the value of the
context), and ralative path from $(cwd) or full pathname from the root of
the work tree in my world.

Re: [PATCH] grep: --full-tree

[James Pickens]

On Wed, Nov 25, 2009 at 3:20 PM, Jeff King <peff@peff.net> wrote:
> Sure, there are all those downsides. But what is the other option?
> Making me use the command line option (or pathspec magic) every single
> time I invoke git grep?

Yes, but only when you want non-default behavior, not every single time.

> That is a huge downside to me.

Is it *really*?  Does it also bother you that you have to tell standalone
unix commands like diff and grep what you want them to diff or grep every
single time you invoke them?

> I started to try to write an argument against this, but I really don't
> know how to. You don't think this particular option gets over the bar.
> Probably because it is not something that has been annoying you
> personally. But is _is_ something that has been annoying me. Now we are
> both making claims from our gut. How do we proceed with a rational
> analysis?

I really think that this config option wouldn't even help you, because
you'll have to remember what that option is set to in each working repo,
and type the right command based on the setting.  That seems worse than
having to use the same options over and over again, which you probably use
the shell's history for anyways and don't actually type the same stuff over
and over.  Oh and you also have to remember to set the option in each new
repo you create.

If you can get the behavior you want using an alias or a script, then I
suggest you do that.  I don't think this config option should be considered
unless *many* people want it, and so far I count only 1.

James

Re: [PATCH] grep: --full-tree

[James Pickens]

On Wed, Nov 25, 2009 at 5:00 PM, Junio C Hamano <gitster@pobox.com> wrote:
>  - git does not accept paths (it lets you specify patterns that match,
>   e.g. t/ to name ptahs under t/ directory).

That's not entirely true, unfortunately:

$ echo >> unpack-trees.c
$ git diff --name-status unpack-trees.c
M       unpack-trees.c
$ git diff --name-status $PWD/unpack-trees.c
M       unpack-trees.c
$ git diff --name-status $PWD/../git/unpack-trees.c
M       unpack-trees.c
$ git diff --name-status ../git/unpack-trees.c
fatal: '../git/unpack-trees.c' is outside repository

So it seems that 'git diff' accepts absolute paths as long as they end up
in the repository, but oddly enough, doesn't do so for relative paths.
It's possible that some users have scripts that use absolute paths, and
changing the interpretation would break those scripts.  Such scripts
*should* be rare, so maybe it's ok to break them, but it needs to be
considered.

James

Re: [PATCH] grep: --full-tree

[Jeff King]

On Thu, Nov 26, 2009 at 10:56:55AM -0700, James Pickens wrote:

> On Wed, Nov 25, 2009 at 3:20 PM, Jeff King <peff@peff.net> wrote:
> > Sure, there are all those downsides. But what is the other option?
> > Making me use the command line option (or pathspec magic) every single
> > time I invoke git grep?
> 
> Yes, but only when you want non-default behavior, not every single time.

Did you miss the part of the thread where I explained that in certain
repos, I want it one way every single time, and in others, I want it the
other way?

So yes, in certain repos, it really is every single time.

> > That is a huge downside to me.
> 
> Is it *really*?  Does it also bother you that you have to tell standalone
> unix commands like diff and grep what you want them to diff or grep every
> single time you invoke them?

This is a strawman. I am not saying every command-line option should be
made into a configuration option. I am saying that some options,
including this one, would be useful as configuration options. I have
already explained several times in this thread exactly what
characteristics of this option make that so.

And please, questions like "Is it *really*?" don't add anything. Yes,
really, or I wouldn't be having this discussion. This behavior has
bitten me many times while using "git grep". I'm not making it up. Maybe
I am the only one in the world, but I don't see how it makes any sense
to argue that I am not actually annoyed by it.

> I really think that this config option wouldn't even help you, because
> you'll have to remember what that option is set to in each working repo,
> and type the right command based on the setting.  That seems worse than

No, the _point_ is that I don't have to remember the right command in
each repo. I can set it up for the workflow that matches that repository
and then issue "git grep" without remembering which type I'm in.

> If you can get the behavior you want using an alias or a script, then I
> suggest you do that.  I don't think this config option should be considered
> unless *many* people want it, and so far I count only 1.

Perhaps I am the only one who wants to use the config option per-repo.
But we have already seen support for both behaviors, which means there
are people who will be dissatisfied with either simply leaving the
default or changing the default. 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).

-Peff

Re: [PATCH] grep: --full-tree

[Jeff King]

On Wed, Nov 25, 2009 at 04:02:57PM -0800, Junio C Hamano wrote:

> Yeah; what is your take on tr/reset-checkout-patch topic, by the way?  I
> do not particularly like a configuration that changes the behaviour of a
> command in a drastic way---it will make helping others much harder, but I
> guess it should be Ok?
> 
> This may sound like an OffTopic, but because we _are_ discussing
> consistency, it matters.

It is near the top of my to-review queue. Honestly, despite any
arguments I may have made when the original reset/checkout -p series was
posted, I have been pretty happy with the current behavior. I'll take a
look now and respond in more detail in that thread.

-Peff

Re: [PATCH] grep: --full-tree

[Junio C Hamano]

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.

Re: [PATCH] grep: --full-tree

[Johannes Schindelin]

Hi,

On Fri, 27 Nov 2009, Jeff King wrote:

> On Thu, Nov 26, 2009 at 10:56:55AM -0700, James Pickens wrote:
> 
> > On Wed, Nov 25, 2009 at 3:20 PM, Jeff King <peff@peff.net> wrote:
> > > Sure, there are all those downsides. But what is the other option? 
> > > Making me use the command line option (or pathspec magic) every 
> > > single time I invoke git grep?
> > 
> > Yes, but only when you want non-default behavior, not every single 
> > time.
> 
> Did you miss the part of the thread where I explained that in certain 
> repos, I want it one way every single time, and in others, I want it the 
> other way?

Guess what.  I have a similar problem, only it is that my "git status" 
output is _always_ too long, so I always have to page it.

Once upon a time, Junio applied a patch that implied -p with status.  I 
was overjoyed.  He reverted that patch later.  Yes, exactly.

So I end up doing "git config --global ps '-p status'" on every new 
account (I usually even forget to curse!), and I really cannot see why you 
do not do the equivalent "git config fullgrep grep --full-tree" in your 
repositories (or even the global thing).

The further benefit is that we stop talking about breaking backwards 
compatibility, and we stop talking about making it hard for Git experts to 
help newbies.

Ciao,
Dscho

Re: [PATCH] grep: --full-tree

[Jeff King]

On Fri, Nov 27, 2009 at 10:31:30AM +0100, Johannes Schindelin wrote:

> Guess what.  I have a similar problem, only it is that my "git status" 
> output is _always_ too long, so I always have to page it.
> 
> Once upon a time, Junio applied a patch that implied -p with status.  I 
> was overjoyed.  He reverted that patch later.  Yes, exactly.
> 
> So I end up doing "git config --global ps '-p status'" on every new 

If only somebody had written a "pager.status" configuration variable,
you could use that. Oh wait. I did. And it shipped in v1.6.0.

> account (I usually even forget to curse!), and I really cannot see why you 
> do not do the equivalent "git config fullgrep grep --full-tree" in your 
> repositories (or even the global thing).
>
> The further benefit is that we stop talking about breaking backwards 
> compatibility, and we stop talking about making it hard for Git experts to 
> help newbies.

I guess you missed the part of the thread where I already discussed
this. It was here:

  http://article.gmane.org/gmane.comp.version-control.git/133672

-Peff

Re: [PATCH] grep: --full-tree

[Matthieu Moy]

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

> Guess what.  I have a similar problem, only it is that my "git status" 
> output is _always_ too long, so I always have to page it.
>
> Once upon a time, Junio applied a patch that implied -p with status.  I 
> was overjoyed.  He reverted that patch later.  Yes, exactly.

In this particular example, a config variable was added (pager.status
= true). But one big difference is that while pager.status = true can
be /annoying/ for some users, it can never really harm (since the
pager will automatically disable itself in the cases where you'd
really don't want it).

OTOH, a config variable that actually changes the beahvior of the
command can indeed harm. Those who ever tried doing portable
programming in PHP, where the apache config can actually change the
semantics of the language probably understand what I mean ;-).

> So I end up doing "git config --global ps '-p status'" on every new 
> account (I usually even forget to curse!), and I really cannot see why you 
> do not do the equivalent "git config fullgrep grep --full-tree" in your 
> repositories (or even the global thing).

(I guess you meant alias.fullgrep)

Maybe "mygrep" or "dwimgrep" would be a better name, except for the
difficulty to type it: the same alias can be defined to different
things on different machines.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [PATCH] grep: --full-tree

[Johannes Schindelin]

Hi,

On Fri, 27 Nov 2009, Jeff King wrote:

> On Fri, Nov 27, 2009 at 10:31:30AM +0100, Johannes Schindelin wrote:
> 
> > Guess what.  I have a similar problem, only it is that my "git status" 
> > output is _always_ too long, so I always have to page it.
> > 
> > Once upon a time, Junio applied a patch that implied -p with status.  
> > I was overjoyed.  He reverted that patch later.  Yes, exactly.
> > 
> > So I end up doing "git config --global ps '-p status'" on every new 
> 
> If only somebody had written a "pager.status" configuration variable,
> you could use that. Oh wait. I did. And it shipped in v1.6.0.

And it makes things inconsistent.  That is why I do not use it.  Do you 
work on 10 different computers?  I do.  And nothing is more unnerving than 
the same command producing something different on the different computers.

Sure, after a few minutes of fiddling I find out that it was my fault to 
begin with, but dammit! if the tool makes it that hard already for an 
expert, it is outright unusable for new users.

I, for one, do not like Git's reputation, but I am tired of trying to 
fight for the users.  BTW quick question: how many Git _users_ were at the 
GitTogether at MV?  0?

> > account (I usually even forget to curse!), and I really cannot see why 
> > you do not do the equivalent "git config fullgrep grep --full-tree" in 
> > your repositories (or even the global thing).
> >
> > The further benefit is that we stop talking about breaking backwards 
> > compatibility, and we stop talking about making it hard for Git 
> > experts to help newbies.
> 
> I guess you missed the part of the thread where I already discussed
> this. It was here:
> 
>   http://article.gmane.org/gmane.comp.version-control.git/133672

I only skimmed it, yes.  And I did not plan to participate in this thread.  
But it seems that my views are not represented enough, even if gitzilla 
chimed in with the very valid, under-acknowledged and over-ignored 
message: consistency is good.  Corollary: inconsistency is bad.

Ciao,
Dscho

Re: [PATCH] grep: --full-tree

[Johannes Schindelin]

Hi,

On Fri, 27 Nov 2009, Matthieu Moy wrote:

> Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:
> 
> > Guess what.  I have a similar problem, only it is that my "git status" 
> > output is _always_ too long, so I always have to page it.
> >
> > Once upon a time, Junio applied a patch that implied -p with status.  
> > I was overjoyed.  He reverted that patch later.  Yes, exactly.
> 
> In this particular example, a config variable was added (pager.status = 
> true). But one big difference is that while pager.status = true can be 
> /annoying/ for some users, it can never really harm (since the pager 
> will automatically disable itself in the cases where you'd really don't 
> want it).

It changes behavior.  And worse: it changes behavior _in a different 
manner_ in different repositories.  I have too many of them to remember 
what I set where.

So it is harmful in a very real sense.

> > So I end up doing "git config --global ps '-p status'" on every new 
> > account (I usually even forget to curse!), and I really cannot see why you 
> > do not do the equivalent "git config fullgrep grep --full-tree" in your 
> > repositories (or even the global thing).
> 
> (I guess you meant alias.fullgrep)

You guessed right, and likewise alias.ps.

Ciao,
Dscho

Re: [PATCH] grep: --full-tree

[Uri Okrent]

I've been following this thread for a long time and now I feel the
need to chime in...

On Fri, Nov 27, 2009 at 2:53 AM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> And it makes things inconsistent.  That is why I do not use it.

The number one problem my users have with git is inconsistent
behavior, both internally to git, and externally with respect to the
rest of the OS. But really, the issue is one of managing expectations,
which is where we here tend to fall down.

When you name the command grep, whether you like it of not,
you've bought into a certain set of expectations from the user
who has been using unix's grep since she was a baby. Saying,
"well, in git it works this way", (or saying "well in git those path
looking things you've been providing to commands are not really
paths, so don't expect them to act as such"), would make my
users want to vomit all over me, and then, not use git (a shame
since it's the best scm system around IMHO).

If we intend the behavior of the command to be materially different
from the good old unix standby, then we shouldn't use the same
name, and create the expectation that they are getting essentially
the same thing (git search, git pickaxe, or something carries no
semantic baggage---and no, I'm not suggesting we change the
name).

> I, for one, do not like Git's reputation...

There's the rub. How do we achieve consistency without breaking the
world? The short answer is, you really can't. As programmers we tend
to be a very timid bunch, but sometimes (and David A. can attest to
this, at least at dayjob) it's better to just make a change for the better,
and just deal with the breakages. It is possible to change behavior (and
even break some scripts! I firmly believe it is worthwhile sacrificing
some scripts on the altar of consistency).

The key once again, is managing expectations. We can't go around
changing everything willy-nilly, and we can't be continually changing
things. Here is where we could take a lesson from the python
community.

When they decided they needed to change things, they bundled a
bunch of backwards incompatible changes together and went for it.
Yes, Python 3 will break your scripts, but the most important thing is,
everybody knows it.

A similar thing was done here with the huge warning that push spits
out, but in the general case I would argue, that the wisest course is to
save backwards incompatible changes for a git 2 or something, where
we know we're breaking the world, and then scratch all our (well thought
out) backwards incompatible itches at once.

Whew. A bit of a rant, but there you go...
-- 
   Uri

Please consider the environment before printing this message.
http://www.panda.org/how_you_can_help/

Re: [PATCH] grep: --full-tree

[Jeff King]

On Fri, Nov 27, 2009 at 11:53:42AM +0100, Johannes Schindelin wrote:

> > If only somebody had written a "pager.status" configuration variable,
> > you could use that. Oh wait. I did. And it shipped in v1.6.0.
> 
> And it makes things inconsistent.  That is why I do not use it. 

Then you can not use this configuration variable, too. Has the existence
of pager.status, since you do not use it, been a problem for you so far?

> Do you work on 10 different computers?  I do.  And nothing is more
> unnerving than the same command producing something different on the
> different computers.

Yes, as a matter of fact, I do work on 10 different computers. I'm sorry
that you find managing your configuration so challenging. But if you
don't use the configuration variable, then your own personal setup is
totally irrelevant.

If your argument is that this lack of consistency will irritate users,
you need to show that:

  1. There are users who switch between a large number of setups, but
     will not apply config consistently.

  2. Some of these setups will be using the new config option.

If they are all controlled by a single user, how is that user any worse
off for the config option existing? They can choose not to use it if
the hassle is not worth it. I do not think the existence of an option is
giving too much rope to these users.

If you are talking about 10 machines, all controlled by different users,
whose terminals you have to sit down on to help them, then yes, it will
be inconvenient for you. But if users are setting up configuration for
these machines, shouldn't _their_ convenience in using configuration
trump _your_ convenience for occasionally sitting down and helping them?

> I, for one, do not like Git's reputation, but I am tired of trying to 
> fight for the users.  BTW quick question: how many Git _users_ were at the 
> GitTogether at MV?  0?

In my opinion, you are actively fighting _against_ a user in this case.

And the GitTogether had a "users complain about git, and we try to
listen" session. There were two google users in person, but we also went
through a list of pre-made questions from other googlers. This issue
wasn't discussed, though. Nor was the question of consistency between
configurations, to my recollection. I think Shawn may have taken notes,
and could be more specific.

> >   http://article.gmane.org/gmane.comp.version-control.git/133672
> 
> I only skimmed it, yes.  And I did not plan to participate in this thread.  
> But it seems that my views are not represented enough, even if gitzilla 
> chimed in with the very valid, under-acknowledged and over-ignored 
> message: consistency is good.  Corollary: inconsistency is bad.

That is an over-simplification. Inconsistency between setups is bad. But
so is inconsistency between git commands, and between git and other
commands. So is not supporting a user's workflow, or supporting it in a
way that is tedious and error-prone. You have to weigh the badness of
those things against each other in finding a solution.

But then, that was the point I already made in the article linked above.

-Peff

Re: [PATCH] grep: --full-tree

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Fri, Nov 27, 2009 at 10:31:30AM +0100, Johannes Schindelin wrote:
>
>> Guess what.  I have a similar problem, only it is that my "git status" 
>> output is _always_ too long, so I always have to page it.
>> 
>> Once upon a time, Junio applied a patch that implied -p with status.  I 
>> was overjoyed.  He reverted that patch later.  Yes, exactly.

It would have been more fair to me if Dscho said "He had to revert", to
hint that it was not due to me changing the preference left and right on a
whim.

>> So I end up doing "git config --global ps '-p status'" on every new 
>
> If only somebody had written a "pager.status" configuration variable,
> you could use that. Oh wait. I did. And it shipped in v1.6.0.

Nice try but, "grep" and "status" are apples and oranges comparision.

A "status" command that pages or does not page the output only when
spitting out to a terminal won't hurt somebody who helps another with the
configuration set differently, as much as a "grep" that shows or not shows
matches from other parts of the tree would, and when used by a script to
make a decision based on the output, the caller has to capture the output
first, and unless the caller drives "status" via pty, e.g. using "expect",
paging behaviour will be disabled no matter what the configuration setting
is.

So neither "it would hurt people who help others" nor "it would hurt
scripts" would apply to "status".  But both would apply to "grep".

>> The further benefit is that we stop talking about breaking backwards 
>> compatibility, and we stop talking about making it hard for Git experts to 
>> help newbies.
>
> I guess you missed the part of the thread where I already discussed
> this. It was here:
>
>   http://article.gmane.org/gmane.comp.version-control.git/133672

This was a very good summary, and was one of the reasons that made me
reconsider placing too much weight on "it would hurt people who help
others" (but not on "it would hurt scripts").

Re: [PATCH] grep: --full-tree

[Junio C Hamano]

Uri Okrent <uokrent@gmail.com> writes:

> The key once again, is managing expectations. We can't go around
> changing everything willy-nilly, and we can't be continually changing
> things. Here is where we could take a lesson from the python
> community.
>
> When they decided they needed to change things, they bundled a
> bunch of backwards incompatible changes together and went for it.
> Yes, Python 3 will break your scripts, but the most important thing is,
> everybody knows it.
>
> A similar thing was done here with the huge warning that push spits
> out, but in the general case I would argue, that the wisest course is to
> save backwards incompatible changes for a git 2 or something, where
> we know we're breaking the world, and then scratch all our (well thought
> out) backwards incompatible itches at once.

You preach to the choir.

That is exactly how we work and what people have been working hard for
1.7.0.  Check the planned changes listed in the recent (and not so recent)
"What's cooking" summary reports.

Changing "grep" is too late for 1.7.0, but we are trying to find an easy
migration path like you mentioned in your message and that is exactly what
this thread is about.

Re: [PATCH] grep: --full-tree

[Uri Okrent]

Junio C Hamano wrote:
> You preach to the choir.
> 
> That is exactly how we work and what people have been working hard for
> 1.7.0.  Check the planned changes listed in the recent (and not so recent)
> "What's cooking" summary reports.

Yes, I guess my only point here was that maybe even 1.7 is not enough of
a "Big Deal" (in the eyes of the public) to warrant breaking scripts. A
2.0 version would be a more visible way to say "Hey test your scripts
before upgrading". Adopting a strategy like that would mean making
backwards incompatible changes a lot let frequently, but when we do we
go for broke.

> Changing "grep" is too late for 1.7.0, but we are trying to find an easy
> migration path like you mentioned in your message and that is exactly what
> this thread is about.

I wasn't actually suggesting we change grep for 1.7. As a matter of
fact, my personal opinion (which I probably neglected to mention) is
that grep default behavior should stay the same since it is semantically
closer to unix (or gnu) grep.

-- 
    Uri

Please consider the environment before printing this message.
http://www.panda.org/how_you_can_help/

Re: [PATCH] grep: --full-tree

[Johannes Schindelin]

Hi,

On Fri, 27 Nov 2009, Jeff King wrote:

> On Fri, Nov 27, 2009 at 11:53:42AM +0100, Johannes Schindelin wrote:
> 
> > > If only somebody had written a "pager.status" configuration variable,
> > > you could use that. Oh wait. I did. And it shipped in v1.6.0.
> > 
> > And it makes things inconsistent.  That is why I do not use it. 
> 
> Then you can not use this configuration variable, too. Has the existence
> of pager.status, since you do not use it, been a problem for you so far?

No, since none of the people I helped use it.

> > Do you work on 10 different computers?  I do.  And nothing is more 
> > unnerving than the same command producing something different on the 
> > different computers.
> 
> Yes, as a matter of fact, I do work on 10 different computers. I'm sorry 
> that you find managing your configuration so challenging. But if you 
> don't use the configuration variable, then your own personal setup is 
> totally irrelevant.

As I just demonstrated, this is a false statement.

> If your argument is that this lack of consistency will irritate users,
> you need to show that:
> 
>   1. There are users who switch between a large number of setups, but
>      will not apply config consistently.

This is a strawman, and you should be ashamed to put it here.  Just 
because nobody does what you actively encourage does not mean that the 
encouraged procedure is good, or for that matter, helps anybody but you.

Just think about it.  If you plan to change the side cars are supposed to 
drive on, it is not enough to have a nice cozy committee deciding on it in 
some little room somewhere in Wyoming.  Especially not if they decide that 
you can drive on the other side if you put a sticker "I am a right-wing 
driver" on your car.

It is inconsistent, and it is violating the law of the least surprise.

> And the GitTogether had a "users complain about git, and we try to
> listen" session.

Oh, that makes me so happy.  <sarcasm>Soooo happy</sarcasm>.  So it was an 
ivory tower meeting, once again?

Ciao,
Dscho

Re: [PATCH] grep: --full-tree

[Jeff King]

On Fri, Nov 27, 2009 at 10:29:11AM -0800, Junio C Hamano wrote:

> > If only somebody had written a "pager.status" configuration variable,
> > you could use that. Oh wait. I did. And it shipped in v1.6.0.
> 
> Nice try but, "grep" and "status" are apples and oranges comparision.

Yes, I think you are right that the existence of pager.* does not
necessarily imply that there should be a config option for grep. But
that makes his example even more irrelevant: he is advocating that I use
a solution in this instance because he uses it in another instance, when
that solution is not even necessary in the other instance (and as I have
hopefully already made clear, is in my opinion inferior).

It is probably better to stay on the topic of the grep option, though.

-Peff

Re: [PATCH] grep: --full-tree

[Jeff King]

On Fri, Nov 27, 2009 at 10:47:45AM -0800, Uri Okrent wrote:

> >Changing "grep" is too late for 1.7.0, but we are trying to find an easy
> >migration path like you mentioned in your message and that is exactly what
> >this thread is about.
> 
> I wasn't actually suggesting we change grep for 1.7. As a matter of
> fact, my personal opinion (which I probably neglected to mention) is
> that grep default behavior should stay the same since it is semantically
> closer to unix (or gnu) grep.

Keeping consistency with non-git grep has been mentioned a few times in
this thread.  I really don't understand how default file selection is
supposed to maintain consistency with non-git grep. Regular grep
defaults to stdin if no paths are given. That mode doesn't make any
sense for git grep.

So of the two options (grepping the list of files from the full tree, or
the list of files rooted at the current directory), how is one closer to
non-git grep than the other?

-Peff

Re: [PATCH] grep: --full-tree

[Jeff King]

On Fri, Nov 27, 2009 at 09:07:51PM +0100, Johannes Schindelin wrote:

> > Yes, as a matter of fact, I do work on 10 different computers. I'm sorry 
> > that you find managing your configuration so challenging. But if you 
> > don't use the configuration variable, then your own personal setup is 
> > totally irrelevant.
> 
> As I just demonstrated, this is a false statement.

I must have missed where you demonstrated it.

> > If your argument is that this lack of consistency will irritate users,
> > you need to show that:
> > 
> >   1. There are users who switch between a large number of setups, but
> >      will not apply config consistently.
> 
> This is a strawman, and you should be ashamed to put it here.  Just 

How is this a strawman? A strawman would be me overstating an
exaggerated position by you and then arguing against it. All I have
claimed is that it is not sufficient for _you_ to be personally annoyed
by this existence of this option. You need to argue that there is a
significant group of people in the same situation who will be ignored.

Or have I mis-spoken in summarizing your claim that a "lack of
consistency will irritate users". Is that not your point?

> Just think about it.  If you plan to change the side cars are supposed to 
> drive on, it is not enough to have a nice cozy committee deciding on it in 
> some little room somewhere in Wyoming.  Especially not if they decide that 
> you can drive on the other side if you put a sticker "I am a right-wing 
> driver" on your car.

When the number of "git grep" crash fatalities rises above zero, maybe
this line of reasoning will be relevant.

I am talking about making software configurable so that people, in their
own private setups, can make the software work as they see fit. Yes, it
is possible for that setup to be visible to other people in some
situations. But I am arguing that we need to weigh the (in my opinion
substantial) inconvenience to users in their everyday work compared to
the inconvenience of one user sitting at another user's terminal (or
cutting and pasting commands, or running a script).

> > And the GitTogether had a "users complain about git, and we try to
> > listen" session.
> 
> Oh, that makes me so happy.  <sarcasm>Soooo happy</sarcasm>.  So it was an 
> ivory tower meeting, once again?

I don't know what to say. You complain and complain about how git is not
being responsive to users. Shawn organizes a session where people at
Google who are using git every day can try to make their complaints in
an organized forum where a bunch of developers will listen and talk
about ways we can address those complaints. And now you are mad about
that?

If you think we need a git conference where lots of users show up, I
think that's a great idea. But until you provide some suggestions about
how to organize such a thing, I don't see how you are helping anything.

-Peff

Re: [PATCH] grep: --full-tree

[Johannes Schindelin]

Hi,

On Fri, 27 Nov 2009, Jeff King wrote:

> On Fri, Nov 27, 2009 at 10:29:11AM -0800, Junio C Hamano wrote:
> 
> > > If only somebody had written a "pager.status" configuration variable,
> > > you could use that. Oh wait. I did. And it shipped in v1.6.0.
> > 
> > Nice try but, "grep" and "status" are apples and oranges comparision.
> 
> Yes, I think you are right that the existence of pager.* does not
> necessarily imply that there should be a config option for grep. But
> that makes his example even more irrelevant: he is advocating that I use
> a solution in this instance because he uses it in another instance, when
> that solution is not even necessary in the other instance (and as I have
> hopefully already made clear, is in my opinion inferior).

Sorry, no, you got it all wrong.

My point was that your config option introduced something _BAD_.  And my 
point was that now, as a consequence of having managed to put it into Git, 
you want more of such bad stuff.

You continue to ignore that inconsistency -- even if it is introduced with 
the best of all intentions -- is bad, bad, bad.

But I guess that I continue to get ignored,
Dscho

Re: [PATCH] grep: --full-tree

[Johannes Schindelin]

Hi,

On Fri, 27 Nov 2009, Jeff King wrote:

> On Fri, Nov 27, 2009 at 09:07:51PM +0100, Johannes Schindelin wrote:
> 
> > > Yes, as a matter of fact, I do work on 10 different computers. I'm sorry 
> > > that you find managing your configuration so challenging. But if you 
> > > don't use the configuration variable, then your own personal setup is 
> > > totally irrelevant.
> > 
> > As I just demonstrated, this is a false statement.
> 
> I must have missed where you demonstrated it.

Usually, my mails are minimal, and I do not write as many mails as I 
used to anymore, so it is hard to miss what I am saying.

For your benefit: both Junio and me talked about experts helping users.  
Even if I do not use the config options, I am affected.  And it does hurt.

> > > If your argument is that this lack of consistency will irritate users,
> > > you need to show that:
> > > 
> > >   1. There are users who switch between a large number of setups, but
> > >      will not apply config consistently.
> > 
> > This is a strawman, and you should be ashamed to put it here.  Just 
> 
> How is this a strawman?

You are comparing config settings which must be different, because they 
affect _what_ project you are working with, with config settings that 
affect _how_ you can work with them.

> When the number of "git grep" crash fatalities rises above zero, maybe 
> this line of reasoning will be relevant.

Sure.  Let's wait for the first crash fatality, and only react then.  No 
need to think ahead.

That's it.  I don't think that I want to participate in this kind of 
discussion anymore,
Dscho

Re: [PATCH] grep: --full-tree

[Felipe Contreras]

On Thu, Nov 26, 2009 at 1:22 AM, Jeff King <peff@peff.net> wrote:
> Probably we would want something flexible, but with sane defaults. Like
> an environment variable to ignore all (or most) config options, but then
> the ability to opt into specific ones. Something like:
>
>  GIT_PLUMBING=1; export GIT_PLUMBING
>  git log ;# does not respect any non-plumbing config
>  git --respect='log.showroot' ;# respect just the one variable
>  git --respect='color.*' log ;# you get all color
>
> But there are two big obstacles (besides the obvious issue that
> introducing this in itself needs a gentle transition plan):
>
>  1. We need to annotate every config option with whether it is
>     potentially problematic. For example, core.filemode should probably
>     be respected no matter what (but I'm not sure if it is simply true
>     for core.*).
>
>  2. Script writers need to actually use the system, which is somewhat
>     more verbose and annoying than what they have to do now. But at
>     least it defaults to safety when they are lazy, and then they can
>     re-add options. Of course, they are stuck on an upgrade treadmill
>     of analyzing and approving each new option that appears in git.

+1 on this.

This would make it easier to add options in the future that would be
potentially dangerous to scripts otherwise. But more than
"non-plumbing" I would rather define these variables as *preferences*;
things that are not essential to the proper functioning of git
commands, and would vary from user to user.

-- 
Felipe Contreras

Re: [PATCH] grep: --full-tree

[Felipe Contreras]

On Fri, Nov 27, 2009 at 8:29 PM, Junio C Hamano <gitster@pobox.com> wrote:
> That is exactly how we work and what people have been working hard for
> 1.7.0.  Check the planned changes listed in the recent (and not so recent)
> "What's cooking" summary reports.
>
> Changing "grep" is too late for 1.7.0, but we are trying to find an easy
> migration path like you mentioned in your message and that is exactly what
> this thread is about.

How about this. For now, make --full-tree available, and that's it.
Then, on 1.8.0, add the configuration option, and if there's consensus
make it default (-1 from me).

However, in order to ease the transition I think Jeff's GIT_PLUMBING
(I would call it GIT_SCRIPTING) should be introduced on 1.7.0 so
people can start exporting it in their scripts. That way people don't
have to worry about adding --(no-)full-tree on their scripts (nor any
other argument that depends on configurations) and when 1.8.0 comes,
there's no script breakage.

-- 
Felipe Contreras

Re: [PATCH] grep: --full-tree

[Felipe Contreras]

On Fri, Nov 27, 2009 at 10:07 PM, Johannes Schindelin
<Johannes.Schindelin@gmx.de> wrote:
> On Fri, 27 Nov 2009, Jeff King wrote:
>> Yes, as a matter of fact, I do work on 10 different computers. I'm sorry
>> that you find managing your configuration so challenging. But if you
>> don't use the configuration variable, then your own personal setup is
>> totally irrelevant.
>
> As I just demonstrated, this is a false statement.

Yes, defaults are important in UI.

>> If your argument is that this lack of consistency will irritate users,
>> you need to show that:
>>
>>   1. There are users who switch between a large number of setups, but
>>      will not apply config consistently.
>
> This is a strawman, and you should be ashamed to put it here.  Just
> because nobody does what you actively encourage does not mean that the
> encouraged procedure is good, or for that matter, helps anybody but you.

Not to mention that it's completely irrelevant. The fact that all
users apply their configurations consistently through their setups, or
not, doesn't make a default preference better or worst.

If the argument is that default preferences are not relevant enough,
then step aside and let the people that care about default preferences
to discuss.

>> And the GitTogether had a "users complain about git, and we try to
>> listen" session.
>
> Oh, that makes me so happy.  <sarcasm>Soooo happy</sarcasm>.  So it was an
> ivory tower meeting, once again?

This is very typical on many open source projects. I think the
benevolent dictator model works pretty good on low-level stuff, but on
UI I think a democratic model works better.

I've been thinking on setting up a pseudo-project on SourceForge and
setup an IdeaTorrent, that way users can generate and organize ideas
so that developers can have meaningful conversations with users:
http://brainstorm.ubuntu.com/

What do you think?

-- 
Felipe Contreras

Re: [PATCH] grep: --full-tree

[Jeff King]

On Sun, Nov 29, 2009 at 11:21:06AM +0100, Johannes Schindelin wrote:

> You continue to ignore that inconsistency -- even if it is introduced with 
> the best of all intentions -- is bad, bad, bad.
> 
> But I guess that I continue to get ignored,

Speaking of ignoring, you have never once responded to my repeated point
that I agree that inconsistency is bad, but it is about weighing that
bad against other bad things.

-Peff

Re: [PATCH] grep: --full-tree

[Jeff King]

On Sun, Nov 29, 2009 at 11:28:27AM +0100, Johannes Schindelin wrote:

> > > > Yes, as a matter of fact, I do work on 10 different computers. I'm sorry 
> > > > that you find managing your configuration so challenging. But if you 
> > > > don't use the configuration variable, then your own personal setup is 
> > > > totally irrelevant.
> > > 
> > > As I just demonstrated, this is a false statement.
> > 
> > I must have missed where you demonstrated it.
> 
> Usually, my mails are minimal, and I do not write as many mails as I 
> used to anymore, so it is hard to miss what I am saying.
> 
> For your benefit: both Junio and me talked about experts helping users.  
> Even if I do not use the config options, I am affected.  And it does hurt.

A point which I adressed in my numbered point (2) in the mail you are
quoting above. But you didn't bother to quote that part.

> > When the number of "git grep" crash fatalities rises above zero, maybe 
> > this line of reasoning will be relevant.
> 
> Sure.  Let's wait for the first crash fatality, and only react then.  No 
> need to think ahead.

You missed my point. My point is that your analogy had many
characteristics that do not apply to this situation. You are comparing a
situation where somebody's preference (to drive on the left side or the
right side) is weighed against a system where everyone needs to follow
the same rule, or people will die in large numbers. The actual situation
at hand is a git grep configuration variable. I am weighing the
preference of people who use git every day and want it to work in a
certain way against the possibility that somebody helping them will be
slightly inconvenienced or surprised. Something that will happen much
less frequently than the person actually _using_ git, and something
which has much smaller negative consequences than people dying.

> That's it.  I don't think that I want to participate in this kind of 
> discussion anymore,

Fine. I have made my point over and over, and not once have you
responded to it directly, so I also feel this is going nowhere.

-Peff

Re: [PATCH] grep: --full-tree

[Uri Okrent]

Felipe Contreras wrote:
> On Thu, Nov 26, 2009 at 1:22 AM, Jeff King <peff@peff.net> wrote:
>> Probably we would want something flexible, but with sane defaults. Like
>> an environment variable to ignore all (or most) config options, but then
>> the ability to opt into specific ones. Something like:
>>
>>  GIT_PLUMBING=1; export GIT_PLUMBING
>>  git log ;# does not respect any non-plumbing config
>>  git --respect='log.showroot' ;# respect just the one variable
>>  git --respect='color.*' log ;# you get all color
>>
>> But there are two big obstacles (besides the obvious issue that
>> introducing this in itself needs a gentle transition plan):
>>
>>  1. We need to annotate every config option with whether it is
>>     potentially problematic. For example, core.filemode should probably
>>     be respected no matter what (but I'm not sure if it is simply true
>>     for core.*).
>>
>>  2. Script writers need to actually use the system, which is somewhat
>>     more verbose and annoying than what they have to do now. But at
>>     least it defaults to safety when they are lazy, and then they can
>>     re-add options. Of course, they are stuck on an upgrade treadmill
>>     of analyzing and approving each new option that appears in git.
> 
> +1 on this.
> 
> This would make it easier to add options in the future that would be
> potentially dangerous to scripts otherwise. But more than
> "non-plumbing" I would rather define these variables as *preferences*;
> things that are not essential to the proper functioning of git
> commands, and would vary from user to user.
> 

Sounds like a good idea to me, speaking as someone who has git support
scripts. Dealing with configuration soup in every script would be very
bad.

The same type of insulation though could probably be achieved by not
adding configuration options that alter the behavior of commands,
and instead have user's rely on aliases for that purpose. (The cat's
probably already out of the bag WRT to configuration though).

-- 
    Uri

Please consider the environment before printing this message.
http://www.panda.org/how_you_can_help/

Re: [PATCH] grep: --full-tree

[Junio C Hamano]

Jeff King <peff@peff.net> writes:

> On Sun, Nov 29, 2009 at 11:28:27AM +0100, Johannes Schindelin wrote:
> ...
>> > When the number of "git grep" crash fatalities rises above zero, maybe 
>> > this line of reasoning will be relevant.
>> 
>> Sure.  Let's wait for the first crash fatality, and only react then.  No 
>> need to think ahead.
>
> ... The actual situation
> at hand is a git grep configuration variable. I am weighing the
> preference of people who use git every day and want it to work in a
> certain way against the possibility that somebody helping them will be
> slightly inconvenienced or surprised.

While my position is *not* "hurting people who help is too grave and we
shouldn't even weigh other upsides against it---bad is bad is bad, and it
is absolutely bad" (which is what I think Dscho is saying), I think
"slightly inconvenienced or surprised" is trying to make it sound a lot
lighter than it is.

Imagine you are helping somebody to track down a bug in a project whose
source happens to be under git.  You two scratch your heads together, and
you try to find if the function you are fixing have other call sites, and
you run "git grep" to find them.  You think you covered the whole tree,
identified all the callsites and made sure that the updated behaviour of
the function with your fix is consistent with all of them.  But it turns
out that you didn't check the whole tree, due to user's configuration, and
you didn't notice.

You can easily waste 30 minutes of two people until you realize what
happened.  Because the whole point of your grep.fulltree configuration is
that you can set it once and forget about it, even after you noticed that
your grep didn't look in the whole tree as you expected, the configuration
variable is not the first thing that will come to your mind.  You will
waste more minutes wondering why grep is not working as you expect, until
you finally come up with a suggestion to set the configuration to make
grep look in the full tree by default in her repository.

Put it another way, your "I can set it and forget about it" may be a way
to solve "differentiating two things is a mental burden and I do not want
to think about it".  But I do not think the "mental burden" problem is
necessarily what we want to solve.  The "set and forget" will bring
confusion.

The best solution to the "mental burden" problem may not even be "I can
set it and forget about it".  An obvious solution to that problem, that is
far easier to explain, is not to have two things to begin with, and that is
what we do: "If you want to grep in the whole tree, you go to the top and
run grep there."  Of course, its downside is that it is often cumbersome
to "got to the top" when you are somewhere deep.

That is why I think it would be a lot better solution to spend our efforts
making sure that both semantics can be called for from the command line in
a concise and clear way.  IOW, the problem I see worth solving first is
not the "mental burden" problem, but is "differentiating two things is
necessary, but it is cumbersome to say which one I want."

You probably can add both configuration and concise command line syntax,
but "solving" the "mental burden" problem will make you forget about the
need to use --full-tree option (or its quivalent that will happen in the
solution of the "cumbersome to say which one I want" problem).  On the
other hand, not "solving" the "mental burden" problem will hopefully train
your brain and your fingers to always be aware of and to say which one you
want, to the point that you do not even have to think.

For that to happen, "cumbersome to say which" problem must be solved
nicely, of course.

> ... Something that will happen much
> less frequently than the person actually _using_ git, and something
> which has much smaller negative consequences than people dying.

It is of course not _fatal_, but there are not many things that are fatal.
Saying "that is not fatal so it is Ok" is not particularly a good way to
weigh downsides against upsides.

Re: [PATCH] grep: --full-tree

[Uri Okrent]

Jeff King wrote:
> On Fri, Nov 27, 2009 at 10:47:45AM -0800, Uri Okrent wrote:
>> As a matter of
>> fact, my personal opinion (which I probably neglected to mention) is
>> that grep default behavior should stay the same since it is semantically
>> closer to unix (or gnu) grep.
> 
> Keeping consistency with non-git grep has been mentioned a few times in
> this thread.  I really don't understand how default file selection is
> supposed to maintain consistency with non-git grep. Regular grep
> defaults to stdin if no paths are given. That mode doesn't make any
> sense for git grep.
> 
> So of the two options (grepping the list of files from the full tree, or
> the list of files rooted at the current directory), how is one closer to
> non-git grep than the other?
> 
> -Peff

I guess you're right, in that neither is exactly the same as non-git,
and so it's impossible to objectively quantify how one is "closer". My
general feeling though is that grep rooted at the current directory is
more similar because grep -r does exist and is common enough that the
layman isn't too surprised at git's default behavior. Git grep with
--full-tree though, has no analogue in non-git grep.

-- 
    Uri

Please consider the environment before printing this message.
http://www.panda.org/how_you_can_help/