Hi, Just a couple of questions. On Tuesday, June 21st, 2022 at 18:27, Junio C Hamano <gitster@xxxxxxxxx> wrote: > "Carlos L. via GitGitGadget" gitgitgadget@xxxxxxxxx writes: > > > From: =?UTF-8?q?Carlos=20L=C3=B3pez?= 00xc@xxxxxxxxxxxxxx > > > > This patch adds a command line option analogous to that of GNU > > grep(1)'s -m / --max-count, which users might already be used to. > > This makes it possible to limit the amount of matches shown in the > > output while keeping the functionality of other options such as -C > > (show code context) or -p (show containing function), which would be > > difficult to do with a shell pipeline (e.g. head(1)). > > > > Signed-off-by: Carlos López 00xc@xxxxxxxxxxxxxx > > --- > > ... > > Documentation/git-grep.txt | 8 ++++++++ > > builtin/grep.c | 9 +++++++++ > > grep.c | 2 ++ > > grep.h | 2 ++ > > 4 files changed, 21 insertions(+) > > > Tests? Right. Is it OK if I include my test(s) in t/t7810-grep.sh, or should it be a different/new file? > > diff --git a/grep.c b/grep.c > > index 82eb7da1022..b32ab75cb6b 100644 > > --- a/grep.c > > +++ b/grep.c > > @@ -1686,6 +1686,8 @@ static int grep_source_1(struct grep_opt *opt, struct grep_source *gs, int colle > > bol = eol + 1; > > if (!left) > > break; > > + if (opt->max_count != -1 && count == opt->max_count) > > + break; > > > I would have written it "if (0 <= opt->max_count && ...)". What > > happens when a trickster asks you to do "git grep -m -2"? Fair enough. Since it's already optimized out above, is there any reason we need to include zero (<=)? > I guess what I am getting at is if we are better off saying that > negative means unlimited, instead of special casing -1 like this. I > didn't think it through so it may be perfectly possible that what > you wrote makes more sense than "anything negative is unlimited". > > I dunno. I think you're right, I'll adjust my patch. Best, Carlos
