From: Mirko Faina <mroik@delayed.space>
Date: Sun, 10 May 2026 00:41:18 GMT
Subject: Re: [PATCH v6] revision.c: implement --max-count-oldest
Message-ID: <af_Td41Mga0Z-MMv@exploit>
In-Reply-To: <2409449.ElGaqSPkdT@piment-oiseau>

On Sat, May 09, 2026 at 02:46:26PM +0200, Jean-Noël AVILA wrote:
> 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.

Will do in v7.

> >  `--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.

Will do.

> Adding a test for these incompatibilities would be great too.

Yes, should've done that sooner. Will do.

> > +       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

Will do.

> >         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;
> >  }
> > 

Thank you