Re: What's cooking in git.git (Feb 2010, #05; Sun, 21)

Jeff King [off-list ref] writes:

On Sun, Feb 21, 2010 at 04:19:18PM -0800, Junio C Hamano wrote:

quoted

* cp/add-u-pathspec (2010-02-09) 2 commits
 - test for add with non-existent pathspec
 - git add -u: die on unmatched pathspec

I am a bit torn on this one.  Traditionally we never complained on
unmatched pathspec when talking about tracked files.  If we were to go

True, though most of those pathspecs for tracked files are when viewing
diffs. It seems more inconsistent here because "git add foo" complains
but "git add -u foo" does not. So I think this one is definitely worth
fixing.

One problem is that it would be adding a new inconsistency.

"git diff" does not complain but "git add -u" will complain if we make
this change, but "add -u" is about updating the path that "git diff"
reports as different.

That is why you would need to make "git diff" and friends _also_ complain,
if we want to add a new consistency between "add" and "add -u" without
breaking the existing consistency between "diff" and "add -u".  I do not
have a problem with making "diff" also complain for an unmatched pathspec
as a longer term direction, but we need to be careful (e.g. How should
this interact with "git log -- pathspec"?)

In any case, teaching "diff-files" about unmatched pathspec warning would
necessitate infrastructure change...

quoted

this route, I think we should probably enhance the "run_diff_files" and
friends in such a way that they mark matched pathspecs, in a way similar
to match_pathspec() in dir.c does, and report unmatched ones based on
that result, instead of adding an extra pass to scan the index.  The same
goes for pathspec_matches() in builtin-grep.c

Once we have an infrastructure for "diff-files" to notice an unmatching
pathspec, "add -u" will notice it, too, without any extra code.

Making "add -u" complain before fixing "diff-files" will have another
issue.  It will expose a bigger inconsistency that you omitted from my
message ;-) The pathspec "git add" without "-u" takes are processed by
pathspec match logic of "ls-files" family, but "git add -u" uses pathspec
match logic of "diff" family.  They have different semantics.

You can say "git add 'frotz/*.c'" but not "git add -u 'frotz/*.c'"; that
also needs to be fixed.

Making "add -u" alone complain using a separate throw-away logic that we
are sure we will have to discard when we make things consistent throughout
the system did not sound very attractive to me.  And that is why I was
unhappy about the solution.

On Mon, Feb 22, 2010 at 12:21:33PM -0800, Junio C Hamano wrote:

quoted

True, though most of those pathspecs for tracked files are when viewing
diffs. It seems more inconsistent here because "git add foo" complains
but "git add -u foo" does not. So I think this one is definitely worth
fixing.

One problem is that it would be adding a new inconsistency.

"git diff" does not complain but "git add -u" will complain if we make
this change, but "add -u" is about updating the path that "git diff"
reports as different.

I see. Personally I don't mind that inconsistency as much, as it is
between two commands, rather than between flags within one command. But
that is perhaps a subjective evaluation.

But:

Making "add -u" complain before fixing "diff-files" will have another
issue.  It will expose a bigger inconsistency that you omitted from my
message ;-) The pathspec "git add" without "-u" takes are processed by
pathspec match logic of "ls-files" family, but "git add -u" uses pathspec
match logic of "diff" family.  They have different semantics.

You can say "git add 'frotz/*.c'" but not "git add -u 'frotz/*.c'"; that
also needs to be fixed.

That is a more worrisome inconsistency to me (and now I get what you
were saying in your earlier message).

Making "add -u" alone complain using a separate throw-away logic that we
are sure we will have to discard when we make things consistent throughout
the system did not sound very attractive to me.  And that is why I was
unhappy about the solution.

OK, now I am unhappy about it, too, and I agree it should be addressed
in the long term. But that is a large-ish project that will not happen
immediately. What is the best thing in the meantime?

I am still tempted by the patch. Even though it trades one inconsistency
for another, I find a false sense of success from "git-add" to be one of
the more ugly errors (and even though "git add -u 'frotz/*.c'" would
still not work with it, at least you would be informed of such).

-Peff