Re: [PATCH 12/12] Add git-check-ignore sub-command

From: Adam Spiers <git@adamspiers.org>
Date: Dec 17, 2012, 00:10 UTC
Message-ID: <20121217001013.GB3673@gmail.com>
In-Reply-To: <7vmwzmtmyd.fsf@alter.siamese.dyndns.org>

On Tue, Oct 16, 2012 at 09:12:58AM -0700, Junio C Hamano wrote:

Show 49 quoted lines

> Adam Spiers <git@adamspiers.org> writes:
> > On Mon, Oct 15, 2012 at 3:31 PM, Junio C Hamano <gitster@pobox.com> wrote:
> >> Nguyễn Thái Ngọc Duy  <pclouds@gmail.com> writes:
> >>> +For each pathname given via the command-line or from a file via
> >>> +`--stdin`, this command will list the first exclude pattern found (if
> >>> +any) which explicitly excludes or includes that pathname.  Note that
> >>> +within any given exclude file, later patterns take precedence over
> >>> +earlier ones, so any matching pattern which this command outputs may
> >>> +not be the one you would immediately expect.
> >>
> >> "The first exclude pattern" is very misleading, isn't it?
> >
> > I don't think so, because of the second sentence.
> >
> >> For example, with these in $GIT_DIR/info/exclude, I would get:
> >>
> >>         $ cat -n .git/info/exclude
> >>           1 *~
> >>           2 Makefile~
> >>         $ git check-ignore -v Makefile~
> >>         .git/info/exclude:2:Makefile~   Makefile~
> >>
> >> which is the correct result (the last one in a single source decides
> >> the fate of the path), but it hardly is "first one found" and the
> >> matching pattern in the output would not be something unexpected for
> >> the users, either.
> >>
> >> The reason it is "the first one found" is because the implementation
> >> arranges the loop in such a way that it can stop early when it finds
> >> a match---it simply checks matches from the end of the source.
> >>
> >> But that is not visible to end-users,
> >
> > Correct; that's precisely why I wrote the second sentence which
> > explicitly explains this.
> >
> >> and they will find the above description just wrong, no?
> >
> > It's not wrong AFAICS, but suggestions for rewording this more clearly
> > are of course welcome.  Maybe s/immediately/intuitively/ ?
> 
> I think this is sufficient:
> 
> 	For each pathname given via the command-line or from a file
> 	via `--stdin`, show the pattern from .gitignore (or other
> 	input files to the exclude mechanism) that decides if the
> 	pathname is excluded.
> 
> and without "Note that" at all.

I don't think this is quite sufficient. Firstly, it does not cover negated patterns (my original text contained "or includes").

Secondly, I think there is still potential for this rewording to result in confused users. If the "backwards-ness" of the internal algorithm is kept hidden from them, then in your example above, most users would be more likely to intuitively expect check-ignore to return the first line of .git/info/exclude ("*~"). When they saw it returning the second, they might draw the conclusion that the first line failed to match (e.g. by mistakenly thinking that the file format requires regular expressions rather than globs), rather than that git starts at the end of the file.

This is precisely why I chose not to hide this aspect of the implementation when initially writing this documentation. Unfortunately my wording managed to confuse several of you, so clearly it was not adequate. Therefore I propose an extension of your version:

	For each pathname given via the command-line or from a file
	via `--stdin`, show the pattern from .gitignore (or other
	input files to the exclude mechanism) that decides if the
	pathname is excluded or included.  Later patterns within a
	file take precedence over earlier ones.