On Fri, Apr 30, 2021 at 2:57 AM Jeremy Faith <jeremy.faith@xxxxxxx> wrote: > > On 30 April 2021 01:22 Junio C Hamano <gitster@xxxxxxxxx> wrote: > > >Jeremy Faith <jeremy.faith@xxxxxxx> writes: > > > >> man git-check-ignore states:- > >> EXIT STATUS > >> ----------- > >> 0:: > >> One or more of the provided paths is ignored. > >> 1:: > >> None of the provided paths are ignored. > >> 128:: > >> A fatal error was encountered. > >> > >> So my change matches what the manual states. > > >That is part of what I meant by "understandable, given the history". > > >The above description is _wrong_ ever since it was introduced, along > >with check-ignore, at 368aa529 (add git-check-ignore sub-command, > >2013-01-06). It should have said "has/have entry that affects it in > >the gitignore/exclude files" instead of "is/are ignored". After > >all, that is what the tool was written to do, i.e. to help debugging > >the gitignore/exclude files. > > > > git-check-ignore(1) > > =================== > > > > NAME > > ---- > > git-check-ignore - Debug gitignore / exclude files > > > >Having said all that. > > > >It is a misunderstanding that check-ignore is a tool to learn if a > >path is or is not ignored, but the misunderstanding is so widespread. > > > >I wonder if we can repurpose the command to "match" the > >misunderstanding, without hurting existing users, by > > > > (1) updating the one-liner description of the command in the > > documentation; > > > (2) keep the EXIT STATUS section as-is; and > > > > (3) adjust the code to exit with status that reflects if there was > > at least one path that was ignored (not "that had an entry in > > the gitignore/exclude files that affected its fate"). > > If I understand correctly:- > (2) requires no change > (3) I believe my one line change does this > > Which just leaves (1) where current line is > git-check-ignore - Debug gitignore / exclude files > I think with the exit status operating as documented this description still > works i.e. check-ignore can be used to test if the .gitignore/exclude files > are working as desired. > > The longer Description is: > DESCRIPTION > For each pathname given via the command-line or from a file via > --stdin, check whether the file is excluded by .gitignore (or other > input files to the exclude mechanism) and output the path if it is > excluded. > Which matches the exit status meaning as is. > > >That certainly is a backward compatible change, but I suspect that > >we may be able to sell it as a bugfix, taking advantage of the > >documentation bug you quoted above. Of course, people do not read > >documentation, so scripts that used to use the command in the way it > >was intended to be used (as opposed to "the way it was documented") > >will still get broken with such a change, though. > > I'm not sure how the old exit status could be used in a useful way but you are > correct there is a chance that some existing scripts depend on it. > > I was originally confused by the exit status when using git versions 1.8.3.1 > and 2.25.1. With these versions check-ignore returned 0 when a matching > pattern was found, it did not matter if it was a positive or negative pattern. > This did not match the exit status documented in the man page so I thought my > .gitignore patterns were not working when they were. > Perhaps I should stop reading man pages... > > I then tried the same patterns on an up to date version of git built from a > a git clone and found that without -v it returned the exit status as the man > page states but with -v the older exit status was given. > The commit that changed the exit status was 7ec8125 from 2020-02-18, first > release that included this commit was 2.26. > > So in 2.26 the exit status without -v was changed, maybe accidentally, in a > way that made it match the man page. But this commit broke scripts(that > do not use -v) that depend on exit status. My change extends the breakage to > scripts that do use -v. On the other hand scripts written that expect the > exit status to match the man page will be fixed! > > I'm not sure what the best course of action is but at least with my change > the exit status matches the man page(even with -v). I'm inclined to agree with Jeremy here. Commit 7ec8125fba96 (check-ignore: fix documentation and implementation to match, 2020-02-18) demonstrated pretty clearly that check-ignore was just flat-out broken since the beginning in regards to negated patterns, in multiple ways: wrong exit status, documentation that was self-contradictory, and non-matching documentation and implementation. Among other things, that commit from last year fixed the exit status without -v. Unfortunately, when I (or the users I was interacting with) use -v, I ignore the exit status and am just looking for the output. So I missed the exit status inconsistency for -v when I created that commit and only fixed the exit status without -v.
