Re: [PATCH v6] revision.c: implement --max-count-oldest

> > +`--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.
> > +
> 
> "Just like --max-count" is a surprising addendum in this sentence,
> because the only thing they have in common is the limiting of commits,
> which it repeats anyway. It's more like "Unlike --max-count, limits the
> output to _<number>_ last commits."

> >  `--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"));
> 
> To help translators, the usual pattern is to say (here and later)
> 
> 	die(_("options '%s' and '%s' cannot be used together"),
> 		"--max-count", "--max-count-oldest");

> > @@ -4521,15 +4535,68 @@ static struct commit *get_revision_internal(struct rev_info *revs)
> >     return c;
> >  }
> >  
> > +static void retrieve_oldest_commits(struct rev_info *revs,
> > +                   struct commit_list **queue)
> > +{
> > +   struct commit *c;
> > +   int max_count = revs->max_count;
> > +   int queuei_count = 0;
> > +   int queueo_count = 0;
> > +   struct commit_list *queueo = NULL;
> > +   struct commit_list *queuei = NULL;
> > +   struct commit_list *reversed_queue = NULL;
> > +
> > +   revs->max_count = -1;
> > +   while ((c = get_revision_internal(revs))) {
> > +       c->object.flags &= ~SHOWN;
> > +       commit_list_insert(c, &queuei);
> > +       queuei_count++;
> > +       while (queuei_count + queueo_count > max_count) {
> > +           if (!queueo_count) {
> > +               while (queuei_count > 0) {
> > +                   c = pop_commit(&queuei);
> > +                   queuei_count--;
> > +                   commit_list_insert(c, &queueo);
> > +                   queueo_count++;
> > +               }
> > +           }
> > +           pop_commit(&queueo);
> > +           queueo_count--;
> > +       }
> > +   }
> > +
> > +   while ((c = pop_commit(&queueo)))
> > +       commit_list_insert(c, &reversed_queue);
> > +   while ((c = pop_commit(&queuei)))
> > +       commit_list_insert(c, &queueo);
> > +   while ((c = pop_commit(&queueo)))
> > +       commit_list_insert(c, &reversed_queue);
> > +
> > +   while ((c = pop_commit(&reversed_queue)))
> > +       commit_list_insert(c, queue);
> > +}
> > +
> >  struct commit *get_revision(struct rev_info *revs)
> >  {
> >     struct commit *c;
> >     struct commit_list *reversed;
> > +   struct commit_list *queue = NULL;
> > +
> > +   if (revs->max_count_type == 1 && !revs->max_count_stage) {
> > +       retrieve_oldest_commits(revs, &queue);
> > +       commit_list_free(revs->commits);
> > +       revs->commits = queue;
> > +       revs->max_count_stage = 1;
> > +   }
> >  
> >     if (revs->reverse) {
> >         reversed = NULL;
> > -       while ((c = get_revision_internal(revs)))
> > -           commit_list_insert(c, &reversed);
> > +       if (revs->max_count_type == 1)
> > +           while ((c = pop_commit(&revs->commits)))
> > +               commit_list_insert(c, &reversed);
> > +       else
> > +           while ((c = get_revision_internal(revs)))
> > +               commit_list_insert(c, &reversed);
> >         commit_list_free(revs->commits);
> >         revs->commits = reversed;
> >         revs->reverse = 0;
> 
> I would have expected that this kind of commit counting is handled at
> the same spot where --max-count is handled, i.e., in
> get_revision_internal(). It could make a difference in combination with
> sorting options, --boundary, and --graph. The goal is that --max-count
> and --max-count-oldest behave the same in this regard. (But I am in no
> way an expert of the revision walker.)