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

From: Junio C Hamano <gitster@pobox.com>
Date: Feb 22, 2010, 20:21 UTC
Message-ID: <7viq9paunm.fsf@alter.siamese.dyndns.org>
In-Reply-To: <20100222105203.GB16531@coredump.intra.peff.net>

Jeff King <peff@peff.net> writes:

Show 13 quoted lines

> On Sun, Feb 21, 2010 at 04:19:18PM -0800, Junio C Hamano wrote:
>
>> * 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...

Show 5 quoted lines

>> 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.