Re: [PATCH v5] 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 combine --max-count with
> --skip. This is not very user-friendly.

To use "--skip=<n>" for this purose, you'd need to know how many
records are going to be omitted to begin with, but that means you'd
run the command without count limitation once only to find out how
many records there are.  "not very user-friendly" sounds like an
understatement of the year.

They can do with something silly like

    git rev-list ... |
    tail -n N |
    xargs -n1 git show ...

and that does count as "not very user-friendly", I would think.

> Teach get_revision() the --max-count-oldest option.
>
> Signed-off-by: Mirko Faina <mroik@delayed.space>
> ---
>  Documentation/rev-list-options.adoc |  3 ++
>  revision.c                          | 77 +++++++++++++++++++++++++++--
>  revision.h                          |  2 +
>  t/t4202-log.sh                      | 14 ++++++
>  4 files changed, 93 insertions(+), 3 deletions(-)

It looks like this needs measurably smaller damage to the codebase
than the other --reverse=before approach ;-).

> diff --git a/Documentation/rev-list-options.adoc b/Documentation/rev-list-options.adoc
> index 2d195a1474..736f34efab 100644
> --- a/Documentation/rev-list-options.adoc
> +++ b/Documentation/rev-list-options.adoc
> @@ -18,6 +18,9 @@ ordering and formatting options, such as `--reverse`.
>  `--max-count=<number>`::
>  	Limit the output to _<number>_ commits.
>  
> +`--max-count-oldest=<number>`::
> +	Limit the output to the _<number>_ oldest 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"));
>  		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
>  |