From: Jean-Noël AVILA <jn.avila@free.fr>
Date: Sat, 09 May 2026 12:46:26 GMT
Subject: Re: [PATCH v6] revision.c: implement --max-count-oldest
Message-ID: <2409449.ElGaqSPkdT@piment-oiseau>
In-Reply-To: <ce8d1ff49ef418ae3720265a124ef53a959d289e.1778017966.git.mroik@delayed.space>

On Tuesday, 5 May 2026 23:54:56 CEST Mirko Faina wrote:
> --max-count is a commit limiting option sets a maximum amount of commits
> to be shown. If a user wants to see only the first N commits of the
> history (the oldest commits) they'd have to do something like
> 
>     git log $(git rev-list HEAD | tail -n N | head -n 1)
> 
> This is not very user-friendly.
> 
> Teach get_revision() the --max-count-oldest option.
> 
> Signed-off-by: Mirko Faina <mroik@delayed.space>
> ---
> Since v5 I've reworded the commit message and rewrote the docs for
> --max-count-oldest to be clearer on its functionality.
> 
>  Documentation/rev-list-options.adoc |  5 ++
>  revision.c                          | 77 +++++++++++++++++++++++++++--
>  revision.h                          |  2 +
>  t/t4202-log.sh                      | 14 ++++++
>  4 files changed, 95 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/rev-list-options.adoc
> b/Documentation/rev-list-options.adoc index 2d195a1474..9f857cabcc 100644
> --- a/Documentation/rev-list-options.adoc
> +++ b/Documentation/rev-list-options.adoc
> @@ -18,6 +18,11 @@ ordering and formatting options, such as `--reverse`.
>  `--max-count=<number>`::
>     Limit the output to _<number>_ commits.
> 
> +`--max-count-oldest=<number>`::
> +   Just like `--max-count=<number>`, it limits the output to _<number>_
> +   commits. But instead of limiting to the first _<number>_ commits it
> +   limits to the last _<number>_ commits.
> +

Putting aside the discussion of --max-count=<neg-value> vs --max-count-
oldest=<value>, I do not think that defining --max-count-old with respect with 
--max-count is legible. It would be better to refine the definition of --max-
count (i.e. "Limit the output to the _<number>_ first commits") and just 
define --max-count-oldest on its own in the same manner. Referring to another 
entry is only practicable when it avoids repeating a long explanation. 
Otherwise, each entry's explanation should be as self-contained as possible.

>  `--skip=<number>`::
>     Skip _<number>_ commits before starting to show the commit output.
> 
> diff --git a/revision.c b/revision.c
> index 599b3a66c3..3aaa77ced5 100644
> --- a/revision.c
> +++ b/revision.c
> @@ -2339,10 +2339,24 @@ static int handle_revision_opt(struct rev_info 
*revs, int
> argc, const char **arg }
> 
>     if ((argcount = parse_long_opt("max-count", argv, &optarg))) {
> +       if (revs->max_count_type == 1)
> +           die(_("can't use --max-count with --max-count-oldest"));
>         revs->max_count = parse_count(optarg);
>         revs->no_walk = 0;
> +       revs->max_count_type = 0;
>         return argcount;
> +   } else if ((argcount = parse_long_opt("max-count-oldest", argv, 
&optarg))) {
> +       if (revs->max_count_type == 0 && revs->max_count != -1)
> +           die(_("can't use --max-count with --max-count-oldest"));
> +       if (revs->skip_count > 0)
> +           die(_("con't use --max-count-oldest with --skip"));

Typo here (con't → can't). In any case, please prefer 
die_for_incompatible_opt2, to uniformize the messages and limit the number of 
translation strings.

Adding a test for these incompatibilities would be great too.

> +       revs->max_count = parse_count(optarg);
> +       revs->no_walk = 0;
> +       revs->max_count_type = 1;
> +       revs->max_count_stage = 0;
>     } else if ((argcount = parse_long_opt("skip", argv, &optarg))) {
> +       if (revs->max_count_type == 1)
> +           die(_("con't use --max-count-oldest with --skip"));

ditto

>         revs->skip_count = parse_count(optarg);
>         return argcount;
>     } else if ((*arg == '-') && isdigit(arg[1])) {
> @@ -4521,15 +4535,68 @@ static struct commit *get_revision_internal(struct 
rev_info
> *revs) return c;
>  }
>