Re: [PATCH] rev-list: Add a new option --skip.

Subsystems: the rest

2 messages, 2 authors, 2016-08-11 · open the first message on its own page

Re: [PATCH] rev-list: Add a new option --skip.

From: Junio C Hamano <hidden>
Date: 2016-08-11 19:55:43

Robert Fitzsimons [off-list ref] writes:

quoted hunk

diff --git a/builtin-rev-list.c b/builtin-rev-list.c
index fb7fc92..432f901 100644
--- a/builtin-rev-list.c
+++ b/builtin-rev-list.c

@@ -246,6 +248,10 @@ int cmd_rev_list(int argc, const char **argv, const char *prefix)
 			read_revisions_from_stdin(&revs);
 			continue;
 		}
+		if (!strncmp(arg, "--skip=", 7)) {
+			skip = atoi(arg + 7);
+			continue;
+		}
 		usage(rev_list_usage);
 
 	}

Hmph....

I am having a hard time convincing myself that this is a feature
that is a narrow special case for rev-list and does not belong
to the generic revision traversal machinery.

That is, would people expect that 'log' family allow the to say:

	$ git log --skip=10 -4 master

Declaring this as a special case for rev-list is certainly safer
(no risk to harm the revision machinery which is quite central
part of git), but if you define and initialize the new field
next to max_count, it makes me feel that it should somehow be
handled at the same layer.

In other words,...

---

 revision.c |   46 ++++++++++++++++++++++++++++++++--------------
 revision.h |    1 +
 2 files changed, 33 insertions(+), 14 deletions(-)

diff --git a/revision.c b/revision.c
index 993bb66..aa63d10 100644
--- a/revision.c
+++ b/revision.c

@@ -524,6 +524,7 @@ void init_revisions(struct rev_info *revs, const char *prefix)
 	revs->prefix = prefix;
 	revs->max_age = -1;
 	revs->min_age = -1;
+	revs->skip_count = -1;
 	revs->max_count = -1;
 
 	revs->prune_fn = NULL;

@@ -760,6 +761,10 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch
 				revs->max_count = atoi(arg + 12);
 				continue;
 			}
+			if (!strncmp(arg, "--skip=", 7)) {
+				revs->skip_count = atoi(arg + 7);
+				continue;
+			}
 			/* accept -<digit>, like traditional "head" */
 			if ((*arg == '-') && isdigit(arg[1])) {
 				revs->max_count = atoi(arg + 1);

@@ -1123,23 +1128,11 @@ static int commit_match(struct commit *commit, struct rev_info *opt)
 			   commit->buffer, strlen(commit->buffer));
 }
 
-struct commit *get_revision(struct rev_info *revs)
+static struct commit *get_revision_1(struct rev_info *revs)
 {
-	struct commit_list *list = revs->commits;
-
-	if (!list)
+	if (!revs->commits)
 		return NULL;
 
-	/* Check the max_count ... */
-	switch (revs->max_count) {
-	case -1:
-		break;
-	case 0:
-		return NULL;
-	default:
-		revs->max_count--;
-	}
-
 	do {
 		struct commit_list *entry = revs->commits;
 		struct commit *commit = entry->item;

@@ -1206,3 +1199,28 @@ struct commit *get_revision(struct rev_info *revs)
 	} while (revs->commits);
 	return NULL;
 }
+
+struct commit *get_revision(struct rev_info *revs)
+{
+	struct commit *c = NULL;
+
+	if (0 < revs->skip_count) {
+		while ((c = get_revision_1(revs)) != NULL) {
+			if (revs->skip_count-- <= 0)
+				break;
+		}
+	}
+
+	/* Check the max_count ... */
+	switch (revs->max_count) {
+	case -1:
+		break;
+	case 0:
+		return NULL;
+	default:
+		revs->max_count--;
+	}
+	if (c)
+		return c;
+	return get_revision_1(revs);
+}

diff --git a/revision.h b/revision.h
index 3adab95..81f522c 100644
--- a/revision.h
+++ b/revision.h

@@ -75,6 +75,7 @@ struct rev_info {
 	struct grep_opt	*grep_filter;
 
 	/* special limits */
+	int skip_count;
 	int max_count;
 	unsigned long max_age;
 	unsigned long min_age;

[PATCH] rev-list: Document --skip and add test cases.

From: Robert Fitzsimons <hidden>
Date: 2016-08-11 19:51:46

Signed-off-by: Robert Fitzsimons <redacted>
---

I am having a hard time convincing myself that this is a feature
that is a narrow special case for rev-list and does not belong
to the generic revision traversal machinery.

Your implementation is much better then mine.  Here's some documentation
and a set of test cases.

Robert


 Documentation/git-rev-list.txt |    5 ++++
 t/t6005-rev-list-count.sh      |   51 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 56 insertions(+), 0 deletions(-)

diff --git a/Documentation/git-rev-list.txt b/Documentation/git-rev-list.txt
index ec43c0b..9e0dcf8 100644
--- a/Documentation/git-rev-list.txt
+++ b/Documentation/git-rev-list.txt

@@ -10,6 +10,7 @@ SYNOPSIS
 --------
 [verse]
 'git-rev-list' [ \--max-count=number ]
+	     [ \--skip=number ]
 	     [ \--max-age=timestamp ]
 	     [ \--min-age=timestamp ]
 	     [ \--sparse ]

@@ -139,6 +140,10 @@ limiting may be applied.
 
 	Limit the number of commits output.
 
+--skip='number'::
+
+	Skip 'number' commits before starting to show the commit output.
+
 --since='date', --after='date'::
 
 	Show commits more recent than a specific date.

diff --git a/t/t6005-rev-list-count.sh b/t/t6005-rev-list-count.sh
new file mode 100755
index 0000000..334fccf
--- /dev/null
+++ b/t/t6005-rev-list-count.sh

@@ -0,0 +1,51 @@
+#!/bin/sh
+
+test_description='git-rev-list --max-count and --skip test'
+
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+    for n in 1 2 3 4 5 ; do \
+        echo $n > a ; \
+        git add a ; \
+        git commit -m "$n" ; \
+    done
+'
+
+test_expect_success 'no options' '
+    test $(git-rev-list HEAD | wc -l) = 5
+'
+
+test_expect_success '--max-count' '
+    test $(git-rev-list HEAD --max-count=0 | wc -l) = 0 &&
+    test $(git-rev-list HEAD --max-count=3 | wc -l) = 3 &&
+    test $(git-rev-list HEAD --max-count=5 | wc -l) = 5 &&
+    test $(git-rev-list HEAD --max-count=10 | wc -l) = 5
+'
+
+test_expect_success '--max-count all forms' '
+    test $(git-rev-list HEAD --max-count=1 | wc -l) = 1 &&
+    test $(git-rev-list HEAD -1 | wc -l) = 1 &&
+    test $(git-rev-list HEAD -n1 | wc -l) = 1 &&
+    test $(git-rev-list HEAD -n 1 | wc -l) = 1
+'
+
+test_expect_success '--skip' '
+    test $(git-rev-list HEAD --skip=0 | wc -l) = 5 &&
+    test $(git-rev-list HEAD --skip=3 | wc -l) = 2 &&
+    test $(git-rev-list HEAD --skip=5 | wc -l) = 0 &&
+    test $(git-rev-list HEAD --skip=10 | wc -l) = 0
+'
+
+test_expect_success '--skip --max-count' '
+    test $(git-rev-list HEAD --skip=0 --max-count=0 | wc -l) = 0 &&
+    test $(git-rev-list HEAD --skip=0 --max-count=10 | wc -l) = 5 &&
+    test $(git-rev-list HEAD --skip=3 --max-count=0 | wc -l) = 0 &&
+    test $(git-rev-list HEAD --skip=3 --max-count=1 | wc -l) = 1 &&
+    test $(git-rev-list HEAD --skip=3 --max-count=2 | wc -l) = 2 &&
+    test $(git-rev-list HEAD --skip=3 --max-count=10 | wc -l) = 2 &&
+    test $(git-rev-list HEAD --skip=5 --max-count=10 | wc -l) = 0 &&
+    test $(git-rev-list HEAD --skip=10 --max-count=10 | wc -l) = 0
+'
+
+test_done

-- 
1.4.4.2.g80fef-dirty

`h`	back out one level
`j`	next message in thread
`k`	previous message in thread
`l`	drill in
`Esc`	close help / fold thread tree
`?`	toggle this help