Jeremy Faith <jeremy.faith@xxxxxxx> writes: >>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. As an end-user facing command, we'd probably want to align this more with the description of check-attr. I think that the description as you quoted is OK 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 have a suspicion that the existing tests may depend on the current "we found a match" semantics---they may not count as "existing scripts". We'd need to update them if that is the case. > 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. Yup, that is understandable as the manpage was utterly wrong, and its description was so plausible that nobody noticed. As I already said, I think it is OK to sell this change as a bugfix to align implementation with the documentation wrt negative results; in addition to the 3 item list quoted at the top, we might need to adjust tests (or add tests if we are not covering the "not excluded because we explicitly have a negative entry" case). Thanks.
