[PATCH v2] revision: add --except option

[PATCH v2] revision: add --except option

[Felipe Contreras]

So that it's possible to remove certain refs from the list without
removing the objects that are referenced by other refs.

For example this repository:

  C (crap)
  B (test)
  A (HEAD, master)

When using '--branches --except crap':

  B (test)
  A (HEAD, master)

But when using '--branches --not crap' nothing will come out.

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---

I don't like the complexity of recalculate_flag(), but it seems to be the most
straight-forward way of making --not and --except work.

 Documentation/git-rev-parse.txt        |  6 ++++
 contrib/completion/git-completion.bash |  2 +-
 revision.c                             | 52 ++++++++++++++++++++++++++++-
 revision.h                             |  3 +-
 t/t6112-rev-list-except.sh             | 60 ++++++++++++++++++++++++++++++++++
 5 files changed, 120 insertions(+), 3 deletions(-)
 create mode 100755 t/t6112-rev-list-except.sh

diff --git a/Documentation/git-rev-parse.txt b/Documentation/git-rev-parse.txt
index 2b126c0..fe5cc6b 100644
--- a/Documentation/git-rev-parse.txt
+++ b/Documentation/git-rev-parse.txt
@@ -110,6 +110,12 @@ can be used.
 	strip '{caret}' prefix from the object names that already have
 	one.
 
+--except::
+	Skip the following object names. For example:
+	'--branches --except master' will show all the branches, except master.
+	This differs from --not in that --except will still show the object, if
+	they are referenced by another object name.
+
 --symbolic::
 	Usually the object names are output in SHA-1 form (with
 	possible '{caret}' prefix); this option makes them output in a
diff --git a/contrib/completion/git-completion.bash b/contrib/completion/git-completion.bash
index 5da920e..aed8c12 100644
--- a/contrib/completion/git-completion.bash
+++ b/contrib/completion/git-completion.bash
@@ -1386,7 +1386,7 @@ _git_ls_tree ()
 
 # Options that go well for log, shortlog and gitk
 __git_log_common_options="
-	--not --all
+	--not --except --all
 	--branches --tags --remotes
 	--first-parent --merges --no-merges
 	--max-count=
diff --git a/revision.c b/revision.c
index 84ccc05..a7664dd 100644
--- a/revision.c
+++ b/revision.c
@@ -1984,6 +1984,8 @@ static int handle_revision_pseudo_opt(const char *submodule,
 		handle_reflog(revs, *flags);
 	} else if (!strcmp(arg, "--not")) {
 		*flags ^= UNINTERESTING | BOTTOM;
+	} else if (!strcmp(arg, "--except")) {
+		*flags |= SKIP;
 	} else if (!strcmp(arg, "--no-walk")) {
 		revs->no_walk = REVISION_WALK_NO_WALK_SORTED;
 	} else if (!prefixcmp(arg, "--no-walk=")) {
@@ -2573,24 +2575,72 @@ void reset_revision_walk(void)
 	clear_object_flags(SEEN | ADDED | SHOWN);
 }
 
+static int refcmp(const char *a, const char *b)
+{
+	a = prettify_refname(a);
+	if (*a == '^')
+		a++;
+	b = prettify_refname(b);
+	if (*b == '^')
+		b++;
+	return strcmp(a, b);
+}
+
+static int recalculate_flag(struct rev_info *revs, unsigned char *sha1, const char *name)
+{
+	int flags = 0;
+	int i;
+	for (i = 0; i < revs->cmdline.nr; i++) {
+		struct object *object;
+		struct rev_cmdline_entry *ce;
+		ce = &revs->cmdline.rev[i];
+		object = ce->item;
+		while (object->type == OBJ_TAG) {
+			struct tag *tag = (struct tag *) object;
+			if (!tag->tagged)
+				continue;
+			object = parse_object(tag->tagged->sha1);
+			if (!object)
+				continue;
+		}
+		if (hashcmp(object->sha1, sha1))
+			continue;
+		if (!strcmp(ce->name, name))
+			continue;
+		flags |= ce->flags;
+	}
+	return flags;
+}
+
 int prepare_revision_walk(struct rev_info *revs)
 {
 	int nr = revs->pending.nr;
 	struct object_array_entry *e, *list;
 	struct commit_list **next = &revs->commits;
+	int i;
 
 	e = list = revs->pending.objects;
 	revs->pending.nr = 0;
 	revs->pending.alloc = 0;
 	revs->pending.objects = NULL;
 	while (--nr >= 0) {
-		struct commit *commit = handle_commit(revs, e->item, e->name);
+		struct commit *commit;
+		for (i = 0; i < revs->cmdline.nr; i++) {
+			struct rev_cmdline_entry *ce;
+			ce = &revs->cmdline.rev[i];
+			if ((ce->flags & SKIP) && !refcmp(ce->name, e->name)) {
+				e->item->flags = recalculate_flag(revs, e->item->sha1, ce->name);
+				goto next;
+			}
+		}
+		commit = handle_commit(revs, e->item, e->name);
 		if (commit) {
 			if (!(commit->object.flags & SEEN)) {
 				commit->object.flags |= SEEN;
 				next = commit_list_append(commit, next);
 			}
 		}
+next:
 		e++;
 	}
 	if (!revs->leak_pending)
diff --git a/revision.h b/revision.h
index 95859ba..89f5037 100644
--- a/revision.h
+++ b/revision.h
@@ -17,7 +17,8 @@
 #define SYMMETRIC_LEFT	(1u<<8)
 #define PATCHSAME	(1u<<9)
 #define BOTTOM		(1u<<10)
-#define ALL_REV_FLAGS	((1u<<11)-1)
+#define SKIP		(1u<<11)
+#define ALL_REV_FLAGS	((1u<<12)-1)
 
 #define DECORATE_SHORT_REFS	1
 #define DECORATE_FULL_REFS	2
diff --git a/t/t6112-rev-list-except.sh b/t/t6112-rev-list-except.sh
new file mode 100755
index 0000000..a40a641
--- /dev/null
+++ b/t/t6112-rev-list-except.sh
@@ -0,0 +1,60 @@
+#!/bin/sh
+
+test_description='test for rev-list --except'
+
+. ./test-lib.sh
+
+test_expect_success 'setup' '
+
+	echo one > content &&
+	git add content &&
+	git commit -m one &&
+	git checkout -b test master &&
+	echo two > content &&
+	git commit -a -m two &&
+	git checkout -b merge master &&
+	git merge test
+'
+
+test_expect_success 'rev-list --except' '
+
+	git rev-list --topo-order --branches --except merge > actual &&
+	git rev-list --topo-order test > expect &&
+	test_cmp expect actual
+'
+
+test_expect_success 'rev-list --except with extra' '
+
+	echo three > content &&
+	git commit -a -m three &&
+	git rev-list --topo-order --branches --except merge > actual &&
+	git rev-list --topo-order test > expect &&
+	test_cmp expect actual
+'
+
+test_expect_success 'rev-list --except with full ref' '
+
+	git rev-list --topo-order --branches --except refs/heads/merge > actual &&
+	git rev-list --topo-order test > expect &&
+	test_cmp expect actual
+'
+
+test_expect_success 'rev-list --except and --not' '
+
+	git rev-list --topo-order test --not master --except master > actual &&
+	git rev-list --topo-order test > expect &&
+	test_cmp expect actual
+'
+
+test_expect_success 'rev-list --except and --not with proper flags' '
+
+	git checkout -b maint master &&
+	git checkout -b next test &&
+	echo four > content &&
+	git commit -a -m four &&
+	git rev-list --topo-order next --not master maint --except maint > actual &&
+	git rev-list --topo-order next --not master > expect &&
+	test_cmp expect actual
+'
+
+test_done
-- 
1.8.4-337-g7358a66-dirty

Re: [PATCH v2] revision: add --except option

[Johannes Sixt]

Am 8/31/2013 23:26, schrieb Felipe Contreras:
> +--except::
> +	Skip the following object names. For example:
> +	'--branches --except master' will show all the branches, except master.
> +	This differs from --not in that --except will still show the object, if
> +	they are referenced by another object name.

--except needs a bit more documentation, in particular, how it interacts
with --not.

It would be better to settle on the meaning of --except before throwing
back and forth implementations.

-- Hannes

Re: [PATCH v2] revision: add --except option

[Felipe Contreras]

On Mon, Sep 2, 2013 at 1:30 AM, Johannes Sixt <j.sixt@viscovery.net> wrote:

> It would be better to settle on the meaning of --except before throwing
> back and forth implementations.

I've lost the count of features that get discussed endlessly, and
nobody implements them ever. The implementation should dictate the
behavior, because it doesn't matter if you have described perfect
behavior, and the implementation is nearly impossible. If you
disagree, feel free to write the code.

Talk is cheap, show me the code. -- Linus Torvalds

-- 
Felipe Contreras