"Carlos L." <00xc@xxxxxxxxxxxxxx> writes: >> Even if we want to handle the zero just like you do, I think this patch >> needs a few tests. We should make sure to test the 0-case (whatever we >> end up wanting it to behave like), and probably the "suppress an earlier >> -m by giving --no-max-count" case. It also seems wise to set up some >> test scenario where there are several files involved so that we can see >> that we don't just print the first m matches globally, but that the >> counter is really handled per file. > > This seems sound. Is there any documentation on how to write tests for git? t/README and Documentation/MyFirstContribution would be two good places to start. >> What "git grep -m -1" should do? IIRC, OPT_INTEGER is for signed >> integer but the new .max_count member, as well as the existing >> "count" that is compared with it, are of "unsigned" type. Either >> erroring out or treating it as unlimited is probably fine, but >> whatever we do, we should document and have a test for it. > > I would favor treating it as an error. As mentioned above, using 0 > to describe "unlimited matches" (e.g. the default) is my > preference, but I am willing to concede if someone can think of a > good use for `-m 0`. With Devil's advocate hat on. "GNU grep has been doing so for the past 20 years and existing users of the command expects '-m 0' to behave that way" is a good enough reason, especially if '-m 0' is not the only possible way to say "unlimited". > Also, from the implementation side (although > not as important) it looks better: if we allow negative values, we > need to distinguish between -1 (unlimited) and -4 (display error > to user, probably) If we are going to document "you can pass a negative value to explicitly say 'unlimited', which is a useful way to countermand another `-m <num>` that appear earlier on the command line", then -1 and -4 would equally be 'unlimited' and there is no need to distinguish anything. Devil's advocate hat off. I personally do not mind if "-m <non-positive>" means "unlimited", as long as that is clearly documented and tested, but for long time "GNU grep" users "-m 0" might appear surprising (not necessarily because they would find that the "-m 0" that immediately fails is useful, but because the behaviour is deliberately made different). Thanks.
