[PATCH v4 1/2] revision.c: implement --reverse=before for walks

[Mirko Faina <mroik@delayed.space>]

In a revision walk `--reverse` can only be applied after any commit
limiting option. This makes getting a limited amount of commits from the
tail impossible. E.g.

    git log --reverse --max-count=3

Some would expect this to give back the first 3 commits of the project.
Instead it returns the last 3 but in reversed order.

Teach `get_revision()` to accpet an argument `(after|before)` from the
CLI, and apply the reversal before or after the commit limiting options
based on this argument. If no argument is provided default to the
current behaviour, applying `--reverse` after the commit limiting
options.

Signed-off-by: Mirko Faina <mroik@delayed.space>
---
 Documentation/rev-list-options.adoc | 16 +++++--
 revision.c                          | 31 ++++++++++++--
 revision.h                          |  8 +++-
 t/t4202-log.sh                      | 66 +++++++++++++++++++++++++++++
 4 files changed, 113 insertions(+), 8 deletions(-)

diff --git a/Documentation/rev-list-options.adoc b/Documentation/rev-list-options.adoc
index 2d195a1474..e97f6f2aff 100644
--- a/Documentation/rev-list-options.adoc
+++ b/Documentation/rev-list-options.adoc
@@ -914,10 +914,18 @@ With `--topo-order`, they would show 8 6 5 3 7 4 2 1 (or 8 7 4 2 6 5
 avoid showing the commits from two parallel development track mixed
 together.
 
-`--reverse`::
-	Output the commits chosen to be shown (see 'Commit Limiting'
-	section above) in reverse order. Cannot be combined with
-	`--walk-reflogs`.
+`--[no-]reverse[=(after|before)]`::
+	Accepts `after` or `before`. Cannot be combined with
+	`--walk-reflogs`. If `after`, output the commits chosen to be
+	shown (see 'Commit Limiting' section above) in reverse order. If
+	`before`, reverse the commits before filtering with `Commit
+	Limiting` options. When multiple `--reverse=` options are given,
+	the final option overrides any previous options. The `--reverse`
+	option (with no specifier) behaves as `--reverse=after`, except
+	that, for historical reasons, it negates any previous reversed
+	state (so `--reverse --reverse` does nothing, nor does
+	`--reverse=before --reverse`. Note that `--reverse=before
+	--reverse --reverse` is the same as `--reverse=after`).
 endif::git-shortlog[]
 
 ifndef::git-shortlog[]
diff --git a/revision.c b/revision.c
index 599b3a66c3..d581f5e38e 100644
--- a/revision.c
+++ b/revision.c
@@ -2686,7 +2686,16 @@ static int handle_revision_opt(struct rev_info *revs, int argc, const char **arg
 			git_log_output_encoding = xstrdup("");
 		return argcount;
 	} else if (!strcmp(arg, "--reverse")) {
-		revs->reverse ^= 1;
+		revs->reverse = !revs->reverse;
+	} else if (skip_prefix(arg, "--reverse=", &optarg)) {
+		if (!strcmp(optarg, "after"))
+			revs->reverse = REVERSE_AFTER;
+		else if(!strcmp(optarg, "before"))
+			revs->reverse = REVERSE_BEFORE;
+		else
+			die(_("unknown value for --reverse: %s"), optarg);
+	} else if (!strcmp(arg, "--no-reverse")) {
+		revs->reverse = NO_REVERSE;
 	} else if (!strcmp(arg, "--children")) {
 		revs->children.name = "children";
 		revs->limited = 1;
@@ -4525,19 +4534,35 @@ struct commit *get_revision(struct rev_info *revs)
 {
 	struct commit *c;
 	struct commit_list *reversed;
+	int max_count = revs->max_count;
+
+	if (revs->reverse && !revs->reverse_output_stage) {
+		if (revs->reverse == 3) {
+			BUG("allowed values for reverse are 0, 1 and 2");
+			revs->reverse = 1;
+		}
+
+		if (revs->reverse == REVERSE_BEFORE)
+			revs->max_count = -1;
 
-	if (revs->reverse) {
 		reversed = NULL;
 		while ((c = get_revision_internal(revs)))
 			commit_list_insert(c, &reversed);
 		commit_list_free(revs->commits);
 		revs->commits = reversed;
-		revs->reverse = 0;
 		revs->reverse_output_stage = 1;
+
+		if (revs->reverse == REVERSE_BEFORE)
+			revs->max_count = max_count;
 	}
 
 	if (revs->reverse_output_stage) {
+		if (revs->reverse == REVERSE_BEFORE && revs->max_count == 0)
+			return NULL;
+
 		c = pop_commit(&revs->commits);
+		if (revs->reverse == REVERSE_BEFORE)
+			revs->max_count--;
 		if (revs->track_linear)
 			revs->linear = !!(c && c->object.flags & TRACK_LINEAR);
 		return c;
diff --git a/revision.h b/revision.h
index 584f1338b5..02881577dc 100644
--- a/revision.h
+++ b/revision.h
@@ -121,6 +121,12 @@ struct ref_exclusions {
 struct oidset;
 struct topo_walk_info;
 
+enum rev_reverse {
+	NO_REVERSE = 0,
+	REVERSE_AFTER = 1,
+	REVERSE_BEFORE = 2,
+};
+
 struct rev_info {
 	/* Starting list */
 	struct commit_list *commits;
@@ -167,6 +173,7 @@ struct rev_info {
 			ignore_missing_links:1;
 
 	/* Traversal flags */
+	enum rev_reverse reverse:2;
 	unsigned int	dense:1,
 			prune:1,
 			no_walk:1,
@@ -196,7 +203,6 @@ struct rev_info {
 			rewrite_parents:1,
 			print_parents:1,
 			show_decorations:1,
-			reverse:1,
 			reverse_output_stage:1,
 			cherry_pick:1,
 			cherry_mark:1,
diff --git a/t/t4202-log.sh b/t/t4202-log.sh
index 05cee9e41b..3bfe2c99b8 100755
--- a/t/t4202-log.sh
+++ b/t/t4202-log.sh
@@ -1882,6 +1882,72 @@ test_expect_success 'log --graph with --name-status' '
 	test_cmp_graph --name-status tangle..reach
 '
 
+cat >expect <<-\EOF
+c3f451c Merge tag 'reach'
+046b221 to remove
+EOF
+
+test_expect_success 'log --reverse --oneline --max-count=2' '
+	test_when_finished git reset --hard HEAD~1 &&
+	touch to_remove &&
+	git add to_remove &&
+	git commit -m "to remove" &&
+	git log --reverse --oneline --max-count=2 >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'log --reverse --reverse --reverse --oneline --max-count=2' '
+	test_when_finished git reset --hard HEAD~1 &&
+	touch to_remove &&
+	git add to_remove &&
+	git commit -m "to remove" &&
+	git log --reverse --reverse --reverse --oneline --max-count=2 >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'log --reverse=after --oneline --max-count=2' '
+	test_when_finished git reset --hard HEAD~1 &&
+	touch to_remove &&
+	git add to_remove &&
+	git commit -m "to remove" &&
+	git log --reverse=after --oneline --max-count=2 >actual &&
+	test_cmp expect actual
+'
+
+cat >expect <<-\EOF
+3a2fdcb initial
+f7dab8e second
+EOF
+
+test_expect_success 'log --reverse=before --oneline --max-count=2' '
+	test_when_finished rm actual &&
+	git log --reverse=before --oneline --max-count=2 >actual &&
+	test_cmp expect actual
+'
+
+cat >expect <<-\EOF
+046b221 to remove
+c3f451c Merge tag 'reach'
+EOF
+
+test_expect_success 'log --reverse --reverse --oneline --max-count=2' '
+	test_when_finished git reset --hard HEAD~1 &&
+	touch to_remove &&
+	git add to_remove &&
+	git commit -m "to remove" &&
+	git log --reverse --reverse --oneline --max-count=2 >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'log --reverse --no-reverse --oneline --max-count=2' '
+	test_when_finished git reset --hard HEAD~1 &&
+	touch to_remove &&
+	git add to_remove &&
+	git commit -m "to remove" &&
+	git log --reverse --no-reverse --oneline --max-count=2 >actual &&
+	test_cmp expect actual
+'
+
 cat >expect <<-\EOF
 * reach
 |
-- 
2.54.0