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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Oct 16, 2012 at 09:12:58AM -0700, Junio C Hamano wrote:
> Adam Spiers <git@xxxxxxxxxxxxxx> writes:
> > On Mon, Oct 15, 2012 at 3:31 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> >> Nguyễn Thái Ngọc Duy  <pclouds@xxxxxxxxx> 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.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]