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

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

Mirko Faina <mroik@delayed.space> writes:

> --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.
> +
>  `--skip=<number>`::
>     Skip _<number>_ commits before starting to show the commit output.

Saving this message to a file and grepping for a tab finds nothing,
which indicates that the patch seems to be unsalvageably whitespace
broken, given that most of the context lines should use tabs for
indent.

What did you do differently this time?  The previous rounds did not
have this problem.


> 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"));
> +       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"));
>         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;
>  }
>  
> +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;
> @@ -4543,7 +4610,11 @@ struct commit *get_revision(struct rev_info *revs)
>         return c;
>     }
>  
> -   c = get_revision_internal(revs);
> +   if (revs->max_count_stage)
> +       c = pop_commit(&revs->commits);
> +   else
> +       c = get_revision_internal(revs);
> +
>     if (c && revs->graph)
>         graph_update(revs->graph, c);
>     if (!c) {
> diff --git a/revision.h b/revision.h
> index 584f1338b5..e157463cb1 100644
> --- a/revision.h
> +++ b/revision.h
> @@ -309,6 +309,8 @@ struct rev_info {
>     /* special limits */
>     int skip_count;
>     int max_count;
> +   unsigned int max_count_type:1;
> +   unsigned int max_count_stage:1;
>     timestamp_t max_age;
>     timestamp_t max_age_as_filter;
>     timestamp_t min_age;
> diff --git a/t/t4202-log.sh b/t/t4202-log.sh
> index 05cee9e41b..668c231cf1 100755
> --- a/t/t4202-log.sh
> +++ b/t/t4202-log.sh
> @@ -1882,6 +1882,20 @@ test_expect_success 'log --graph with --name-status' '
>     test_cmp_graph --name-status tangle..reach
>  '
>  
> +test_expect_success 'log --max-count-oldest=3 --oneline' '
> +   test_when_finished rm expect &&
> +   git log --oneline | tail -n3 >expect &&
> +   git log --oneline --max-count-oldest=3 >actual &&
> +   test_cmp expect actual
> +'
> +
> +test_expect_success 'log --max-count-oldest=3 --reverse --oneline' '
> +   test_when_finished rm expect &&
> +   git log --oneline | tail -n3 | tac >expect &&
> +   git log --oneline --max-count-oldest=3 --reverse >actual &&
> +   test_cmp expect actual
> +'
> +
>  cat >expect <<-\EOF
>  * reach
>  |