Re: [PATCH v6] revision.c: implement --max-count-oldest
On Sat, May 09, 2026 at 02:46:26PM +0200, Jean-Noël AVILA wrote:
Show 43 quoted lines
> 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.
Show 28 quoted lines
> > `--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.
Yes, should've done that sooner. Will do.
Show 9 quoted lines
> > + 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"));
>
> dittoShow 8 quoted lines
> > 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;
> > }
> >