Re: [PATCH] grep: add --max-count command line option

[Junio C Hamano <gitster@pobox.com>]

"Carlos L. via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: =?UTF-8?q?Carlos=20L=C3=B3pez?= <00xc@protonmail.com>

Offtopic, but I wonder why this line is encoded like so?  The
"Signed-off-by:" line is not, and it is safely transmitted, so
it feels like we do not need to encode the in-body header that
is added only for e-mail but not in the original commit...

> 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@protonmail.com>
> ---
> ...
> +-m <num>::
> +--max-count <num>::
> +	Limit the amount of matches per file. When using the -v or
> +	--invert-match option, the search stops after the specified
> +	number of non-matches. Setting this option to 0 has no effect.
> +

Good thing that this is defined as "per-file" limit.  If it were a
global limit, the interaction between this one and "--threads=<num>"
would have been interesting.  Perhaps add a test to make sure the
feature continues to work with "--threads=2" (I am assuming that you
have already tested this implementation works with the option).

Martin already commented on the wording "no effect"; I agree it is a
poor choice of words from the point of view of "overriding with 0".

It indeed is curious why GNU grep chose to immediately exit with 1
when "-m 0" was given, but that was decision made more than 20 years
ago (http://gnu.ist.utl.pt/software/grep/changes.html and look for
"2000-03-17").  Between "being consistent even with a seemingly
useless design choice made by somebody else" and "choose to be
different in a corner case where nobody should care and allow us to
be more useful", I am slightly in favor in this particular case.

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.

Thanks.