[PATCH] revision: add --except option

[PATCH] 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:

  * 374e8dd (crap) crap
  * 4cbbf7b (test) two
  * d025ae0 (HEAD, master) one

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

  * 4cbbf7b (test) two
  * d025ae0 (HEAD, master) one

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

Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
---
 Documentation/git-rev-parse.txt        |  6 ++++++
 contrib/completion/git-completion.bash |  2 +-
 revision.c                             | 10 ++++++++++
 revision.h                             |  3 ++-
 t/t6112-rev-list-except.sh             | 35 ++++++++++++++++++++++++++++++++++
 5 files changed, 54 insertions(+), 2 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..375adab 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=")) {
@@ -2578,6 +2580,7 @@ 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;
@@ -2585,12 +2588,19 @@ int prepare_revision_walk(struct rev_info *revs)
 	revs->pending.objects = NULL;
 	while (--nr >= 0) {
 		struct commit *commit = handle_commit(revs, e->item, e->name);
+		for (i = 0; i < revs->cmdline.nr; i++) {
+			struct rev_cmdline_entry *ce;
+			ce = &revs->cmdline.rev[i];
+			if ((ce->flags & SKIP) && !strcmp(ce->name, e->name))
+				goto next;
+		}
 		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..b8f9a61
--- /dev/null
+++ b/t/t6112-rev-list-except.sh
@@ -0,0 +1,35 @@
+#!/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_done
-- 
1.8.4-fc

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

[Felipe Contreras]

On Fri, Aug 30, 2013 at 12:00 AM, Felipe Contreras
<felipe.contreras@gmail.com> wrote:
> 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:
>
>   * 374e8dd (crap) crap
>   * 4cbbf7b (test) two
>   * d025ae0 (HEAD, master) one
>
> When using '--branches --except crap':
>
>   * 4cbbf7b (test) two
>   * d025ae0 (HEAD, master) one
>
> But when using '--branches --not crap' nothing will come out.

Doesn't work with certain refs, here's a fix:

diff --git a/revision.c b/revision.c
index 375adab..25564c1 100644
--- a/revision.c
+++ b/revision.c
@@ -2575,6 +2575,13 @@ void reset_revision_walk(void)
        clear_object_flags(SEEN | ADDED | SHOWN);
 }

+static int refcmp(const char *a, const char *b)
+{
+       a = prettify_refname(a);
+       b = prettify_refname(b);
+       return strcmp(a, b);
+}
+
 int prepare_revision_walk(struct rev_info *revs)
 {
        int nr = revs->pending.nr;
@@ -2591,7 +2598,7 @@ int prepare_revision_walk(struct rev_info *revs)
                for (i = 0; i < revs->cmdline.nr; i++) {
                        struct rev_cmdline_entry *ce;
                        ce = &revs->cmdline.rev[i];
-                       if ((ce->flags & SKIP) && !strcmp(ce->name, e->name))
+                       if ((ce->flags & SKIP) && !refcmp(ce->name, e->name))
                                goto next;
                }
                if (commit) {
diff --git a/t/t6112-rev-list-except.sh b/t/t6112-rev-list-except.sh
index b8f9a61..a295f43 100755
--- a/t/t6112-rev-list-except.sh
+++ b/t/t6112-rev-list-except.sh
@@ -32,4 +32,11 @@ test_expect_success 'rev-list --except with extra' '
        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_done

-- 
Felipe Contreras

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

[Junio C Hamano]

Felipe Contreras <felipe.contreras@gmail.com> writes:

> 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:
>
>   * 374e8dd (crap) crap
>   * 4cbbf7b (test) two
>   * d025ae0 (HEAD, master) one

Can we make it more clear that your assumption is "crap" is a child
of "test" which is a child of "master"?  Without that, the "nothing
will come out" will not follow.

> When using '--branches --except crap':
>
>   * 4cbbf7b (test) two
>   * d025ae0 (HEAD, master) one
>
> But when using '--branches --not crap' nothing will come out.

If you have a history where

 - branches "master" and "maint" point at commit A;
 - branch "next" points at commit B that is a descendant of A; and
 - there are tags X and Y pointing at commits that are ahead of B
   or behind A

i.e.

	----X----A----B----Y

what are the desired semantics for these?

 (1) --branches --except maint

 (2) --all --not --branches --except maint

 (3) ^master next --except maint

"--branches" wants to include "master", "next", and "maint", and the
"--except" tells us we do not want to take "maint" into account, but
should that affect what "master" wants to do (either include or
exclude what are reachable from it)?

As the way we parse the revisions from the command line is to mark
"objects", not "refs", as we go, it looks like that the flag SKIP in
this patch is placed conceptually at a wrong level.

I agree "--branches --except maint" is a good concept, but to
implement what this patch wants to do properly, I suspect that the
revision parser may need to be extended to be a two-phase process;
the first phase will collect list of objects involved in the range
expression without marking them with UNINTERESTING mark (that would
also involve expanding things like --all or --branches), while
remembering those given with --except, exclude the "except" set from
the first set, and then finally marking the objects using the
remainder, or something like that.

> @@ -2585,12 +2588,19 @@ int prepare_revision_walk(struct rev_info *revs)
>  	revs->pending.objects = NULL;
>  	while (--nr >= 0) {
>  		struct commit *commit = handle_commit(revs, e->item, e->name);
> +		for (i = 0; i < revs->cmdline.nr; i++) {
> +			struct rev_cmdline_entry *ce;

"ce" will have a strong association with "cache entry"; avoid using
that variable name for anything else to avoid confusion.

> +			ce = &revs->cmdline.rev[i];
> +			if ((ce->flags & SKIP) && !strcmp(ce->name, e->name))
> +				goto next;

I think this SKIP will not help an object that is already tainted by
UNINTERESTING; if it is discovered during a traversal from another
object that will remain in the rev->commits, the travesal will stop
there, even if a ref that is marked with SKIP will "goto next" here.

> +		}
>  		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/t/t6112-rev-list-except.sh b/t/t6112-rev-list-except.sh
> new file mode 100755
> index 0000000..b8f9a61
> --- /dev/null
> +++ b/t/t6112-rev-list-except.sh
> @@ -0,0 +1,35 @@
> +#!/bin/sh
> +
> +test_description='test for rev-list --except'
> +
> +. ./test-lib.sh
> +
> +test_expect_success 'setup' '
> +
> +	echo one > content &&

Style.

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

[Felipe Contreras]

On Fri, Aug 30, 2013 at 1:32 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> 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:
>>
>>   * 374e8dd (crap) crap
>>   * 4cbbf7b (test) two
>>   * d025ae0 (HEAD, master) one
>
> Can we make it more clear that your assumption is "crap" is a child
> of "test" which is a child of "master"?  Without that, the "nothing
> will come out" will not follow.
>
>> When using '--branches --except crap':
>>
>>   * 4cbbf7b (test) two
>>   * d025ae0 (HEAD, master) one
>>
>> But when using '--branches --not crap' nothing will come out.
>
> If you have a history where
>
>  - branches "master" and "maint" point at commit A;
>  - branch "next" points at commit B that is a descendant of A; and
>  - there are tags X and Y pointing at commits that are ahead of B
>    or behind A
>
> i.e.
>
>         ----X----A----B----Y
>
> what are the desired semantics for these?
>
>  (1) --branches --except maint
>
>  (2) --all --not --branches --except maint
>
>  (3) ^master next --except maint
>
> "--branches" wants to include "master", "next", and "maint", and the
> "--except" tells us we do not want to take "maint" into account, but
> should that affect what "master" wants to do (either include or
> exclude what are reachable from it)?

No, it should not. '--branches --except main' becomes 'master next'.

> As the way we parse the revisions from the command line is to mark
> "objects", not "refs", as we go, it looks like that the flag SKIP in
> this patch is placed conceptually at a wrong level.

refs are marked as well.

> I agree "--branches --except maint" is a good concept, but to
> implement what this patch wants to do properly, I suspect that the
> revision parser may need to be extended to be a two-phase process;
> the first phase will collect list of objects involved in the range
> expression without marking them with UNINTERESTING mark (that would
> also involve expanding things like --all or --branches), while
> remembering those given with --except, exclude the "except" set from
> the first set, and then finally marking the objects using the
> remainder, or something like that.

That's not necessary, this patch does the trick.

>> +                     ce = &revs->cmdline.rev[i];
>> +                     if ((ce->flags & SKIP) && !strcmp(ce->name, e->name))
>> +                             goto next;
>
> I think this SKIP will not help an object that is already tainted by
> UNINTERESTING; if it is discovered during a traversal from another
> object that will remain in the rev->commits, the travesal will stop
> there, even if a ref that is marked with SKIP will "goto next" here.

No, the traversal will continue just fine. At this point we are still
not traversing anything, simply adding the heads that will need to be
traversed later on to a list. Whether this object has been tainted by
UNINTERESTED or not is irrelevant.

If you do 'master ^maint --except master', handle_commit will return
three commits:

master -> ce->flags will have SKIP, nothing more happens
maint -> ce->flags doesn't have SKIP, processing continues, and it's
added to the list of commits, the commit has UNINTERESTING, but that
doesn't matter
master -> ce->flags will have SKIP, nothing more happens

Essentially it's the same as:

maint -> it's added to the list of commits, the commit has UNINTERESTING

So, it's exactly the same as if you had typed '^maint', which is
exactly what we want.

-- 
Felipe Contreras

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

[Felipe Contreras]

On Fri, Aug 30, 2013 at 2:26 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Pardon terseness, typo and HTML from a tablet.
>
>
> On Aug 30, 2013 12:19 AM, "Felipe Contreras" <felipe.contreras@gmail.com>
> wrote:
>>
>> On Fri, Aug 30, 2013 at 1:32 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> > Felipe Contreras <felipe.contreras@gmail.com> writes:
>> >
>> >> 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:
>> >>
>> >>   * 374e8dd (crap) crap
>> >>   * 4cbbf7b (test) two
>> >>   * d025ae0 (HEAD, master) one
>> >
>> > Can we make it more clear that your assumption is "crap" is a child
>> > of "test" which is a child of "master"?  Without that, the "nothing
>> > will come out" will not follow.
>> >
>> >> When using '--branches --except crap':
>> >>
>> >>   * 4cbbf7b (test) two
>> >>   * d025ae0 (HEAD, master) one
>> >>
>> >> But when using '--branches --not crap' nothing will come out.
>> >
>> > If you have a history where
>> >
>> >  - branches "master" and "maint" point at commit A;
>> >  - branch "next" points at commit B that is a descendant of A; and
>> >  - there are tags X and Y pointing at commits that are ahead of B
>> >    or behind A
>> >
>> > i.e.
>> >
>> >         ----X----A----B----Y
>> >
>> > what are the desired semantics for these?
>> >
>> >  (1) --branches --except maint
>> >
>> >  (2) --all --not --branches --except maint
>> >
>> >  (3) ^master next --except maint
>> >
>> > "--branches" wants to include "master", "next", and "maint", and the
>> > "--except" tells us we do not want to take "maint" into account, but
>> > should that affect what "master" wants to do (either include or
>> > exclude what are reachable from it)?
>>
>> No, it should not. '--branches --except main' becomes 'master next'.
>>
>> > As the way we parse the revisions from the command line is to mark
>> > "objects", not "refs", as we go, it looks like that the flag SKIP in
>> > this patch is placed conceptually at a wrong level.
>>
>> refs are marked as well.
>>
>> > I agree "--branches --except maint" is a good concept, but to
>> > implement what this patch wants to do properly, I suspect that the
>> > revision parser may need to be extended to be a two-phase process;
>> > the first phase will collect list of objects involved in the range
>> > expression without marking them with UNINTERESTING mark (that would
>> > also involve expanding things like --all or --branches), while
>> > remembering those given with --except, exclude the "except" set from
>> > the first set, and then finally marking the objects using the
>> > remainder, or something like that.
>>
>> That's not necessary, this patch does the trick.
>>
>> >> +                     ce = &revs->cmdline.rev[i];
>> >> +                     if ((ce->flags & SKIP) && !strcmp(ce->name,
>> >> e->name))
>> >> +                             goto next;
>> >
>> > I think this SKIP will not help an object that is already tainted by
>> > UNINTERESTING; if it is discovered during a traversal from another
>> > object that will remain in the rev->commits, the travesal will stop
>> > there, even if a ref that is marked with SKIP will "goto next" here.
>>
>> No, the traversal will continue just fine. At this point we are still
>> not traversing anything, simply adding the heads that will need to be
>> traversed later on to a list. Whether this object has been tainted by
>> UNINTERESTED or not is irrelevant.
>>
>> If you do 'master ^maint --except master', handle_commit will return
>> three commits:
>
> Would the same argument apply to
>
>   next ^maint --except maint
>
> where next gets in the queue, maint in tainted, and skipped?

maint is not skipped, as it's not the same as ^maint, basically it's
the same as:

next ^maint

I think that's good, as there's absolutely no reason why anybody would
want '^maint --except maint' to cancel each other out.

-- 
Felipe Contreras

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

[Johannes Sixt]

Am 8/30/2013 9:32, schrieb Felipe Contreras:
> On Fri, Aug 30, 2013 at 2:26 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> On Aug 30, 2013 12:19 AM, "Felipe Contreras" <felipe.contreras@gmail.com>
>> Would the same argument apply to
>>
>>   next ^maint --except maint
>>
>> where next gets in the queue, maint in tainted, and skipped?
> 
> maint is not skipped, as it's not the same as ^maint, basically it's
> the same as:
> 
> next ^maint
> 
> I think that's good, as there's absolutely no reason why anybody would
> want '^maint --except maint' to cancel each other out.

But isn't this basically the same as '--not maint --except maint'? This by
itself looks strange. But when disguised in the form '--not --branches
--except maint', it would make sense to mean '--not master next', aka
'^master ^next'.

-- Hannes

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

[Junio C Hamano]

Felipe Contreras <felipe.contreras@gmail.com> writes:

>>> If you do 'master ^maint --except master', handle_commit will return
>>> three commits:
>>
>> Would the same argument apply to
>>
>>   next ^maint --except maint
>>
>> where next gets in the queue, maint in tainted, and skipped?
>
> maint is not skipped, as it's not the same as ^maint, basically it's
> the same as:
>
> next ^maint
>
> I think that's good, as there's absolutely no reason why anybody would
> want '^maint --except maint' to cancel each other out.

I do not expect anybody to say "^maint --except maint", but the
negative one can come from expansion of --branches and such.
Essentially, the semantics the patch implements is "--except applies
only to positive tips" (I have not thought about its implications on
left-right traversals; there may be some impact there as well).

Being able to exclude only positive ones is better than not being
able to exclude any, but if we consider j6t's example, I think
applying except to negative ones is equally useful.

For example, "I want to see everything I have reachable from my refs
(not just branches), but exclude those that are reachable from my
stable branches (my local branches except those whose names match
the pattern 'wip/*')" is a reasonable thing to ask.  A wip/* branch
may be based on the work in a remote tracking branch that you have
not merged to your 'maint', 'master' or 'next', and you want to view
all the commits on the remote tracking branch you haven't merged to
your stable branches, including the ones that you already use in
your WIP that are not ready.  And the request may be spelled as

	--all --not --branches --except 'wip/*'

Which means that the approach taken by the patch to only allow
exclusion of negative ones makes the idea only 50% useful compared
to its potential.  And I suspect that "we can start from 50% which
is better than 0% and later fill the other 50%" would not work in
this case, without ripping out the "SKIP on object" approach and
redoing the "--except" support from scratch, because "SKIP on
object" fundamentally cannot undo the effects of the negative ones,
because it records the information at a wrong level.

It may be a good idea to step back a bit and think of this topic as
a way to enhance the --branches option and its friends with only the
inclusive wildcard semantics.  It lets us include those that match
the pattern with "--branches=wip/*", but there is no way to say "oh
by the way, I do not want those that match this pattern included
when you expand this short-hand".  We have --branches=pattern that
is inclusive; perhaps it can be prefixed with --branches=!pattern to
pre-declare "whatever the next --branches expands to, do not include
those that match this pattern", or something, which would make the
earlier "wip" example to be:

	--all --not --branches='!wip/*' --branches

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

[Felipe Contreras]

On Fri, Aug 30, 2013 at 11:48 AM, Junio C Hamano <gitster@pobox.com> wrote:

> Which means that the approach taken by the patch to only allow
> exclusion of negative ones makes the idea only 50% useful compared
> to its potential.  And I suspect that "we can start from 50% which
> is better than 0% and later fill the other 50%" would not work in
> this case, without ripping out the "SKIP on object" approach and
> redoing the "--except" support from scratch, because "SKIP on
> object" fundamentally cannot undo the effects of the negative ones,
> because it records the information at a wrong level.

I think it is not 50%, it is 98%. I think one or two persons might use
this secondary feature if ever, and I think waiting for that
implementation will delay the feature that 98% of people would use,
and maybe block it entirely.

-- 
Felipe Contreras

[PATCH] revision: introduce --exclude=<glob> to tame wildcards

[Junio C Hamano]

People often find "git log --branches" etc. that includes _all_
branches is cumbersome to use when they want to grab most but except
some.  The same applies to --tags, --all and --glob.

Teach the revision machinery to remember patterns, and then upon the
next such a globbing option, exclude those that match the pattern.

With this, I can view only my integration branches (e.g. maint,
master, etc.) without topic branches, which are named after two
letters from primary authors' names, slash and topic name.

    git rev-list --no-walk --exclude=??/* --branches |
    git name-rev --refs refs/heads/* --stdin

This one shows things reachable from local and remote branches that
have not been merged to the integration branches.

    git log --remotes --branches --not --exclude=??/* --branches

It may be a bit rough around the edges, in that the pattern to give
the exclude option depends on what globbing option follows.  In
these examples, the pattern "??/*" is used, not "refs/heads/??/*",
because the globbing option that follows the -"-exclude=<pattern>"
is "--branches".  As each use of globbing option resets previously
set "--exclude", this may not be such a bad thing, though.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 Junio C Hamano <gitster@pobox.com> writes:

 > It may be a good idea to step back a bit and think of this topic as
 > a way to enhance the --branches option and its friends with only the
 > inclusive wildcard semantics.  It lets us include those that match
 > the pattern with "--branches=wip/*", but there is no way to say "oh
 > by the way, I do not want those that match this pattern included
 > when you expand this short-hand".  We have --branches=pattern that
 > is inclusive; perhaps it can be prefixed with --branches=!pattern to
 > pre-declare "whatever the next --branches expands to, do not include
 > those that match this pattern", or something, which would make the
 > earlier "wip" example to be:
 >
 > 	--all --not --branches='!wip/*' --branches

 So here is a quick attempt at that approach, which does not look
 too intrusive. 

 revision.c | 50 ++++++++++++++++++++++++++++++++++++++++++++++++--
 revision.h |  3 +++
 2 files changed, 51 insertions(+), 2 deletions(-)

diff --git a/revision.c b/revision.c
index 84ccc05..3e82874 100644
--- a/revision.c
+++ b/revision.c
@@ -1180,11 +1180,28 @@ struct all_refs_cb {
 	const char *name_for_errormsg;
 };
 
+static int ref_excluded(struct rev_info *revs, const char *path)
+{
+	struct string_list_item *item;
+
+	if (!revs->ref_excludes)
+		return 0;
+	for_each_string_list_item(item, revs->ref_excludes) {
+		if (!fnmatch(item->string, path, 0))
+			return 1;
+	}
+	return 0;
+}
+
 static int handle_one_ref(const char *path, const unsigned char *sha1, int flag, void *cb_data)
 {
 	struct all_refs_cb *cb = cb_data;
-	struct object *object = get_reference(cb->all_revs, path, sha1,
-					      cb->all_flags);
+	struct object *object;
+
+	if (ref_excluded(cb->all_revs, path))
+	    return 0;
+
+	object = get_reference(cb->all_revs, path, sha1, cb->all_flags);
 	add_rev_cmdline(cb->all_revs, object, path, REV_CMD_REF, cb->all_flags);
 	add_pending_sha1(cb->all_revs, path, sha1, cb->all_flags);
 	return 0;
@@ -1197,6 +1214,24 @@ static void init_all_refs_cb(struct all_refs_cb *cb, struct rev_info *revs,
 	cb->all_flags = flags;
 }
 
+static void clear_ref_exclusion(struct rev_info *revs)
+{
+	if (revs->ref_excludes) {
+		string_list_clear(revs->ref_excludes, 0);
+		free(revs->ref_excludes);
+	}
+	revs->ref_excludes = NULL;
+}
+
+static void add_ref_exclusion(struct rev_info *revs, const char *exclude)
+{
+	if (!revs->ref_excludes) {
+		revs->ref_excludes = xcalloc(1, sizeof(*revs->ref_excludes));
+		revs->ref_excludes->strdup_strings = 1;
+	}
+	string_list_append(revs->ref_excludes, exclude);
+}
+
 static void handle_refs(const char *submodule, struct rev_info *revs, unsigned flags,
 		int (*for_each)(const char *, each_ref_fn, void *))
 {
@@ -1953,33 +1988,44 @@ static int handle_revision_pseudo_opt(const char *submodule,
 	if (!strcmp(arg, "--all")) {
 		handle_refs(submodule, revs, *flags, for_each_ref_submodule);
 		handle_refs(submodule, revs, *flags, head_ref_submodule);
+		clear_ref_exclusion(revs);
 	} else if (!strcmp(arg, "--branches")) {
 		handle_refs(submodule, revs, *flags, for_each_branch_ref_submodule);
+		clear_ref_exclusion(revs);
 	} else if (!strcmp(arg, "--bisect")) {
 		handle_refs(submodule, revs, *flags, for_each_bad_bisect_ref);
 		handle_refs(submodule, revs, *flags ^ (UNINTERESTING | BOTTOM), for_each_good_bisect_ref);
 		revs->bisect = 1;
 	} else if (!strcmp(arg, "--tags")) {
 		handle_refs(submodule, revs, *flags, for_each_tag_ref_submodule);
+		clear_ref_exclusion(revs);
 	} else if (!strcmp(arg, "--remotes")) {
 		handle_refs(submodule, revs, *flags, for_each_remote_ref_submodule);
+		clear_ref_exclusion(revs);
 	} else if ((argcount = parse_long_opt("glob", argv, &optarg))) {
 		struct all_refs_cb cb;
 		init_all_refs_cb(&cb, revs, *flags);
 		for_each_glob_ref(handle_one_ref, optarg, &cb);
+		clear_ref_exclusion(revs);
+		return argcount;
+	} else if ((argcount = parse_long_opt("exclude", argv, &optarg))) {
+		add_ref_exclusion(revs, optarg);
 		return argcount;
 	} else if (!prefixcmp(arg, "--branches=")) {
 		struct all_refs_cb cb;
 		init_all_refs_cb(&cb, revs, *flags);
 		for_each_glob_ref_in(handle_one_ref, arg + 11, "refs/heads/", &cb);
+		clear_ref_exclusion(revs);
 	} else if (!prefixcmp(arg, "--tags=")) {
 		struct all_refs_cb cb;
 		init_all_refs_cb(&cb, revs, *flags);
 		for_each_glob_ref_in(handle_one_ref, arg + 7, "refs/tags/", &cb);
+		clear_ref_exclusion(revs);
 	} else if (!prefixcmp(arg, "--remotes=")) {
 		struct all_refs_cb cb;
 		init_all_refs_cb(&cb, revs, *flags);
 		for_each_glob_ref_in(handle_one_ref, arg + 10, "refs/remotes/", &cb);
+		clear_ref_exclusion(revs);
 	} else if (!strcmp(arg, "--reflog")) {
 		handle_reflog(revs, *flags);
 	} else if (!strcmp(arg, "--not")) {
diff --git a/revision.h b/revision.h
index 95859ba..b4dc56c 100644
--- a/revision.h
+++ b/revision.h
@@ -59,6 +59,9 @@ struct rev_info {
 	/* The end-points specified by the end user */
 	struct rev_cmdline_info cmdline;
 
+	/* excluding from --branches, --refs, etc. expansion */
+	struct string_list *ref_excludes;
+
 	/* Basic information */
 	const char *prefix;
 	const char *def;
-- 
1.8.4-358-g02cbb92

Re: [PATCH] revision: introduce --exclude=<glob> to tame wildcards

[Duy Nguyen]

On Sat, Aug 31, 2013 at 6:55 AM, Junio C Hamano <gitster@pobox.com> wrote:
> +static int ref_excluded(struct rev_info *revs, const char *path)
> +{
> +       struct string_list_item *item;
> +
> +       if (!revs->ref_excludes)
> +               return 0;
> +       for_each_string_list_item(item, revs->ref_excludes) {
> +               if (!fnmatch(item->string, path, 0))
> +                       return 1;
> +       }
> +       return 0;
> +}

If you pursue this, please use wildmatch instead so it supports "foo/**".
-- 
Duy

Re: [PATCH] revision: introduce --exclude=<glob> to tame wildcards

[Junio C Hamano]

Duy Nguyen <pclouds@gmail.com> writes:

> On Sat, Aug 31, 2013 at 6:55 AM, Junio C Hamano <gitster@pobox.com> wrote:
>> +static int ref_excluded(struct rev_info *revs, const char *path)
>> +{
>> +       struct string_list_item *item;
>> +
>> +       if (!revs->ref_excludes)
>> +               return 0;
>> +       for_each_string_list_item(item, revs->ref_excludes) {
>> +               if (!fnmatch(item->string, path, 0))
>> +                       return 1;
>> +       }
>> +       return 0;
>> +}
>
> If you pursue this, please use wildmatch instead so it supports "foo/**".

The thought crossed my mind and I think we should match what the
existing --glob=<pattern> option does.  A cursory look in
refs.c::filter_refs() used by refs.c::for_each_glob_ref_in() tells
me that we are using fnmatch without FNM_PATHNAME, so that is what
the above part does.

Re: [PATCH] revision: introduce --exclude=<glob> to tame wildcards

[Felipe Contreras]

On Fri, Aug 30, 2013 at 6:55 PM, Junio C Hamano <gitster@pobox.com> wrote:
> People often find "git log --branches" etc. that includes _all_
> branches is cumbersome to use when they want to grab most but except
> some.  The same applies to --tags, --all and --glob.

This idea looks very familiar, from the wording of this commit message
it seems you came with the idea out of nowhere. Did you?

> Teach the revision machinery to remember patterns, and then upon the
> next such a globbing option, exclude those that match the pattern.
>
> With this, I can view only my integration branches (e.g. maint,
> master, etc.) without topic branches, which are named after two
> letters from primary authors' names, slash and topic name.
>
>     git rev-list --no-walk --exclude=??/* --branches |
>     git name-rev --refs refs/heads/* --stdin

My patch does the trick with:

--branches --except --glob='heads/??/*'

-- 
Felipe Contreras

Re: [PATCH] revision: introduce --exclude=<glob> to tame wildcards

[Junio C Hamano]

Felipe Contreras <felipe.contreras@gmail.com> writes:

> On Fri, Aug 30, 2013 at 6:55 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> People often find "git log --branches" etc. that includes _all_
>> branches is cumbersome to use when they want to grab most but except
>> some.  The same applies to --tags, --all and --glob.
>
> This idea looks very familiar, from the wording of this commit message
> it seems you came with the idea out of nowhere. Did you?

As the comment after three-dash quotes, the inspiration came from
this suggestion:

 > It may be a good idea to step back a bit and think of this topic as
 > a way to enhance the --branches option and its friends with only the
 > inclusive wildcard semantics.

which is not anything new.  It takes from J6t's

      To unclutter 'git branch' output, I rename work-in-progress branches
      to begin with "wip/", ready-to-merge branches do not carry this
      prefix. To inspect unmerged work of the latter kind of branches I
      would like to type... what?

But the thing is, that is nothing new, either.

Pretty much ever since we added --branches --tags and later --glob,

 (1) traversing from "almost all but minus some branches", and
 (2) stopping traversal at "almost all but minus some branches"

were what people sometimes wanted to have (which is pretty much what
the first paragraph of the proposed commit message says) using
"negative glob". Looking the phrase in the list archive finds for
example $gmane/159770 from 2010, but I would not be surprised if you
find older message wishing the same.

Re: [PATCH] revision: introduce --exclude=<glob> to tame wildcards

[Felipe Contreras]

On Tue, Sep 3, 2013 at 10:45 AM, Junio C Hamano <gitster@pobox.com> wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> On Fri, Aug 30, 2013 at 6:55 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>> People often find "git log --branches" etc. that includes _all_
>>> branches is cumbersome to use when they want to grab most but except
>>> some.  The same applies to --tags, --all and --glob.
>>
>> This idea looks very familiar, from the wording of this commit message
>> it seems you came with the idea out of nowhere. Did you?
>
> As the comment after three-dash quotes, the inspiration came from
> this suggestion:
>
>  > It may be a good idea to step back a bit and think of this topic as
>  > a way to enhance the --branches option and its friends with only the
>  > inclusive wildcard semantics.
>
> which is not anything new.  It takes from J6t's
>
>       To unclutter 'git branch' output, I rename work-in-progress branches
>       to begin with "wip/", ready-to-merge branches do not carry this
>       prefix. To inspect unmerged work of the latter kind of branches I
>       would like to type... what?

Yeah, and where did that come from?

> But the thing is, that is nothing new, either.
>
> Pretty much ever since we added --branches --tags and later --glob,
>
>  (1) traversing from "almost all but minus some branches", and
>  (2) stopping traversal at "almost all but minus some branches"
>
> were what people sometimes wanted to have (which is pretty much what
> the first paragraph of the proposed commit message says) using
> "negative glob". Looking the phrase in the list archive finds for
> example $gmane/159770 from 2010, but I would not be surprised if you
> find older message wishing the same.

It is very peculiar that your patch suddenly appeared after I sent a
patch for --except, and nobody has sent a similar patch, which is not
what you would expect if "this was nothing new".

-- 
Felipe Contreras

Re: [PATCH] revision: introduce --exclude=<glob> to tame wildcards

[Johannes Sixt]

Am 31.08.2013 01:55, schrieb Junio C Hamano:
> People often find "git log --branches" etc. that includes _all_
> branches is cumbersome to use when they want to grab most but except
> some.  The same applies to --tags, --all and --glob.
> 
> Teach the revision machinery to remember patterns, and then upon the
> next such a globbing option, exclude those that match the pattern.
> 
> With this, I can view only my integration branches (e.g. maint,
> master, etc.) without topic branches, which are named after two
> letters from primary authors' names, slash and topic name.
> 
>     git rev-list --no-walk --exclude=??/* --branches |
>     git name-rev --refs refs/heads/* --stdin
> 
> This one shows things reachable from local and remote branches that
> have not been merged to the integration branches.
> 
>     git log --remotes --branches --not --exclude=??/* --branches
> 
> It may be a bit rough around the edges, in that the pattern to give
> the exclude option depends on what globbing option follows.  In
> these examples, the pattern "??/*" is used, not "refs/heads/??/*",
> because the globbing option that follows the -"-exclude=<pattern>"
> is "--branches".  As each use of globbing option resets previously
> set "--exclude", this may not be such a bad thing, though.

I argued "--except should trump everything" earlier, but the case
involving --not

  --branches --except maint --not master

to mean the same as

  --branches --except maint master

just does not make sense.

An alternative would be that --not would divide the command line
arguments into ranges within which one --except would subtract
subsequent refs from earlier globbing arguments in the same range.
That's not simpler to explain than your current proposal.

So I like the relative simplicity of this approach. Here is a bit of
documentation.

--- 8< ---
Subject: [PATCH] document --exclude option

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
 Documentation/rev-list-options.txt | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt
index 5bdfb42..650c252 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -174,6 +174,21 @@ parents) and `--max-parents=-1` (negative numbers denote no upper limit).
 	is automatically prepended if missing. If pattern lacks '?', '{asterisk}',
 	or '[', '/{asterisk}' at the end is implied.
 
+--exclude=<glob-pattern>::
+
+	Do not include refs matching '<glob-pattern>' that the next `--all`,
+	`--branches`, `--tags`, `--remotes`, or `--glob` would otherwise
+	consider. Repetitions of this option accumulate exclusion patterns
+	up to the next `--all`, `--branches`, `--tags`, `--remotes`, or
+	`--glob` option (other options or arguments do not clear
+	accumlated patterns).
++
+The patterns given should not begin with `refs/heads`, `refs/tags`, or
+`refs/remotes` when applied to `--branches`, `--tags`, or `--remotes`,
+restrictively, and they must begin with `refs/` when applied to `--glob`
+or `--all`. If a trailing '/{asterisk}' is intended, it must be given
+explicitly.
+
 --ignore-missing::
 
 	Upon seeing an invalid object name in the input, pretend as if
-- 
1.8.4.33.gd68f7e8

Re: [PATCH] revision: introduce --exclude=<glob> to tame wildcards

[Felipe Contreras]

On Mon, Sep 2, 2013 at 3:11 PM, Johannes Sixt <j6t@kdbg.org> wrote:
> Am 31.08.2013 01:55, schrieb Junio C Hamano:
>> People often find "git log --branches" etc. that includes _all_
>> branches is cumbersome to use when they want to grab most but except
>> some.  The same applies to --tags, --all and --glob.
>>
>> Teach the revision machinery to remember patterns, and then upon the
>> next such a globbing option, exclude those that match the pattern.
>>
>> With this, I can view only my integration branches (e.g. maint,
>> master, etc.) without topic branches, which are named after two
>> letters from primary authors' names, slash and topic name.
>>
>>     git rev-list --no-walk --exclude=??/* --branches |
>>     git name-rev --refs refs/heads/* --stdin
>>
>> This one shows things reachable from local and remote branches that
>> have not been merged to the integration branches.
>>
>>     git log --remotes --branches --not --exclude=??/* --branches
>>
>> It may be a bit rough around the edges, in that the pattern to give
>> the exclude option depends on what globbing option follows.  In
>> these examples, the pattern "??/*" is used, not "refs/heads/??/*",
>> because the globbing option that follows the -"-exclude=<pattern>"
>> is "--branches".  As each use of globbing option resets previously
>> set "--exclude", this may not be such a bad thing, though.
>
> I argued "--except should trump everything" earlier, but the case
> involving --not
>
>   --branches --except maint --not master
>
> to mean the same as
>
>   --branches --except maint master
>
> just does not make sense.

No, but this could make sense:

--branches ^master --except maint --not master
==
--branches --except maint

> An alternative would be that --not would divide the command line
> arguments into ranges within which one --except would subtract
> subsequent refs from earlier globbing arguments in the same range.
> That's not simpler to explain than your current proposal.

Something like that can be easily done in my approach:

--- a/revision.c
+++ b/revision.c
@@ -1984,6 +1984,7 @@ static int handle_revision_pseudo_opt(const char
*submodule,
                handle_reflog(revs, *flags);
        } else if (!strcmp(arg, "--not")) {
                *flags ^= UNINTERESTING | BOTTOM;
+               *flags &= ~SKIP;
        } else if (!strcmp(arg, "--except")) {
                *flags |= SKIP;
        } else if (!strcmp(arg, "--no-walk")) {
@@ -2628,7 +2629,8 @@ int prepare_revision_walk(struct rev_info *revs)
                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)) {
+                       if ((ce->flags & SKIP) && !refcmp(ce->name, e->name) &&
+                                       ((ce->flags & UNINTERESTING)
== (e->item->flags & UNINTERESTING))) {
                                e->item->flags =
recalculate_flag(revs, e->item->sha1, ce->name);
                                goto next;
                        }
diff --git a/t/t6112-rev-list-except.sh b/t/t6112-rev-list-except.sh
index a40a641..441e1da 100755
--- a/t/t6112-rev-list-except.sh
+++ b/t/t6112-rev-list-except.sh
@@ -57,4 +57,21 @@ test_expect_success 'rev-list --except and --not
with proper flags' '
        test_cmp expect actual
 '

+test_expect_success 'rev-list --not ranges' '
+
+       git rev-list --topo-order test --not master --except master
test > actual &&
+       git rev-list --topo-order test > expect &&
+       test_cmp expect actual
+'
+
+test_expect_success 'rev-list multiple --not ranges' '
+
+       git checkout -b extra test &&
+       echo five > content &&
+       git commit -a -m five &&
+       git rev-list --topo-order test --not master --except master
test --not extra > actual &&
+       git rev-list --topo-order test extra > expect &&
+       test_cmp expect actual
+'
+

-- 
Felipe Contreras

Re: [PATCH] revision: introduce --exclude=<glob> to tame wildcards

[Michael Haggerty]

On 09/02/2013 10:11 PM, Johannes Sixt wrote:
> Am 31.08.2013 01:55, schrieb Junio C Hamano:
>> People often find "git log --branches" etc. that includes _all_
>> branches is cumbersome to use when they want to grab most but except
>> some.  The same applies to --tags, --all and --glob.
>>
>> Teach the revision machinery to remember patterns, and then upon the
>> next such a globbing option, exclude those that match the pattern.
>>
>> With this, I can view only my integration branches (e.g. maint,
>> master, etc.) without topic branches, which are named after two
>> letters from primary authors' names, slash and topic name.
>>
>>     git rev-list --no-walk --exclude=??/* --branches |
>>     git name-rev --refs refs/heads/* --stdin
>>
>> This one shows things reachable from local and remote branches that
>> have not been merged to the integration branches.
>>
>>     git log --remotes --branches --not --exclude=??/* --branches
>>
>> It may be a bit rough around the edges, in that the pattern to give
>> the exclude option depends on what globbing option follows.  In
>> these examples, the pattern "??/*" is used, not "refs/heads/??/*",
>> because the globbing option that follows the -"-exclude=<pattern>"
>> is "--branches".  As each use of globbing option resets previously
>> set "--exclude", this may not be such a bad thing, though.
> 
> I argued "--except should trump everything" earlier, but the case
> involving --not
> 
>   --branches --except maint --not master
> 
> to mean the same as
> 
>   --branches --except maint master
> 
> just does not make sense.
> 
> An alternative would be that --not would divide the command line
> arguments into ranges within which one --except would subtract
> subsequent refs from earlier globbing arguments in the same range.
> That's not simpler to explain than your current proposal.
> 
> So I like the relative simplicity of this approach. Here is a bit of
> documentation.
> 
> --- 8< ---
> Subject: [PATCH] document --exclude option
> 
> Signed-off-by: Johannes Sixt <j6t@kdbg.org>
> ---
>  Documentation/rev-list-options.txt | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt
> index 5bdfb42..650c252 100644
> --- a/Documentation/rev-list-options.txt
> +++ b/Documentation/rev-list-options.txt
> @@ -174,6 +174,21 @@ parents) and `--max-parents=-1` (negative numbers denote no upper limit).
>  	is automatically prepended if missing. If pattern lacks '?', '{asterisk}',
>  	or '[', '/{asterisk}' at the end is implied.
>  
> +--exclude=<glob-pattern>::
> +
> +	Do not include refs matching '<glob-pattern>' that the next `--all`,
> +	`--branches`, `--tags`, `--remotes`, or `--glob` would otherwise
> +	consider. Repetitions of this option accumulate exclusion patterns
> +	up to the next `--all`, `--branches`, `--tags`, `--remotes`, or
> +	`--glob` option (other options or arguments do not clear
> +	accumlated patterns).
> ++
> +The patterns given should not begin with `refs/heads`, `refs/tags`, or
> +`refs/remotes` when applied to `--branches`, `--tags`, or `--remotes`,
> +restrictively, and they must begin with `refs/` when applied to `--glob`

s/restrictively/respectively/

> +or `--all`. If a trailing '/{asterisk}' is intended, it must be given
> +explicitly.
> +
>  --ignore-missing::
>  
>  	Upon seeing an invalid object name in the input, pretend as if
> 

It seems to me that this is growing into a language for expressing
boolean expressions without allowing terms to be combined in the full
generality that, say, a real programming language would allow.  Maybe
instead of trying to decide on the "perfect" grouping and precedence
rules, it would be clearer to allow the user to specify them.  I almost
hate to suggest it, but have you considered making the expression
"syntax" a little bit more flexible by allowing parentheses, à la
find(1), or something analogous?:

'(' --tags --except='v[0-9]*' ')' -o '(' --branches --except='mh/*' ')'

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/

Re: [PATCH] revision: introduce --exclude=<glob> to tame wildcards

[Junio C Hamano]

Johannes Sixt <j6t@kdbg.org> writes:

> Am 31.08.2013 01:55, schrieb Junio C Hamano:
>> People often find "git log --branches" etc. that includes _all_
>> branches is cumbersome to use when they want to grab most but except
>> some.  The same applies to --tags, --all and --glob.
>> 
>> Teach the revision machinery to remember patterns, and then upon the
>> next such a globbing option, exclude those that match the pattern.
>> 
>> With this, I can view only my integration branches (e.g. maint,
>> master, etc.) without topic branches, which are named after two
>> letters from primary authors' names, slash and topic name.
>> 
>>     git rev-list --no-walk --exclude=??/* --branches |
>>     git name-rev --refs refs/heads/* --stdin
>> 
>> This one shows things reachable from local and remote branches that
>> have not been merged to the integration branches.
>> 
>>     git log --remotes --branches --not --exclude=??/* --branches
>> 
>> It may be a bit rough around the edges, in that the pattern to give
>> the exclude option depends on what globbing option follows.  In
>> these examples, the pattern "??/*" is used, not "refs/heads/??/*",
>> because the globbing option that follows the -"-exclude=<pattern>"
>> is "--branches".  As each use of globbing option resets previously
>> set "--exclude", this may not be such a bad thing, though.
>
> I argued "--except should trump everything" earlier, but the case
> involving --not
>
>   --branches --except maint --not master
>
> to mean the same as
>
>   --branches --except maint master
>
> just does not make sense.

I'll have to mull the above two over to firmly grasp what you mean
by "'except' trumping 'not'" before deciding if you are agreeing or
disagreeing with the approach of taking it as a "taming wildcard"
issue...

> An alternative would be that --not would divide the command line
> arguments into ranges within which one --except would subtract
> subsequent refs from earlier globbing arguments in the same range.
> That's not simpler to explain than your current proposal.
>
> So I like the relative simplicity of this approach. Here is a bit of
> documentation.

Oh, thanks for helping.  An example or two may also help, and using
your original "I have branches and wip/" situation would be good.

        git log --exclude='wip/*' --branches::

                Show history of local branches whose names do not
                match pattern `wip/*`.

	git log --remotes --not --exclude='wip/*' --branches::

		Show history of other people's work that are not
		merged to local branches whose names do not match
		pattern `wip/*`.

or something like that, perhaps?

> --- 8< ---
> Subject: [PATCH] document --exclude option
>
> Signed-off-by: Johannes Sixt <j6t@kdbg.org>
> ---
>  Documentation/rev-list-options.txt | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
>
> diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt
> index 5bdfb42..650c252 100644
> --- a/Documentation/rev-list-options.txt
> +++ b/Documentation/rev-list-options.txt
> @@ -174,6 +174,21 @@ parents) and `--max-parents=-1` (negative numbers denote no upper limit).
>  	is automatically prepended if missing. If pattern lacks '?', '{asterisk}',
>  	or '[', '/{asterisk}' at the end is implied.
>  
> +--exclude=<glob-pattern>::
> +
> +	Do not include refs matching '<glob-pattern>' that the next `--all`,
> +	`--branches`, `--tags`, `--remotes`, or `--glob` would otherwise
> +	consider. Repetitions of this option accumulate exclusion patterns
> +	up to the next `--all`, `--branches`, `--tags`, `--remotes`, or
> +	`--glob` option (other options or arguments do not clear
> +	accumlated patterns).
> ++
> +The patterns given should not begin with `refs/heads`, `refs/tags`, or
> +`refs/remotes` when applied to `--branches`, `--tags`, or `--remotes`,
> +restrictively, and they must begin with `refs/` when applied to `--glob`
> +or `--all`. If a trailing '/{asterisk}' is intended, it must be given
> +explicitly.
> +
>  --ignore-missing::
>  
>  	Upon seeing an invalid object name in the input, pretend as if

Re: [PATCH] revision: introduce --exclude=<glob> to tame wildcards

[Johannes Sixt]

Am 03.09.2013 18:03, schrieb Junio C Hamano:
> Johannes Sixt <j6t@kdbg.org> writes:
>> So I like the relative simplicity of this approach. Here is a bit of
>> documentation.
> 
> Oh, thanks for helping.  An example or two may also help, and using
> your original "I have branches and wip/" situation would be good.
> 
>         git log --exclude='wip/*' --branches::
> 
>                 Show history of local branches whose names do not
>                 match pattern `wip/*`.
> 
> 	git log --remotes --not --exclude='wip/*' --branches::
> 
> 		Show history of other people's work that are not
> 		merged to local branches whose names do not match
> 		pattern `wip/*`.
> 
> or something like that, perhaps?

Agreed and noted.

The new option must also be integrated in rev-parse. That will take a
bit more work.

I'll pick up the topic when I find the time to do so.

-- Hannes

[PATCH 0/5] ref glob exclusion follow-up

[Junio C Hamano]

In $gmane/234730, J6t pointed out that "rev-list --exclude" needs a
matching updates to the globbing option "rev-parse" supports.

Here is a follow-up to do just that. They are meant to be applied on
top of the two patch series that has been cooking in 'pu'.

 - $gmane/233484: e7b432c5 (revision: introduce --exclude=<glob> to
   tame wildcards, 2013-08-30)
    
 - $gmane/233656 by J6t: 5c5d4cdb (document --exclude option,
  2013-09-02) came from

The second one [4/5] in this follow-up series should probably be
squashed into [1/5] e7b432c5.  We may even want to squash these five
patches further down to two patches, one for rev-list and the other
for rev-parse.

Junio C Hamano (3):
  rev-list --exclude: tests
  rev-list --exclude: export add/clear-ref-exclusion and ref-excluded
    API
  rev-parse: introduce --exclude=<glob> to tame wildcards

 Documentation/git-rev-parse.txt | 14 +++++++++++
 builtin/rev-parse.c             | 17 +++++++++++++
 revision.c                      | 46 +++++++++++++++++------------------
 revision.h                      |  5 ++++
 t/t6018-rev-list-glob.sh        | 54 +++++++++++++++++++++++++++++++++++++++++
 5 files changed, 113 insertions(+), 23 deletions(-)

-- 
1.8.5-rc0-281-g8951339

[PATCH 3/5] rev-list --exclude: tests

[Junio C Hamano]

Add tests for the --exclude=<glob> feature.

A few tests are added for cases where use of globbing and
"--exclude" results in no positive revisions:

 * "--exclude=<glob>" before "--all" etc. resulted in no results;

 * "--stdin" is used but no input was given;

 * "--all" etc. is used but no matching refs are found.

Currently, we fail such a request with the same error message we
would give to a command line that does not specify any positive
revision (e.g. "git rev-list<ENTER>").

We may want to treat these cases differently and not error out, but
the logic to detect that would be common to all of them, so I'd
leave it outside this topic for now, and stop at adding these tests
as food-for-thought.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 t/t6018-rev-list-glob.sh | 42 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 42 insertions(+)

diff --git a/t/t6018-rev-list-glob.sh b/t/t6018-rev-list-glob.sh
index f00cebf..77ef323 100755
--- a/t/t6018-rev-list-glob.sh
+++ b/t/t6018-rev-list-glob.sh
@@ -231,6 +231,48 @@ test_expect_success 'rev-list --remotes=foo' '
 
 '
 
+test_expect_success 'rev-list --exclude with --branches' '
+	compare rev-list "--exclude=*/* --branches" "master someref subspace-x"
+'
+
+test_expect_success 'rev-list --exclude with --all' '
+	compare rev-list "--exclude=refs/remotes/* --all" "--branches --tags"
+'
+
+test_expect_success 'rev-list accumulates multiple --exclude' '
+	compare rev-list "--exclude=refs/remotes/* --exclude=refs/tags/* --all" --branches
+'
+
+
+# "git rev-list<ENTER>" is likely to be a bug in the calling script and may
+# deserve an error message, but do cases where set of refs programatically
+# given using globbing and/or --stdin need to fail with the same error, or
+# are we better off reporting a success with no output?  The following few
+# tests document the current behaviour to remind us that we might want to
+# think about this issue.
+
+test_expect_failure 'rev-list may want to succeed with empty output on no input (1)' '
+	>expect &&
+	git rev-list --stdin <expect >actual &&
+	test_cmp expect actual
+'
+
+test_expect_failure 'rev-list may want to succeed with empty output on no input (2)' '
+	>expect &&
+	git rev-list --exclude=* --all >actual &&
+	test_cmp expect actual
+'
+
+test_expect_failure 'rev-list may want to succeed with empty output on no input (3)' '
+	(
+		test_create_repo empty &&
+		cd empty &&
+		>expect &&
+		git rev-list --all >actual &&
+		test_cmp expect actual
+	)
+'
+
 test_expect_success 'shortlog accepts --glob/--tags/--remotes' '
 
 	compare shortlog "subspace/one subspace/two" --branches=subspace &&
-- 
1.8.5-rc0-281-g8951339

[PATCH 4/5] rev-list --exclude: export add/clear-ref-exclusion and ref-excluded API

[Junio C Hamano]

... while updating their function signature.  To be squashed into
the initial patch to rev-list.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 revision.c | 46 +++++++++++++++++++++++-----------------------
 revision.h |  5 +++++
 2 files changed, 28 insertions(+), 23 deletions(-)

diff --git a/revision.c b/revision.c
index 3e82874..ddc71b9 100644
--- a/revision.c
+++ b/revision.c
@@ -1180,13 +1180,13 @@ struct all_refs_cb {
 	const char *name_for_errormsg;
 };
 
-static int ref_excluded(struct rev_info *revs, const char *path)
+int ref_excluded(struct string_list *ref_excludes, const char *path)
 {
 	struct string_list_item *item;
 
-	if (!revs->ref_excludes)
+	if (!ref_excludes)
 		return 0;
-	for_each_string_list_item(item, revs->ref_excludes) {
+	for_each_string_list_item(item, ref_excludes) {
 		if (!fnmatch(item->string, path, 0))
 			return 1;
 	}
@@ -1198,7 +1198,7 @@ static int handle_one_ref(const char *path, const unsigned char *sha1, int flag,
 	struct all_refs_cb *cb = cb_data;
 	struct object *object;
 
-	if (ref_excluded(cb->all_revs, path))
+	if (ref_excluded(cb->all_revs->ref_excludes, path))
 	    return 0;
 
 	object = get_reference(cb->all_revs, path, sha1, cb->all_flags);
@@ -1214,22 +1214,22 @@ static void init_all_refs_cb(struct all_refs_cb *cb, struct rev_info *revs,
 	cb->all_flags = flags;
 }
 
-static void clear_ref_exclusion(struct rev_info *revs)
+void clear_ref_exclusion(struct string_list **ref_excludes_p)
 {
-	if (revs->ref_excludes) {
-		string_list_clear(revs->ref_excludes, 0);
-		free(revs->ref_excludes);
+	if (*ref_excludes_p) {
+		string_list_clear(*ref_excludes_p, 0);
+		free(*ref_excludes_p);
 	}
-	revs->ref_excludes = NULL;
+	*ref_excludes_p = NULL;
 }
 
-static void add_ref_exclusion(struct rev_info *revs, const char *exclude)
+void add_ref_exclusion(struct string_list **ref_excludes_p, const char *exclude)
 {
-	if (!revs->ref_excludes) {
-		revs->ref_excludes = xcalloc(1, sizeof(*revs->ref_excludes));
-		revs->ref_excludes->strdup_strings = 1;
+	if (!*ref_excludes_p) {
+		*ref_excludes_p = xcalloc(1, sizeof(**ref_excludes_p));
+		(*ref_excludes_p)->strdup_strings = 1;
 	}
-	string_list_append(revs->ref_excludes, exclude);
+	string_list_append(*ref_excludes_p, exclude);
 }
 
 static void handle_refs(const char *submodule, struct rev_info *revs, unsigned flags,
@@ -1988,44 +1988,44 @@ static int handle_revision_pseudo_opt(const char *submodule,
 	if (!strcmp(arg, "--all")) {
 		handle_refs(submodule, revs, *flags, for_each_ref_submodule);
 		handle_refs(submodule, revs, *flags, head_ref_submodule);
-		clear_ref_exclusion(revs);
+		clear_ref_exclusion(&revs->ref_excludes);
 	} else if (!strcmp(arg, "--branches")) {
 		handle_refs(submodule, revs, *flags, for_each_branch_ref_submodule);
-		clear_ref_exclusion(revs);
+		clear_ref_exclusion(&revs->ref_excludes);
 	} else if (!strcmp(arg, "--bisect")) {
 		handle_refs(submodule, revs, *flags, for_each_bad_bisect_ref);
 		handle_refs(submodule, revs, *flags ^ (UNINTERESTING | BOTTOM), for_each_good_bisect_ref);
 		revs->bisect = 1;
 	} else if (!strcmp(arg, "--tags")) {
 		handle_refs(submodule, revs, *flags, for_each_tag_ref_submodule);
-		clear_ref_exclusion(revs);
+		clear_ref_exclusion(&revs->ref_excludes);
 	} else if (!strcmp(arg, "--remotes")) {
 		handle_refs(submodule, revs, *flags, for_each_remote_ref_submodule);
-		clear_ref_exclusion(revs);
+		clear_ref_exclusion(&revs->ref_excludes);
 	} else if ((argcount = parse_long_opt("glob", argv, &optarg))) {
 		struct all_refs_cb cb;
 		init_all_refs_cb(&cb, revs, *flags);
 		for_each_glob_ref(handle_one_ref, optarg, &cb);
-		clear_ref_exclusion(revs);
+		clear_ref_exclusion(&revs->ref_excludes);
 		return argcount;
 	} else if ((argcount = parse_long_opt("exclude", argv, &optarg))) {
-		add_ref_exclusion(revs, optarg);
+		add_ref_exclusion(&revs->ref_excludes, optarg);
 		return argcount;
 	} else if (!prefixcmp(arg, "--branches=")) {
 		struct all_refs_cb cb;
 		init_all_refs_cb(&cb, revs, *flags);
 		for_each_glob_ref_in(handle_one_ref, arg + 11, "refs/heads/", &cb);
-		clear_ref_exclusion(revs);
+		clear_ref_exclusion(&revs->ref_excludes);
 	} else if (!prefixcmp(arg, "--tags=")) {
 		struct all_refs_cb cb;
 		init_all_refs_cb(&cb, revs, *flags);
 		for_each_glob_ref_in(handle_one_ref, arg + 7, "refs/tags/", &cb);
-		clear_ref_exclusion(revs);
+		clear_ref_exclusion(&revs->ref_excludes);
 	} else if (!prefixcmp(arg, "--remotes=")) {
 		struct all_refs_cb cb;
 		init_all_refs_cb(&cb, revs, *flags);
 		for_each_glob_ref_in(handle_one_ref, arg + 10, "refs/remotes/", &cb);
-		clear_ref_exclusion(revs);
+		clear_ref_exclusion(&revs->ref_excludes);
 	} else if (!strcmp(arg, "--reflog")) {
 		handle_reflog(revs, *flags);
 	} else if (!strcmp(arg, "--not")) {
diff --git a/revision.h b/revision.h
index b4dc56c..c67c46d 100644
--- a/revision.h
+++ b/revision.h
@@ -192,6 +192,11 @@ struct rev_info {
 	struct decoration line_log_data;
 };
 
+extern int ref_excluded(struct string_list *, const char *path);
+void clear_ref_exclusion(struct string_list **);
+void add_ref_exclusion(struct string_list **, const char *exclude);
+
+
 #define REV_TREE_SAME		0
 #define REV_TREE_NEW		1	/* Only new files */
 #define REV_TREE_OLD		2	/* Only files removed */
-- 
1.8.5-rc0-281-g8951339

[PATCH 5/5] rev-parse: introduce --exclude=<glob> to tame wildcards

[Junio C Hamano]

Teach "rev-parse" the same "I'm going to glob, but omit the ones
that match these patterns" feature as "rev-list".

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/git-rev-parse.txt | 14 ++++++++++++++
 builtin/rev-parse.c             | 17 +++++++++++++++++
 t/t6018-rev-list-glob.sh        | 12 ++++++++++++
 3 files changed, 43 insertions(+)

diff --git a/Documentation/git-rev-parse.txt b/Documentation/git-rev-parse.txt
index 2b126c0..d4639a2 100644
--- a/Documentation/git-rev-parse.txt
+++ b/Documentation/git-rev-parse.txt
@@ -155,6 +155,20 @@ shown.  If the pattern does not contain a globbing character (`?`,
 	character (`?`, `*`, or `[`), it is turned into a prefix
 	match by appending `/*`.
 
+--exclude=<glob-pattern>::
+	Do not include refs matching '<glob-pattern>' that the next `--all`,
+	`--branches`, `--tags`, `--remotes`, or `--glob` would otherwise
+	consider. Repetitions of this option accumulate exclusion patterns
+	up to the next `--all`, `--branches`, `--tags`, `--remotes`, or
+	`--glob` option (other options or arguments do not clear
+	accumlated patterns).
++
+The patterns given should not begin with `refs/heads`, `refs/tags`, or
+`refs/remotes` when applied to `--branches`, `--tags`, or `--remotes`,
+restrictively, and they must begin with `refs/` when applied to `--glob`
+or `--all`. If a trailing '/{asterisk}' is intended, it must be given
+explicitly.
+
 --show-toplevel::
 	Show the absolute path of the top-level directory.
 
diff --git a/builtin/rev-parse.c b/builtin/rev-parse.c
index de894c7..f52f804 100644
--- a/builtin/rev-parse.c
+++ b/builtin/rev-parse.c
@@ -9,6 +9,8 @@
 #include "quote.h"
 #include "builtin.h"
 #include "parse-options.h"
+#include "diff.h"
+#include "revision.h"
 
 #define DO_REVS		1
 #define DO_NOREV	2
@@ -30,6 +32,8 @@ static int abbrev_ref;
 static int abbrev_ref_strict;
 static int output_sq;
 
+static struct string_list *ref_excludes;
+
 /*
  * Some arguments are relevant "revision" arguments,
  * others are about output format or other details.
@@ -185,6 +189,8 @@ static int show_default(void)
 
 static int show_reference(const char *refname, const unsigned char *sha1, int flag, void *cb_data)
 {
+	if (ref_excluded(ref_excludes, refname))
+		return 0;
 	show_rev(NORMAL, sha1, refname);
 	return 0;
 }
@@ -633,32 +639,43 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
 			if (!prefixcmp(arg, "--branches=")) {
 				for_each_glob_ref_in(show_reference, arg + 11,
 					"refs/heads/", NULL);
+				clear_ref_exclusion(&ref_excludes);
 				continue;
 			}
 			if (!strcmp(arg, "--branches")) {
 				for_each_branch_ref(show_reference, NULL);
+				clear_ref_exclusion(&ref_excludes);
 				continue;
 			}
 			if (!prefixcmp(arg, "--tags=")) {
 				for_each_glob_ref_in(show_reference, arg + 7,
 					"refs/tags/", NULL);
+				clear_ref_exclusion(&ref_excludes);
 				continue;
 			}
 			if (!strcmp(arg, "--tags")) {
 				for_each_tag_ref(show_reference, NULL);
+				clear_ref_exclusion(&ref_excludes);
 				continue;
 			}
 			if (!prefixcmp(arg, "--glob=")) {
 				for_each_glob_ref(show_reference, arg + 7, NULL);
+				clear_ref_exclusion(&ref_excludes);
 				continue;
 			}
 			if (!prefixcmp(arg, "--remotes=")) {
 				for_each_glob_ref_in(show_reference, arg + 10,
 					"refs/remotes/", NULL);
+				clear_ref_exclusion(&ref_excludes);
 				continue;
 			}
 			if (!strcmp(arg, "--remotes")) {
 				for_each_remote_ref(show_reference, NULL);
+				clear_ref_exclusion(&ref_excludes);
+				continue;
+			}
+			if (!prefixcmp(arg, "--exclude=")) {
+				add_ref_exclusion(&ref_excludes, arg + 10);
 				continue;
 			}
 			if (!strcmp(arg, "--show-toplevel")) {
diff --git a/t/t6018-rev-list-glob.sh b/t/t6018-rev-list-glob.sh
index 77ef323..d00f7db 100755
--- a/t/t6018-rev-list-glob.sh
+++ b/t/t6018-rev-list-glob.sh
@@ -129,6 +129,18 @@ test_expect_success 'rev-parse --remotes=foo' '
 
 '
 
+test_expect_success 'rev-parse --exclude with --branches' '
+	compare rev-parse "--exclude=*/* --branches" "master someref subspace-x"
+'
+
+test_expect_success 'rev-parse --exclude with --all' '
+	compare rev-parse "--exclude=refs/remotes/* --all" "--branches --tags"
+'
+
+test_expect_success 'rev-parse accumulates multiple --exclude' '
+	compare rev-parse "--exclude=refs/remotes/* --exclude=refs/tags/* --all" --branches
+'
+
 test_expect_success 'rev-list --glob=refs/heads/subspace/*' '
 
 	compare rev-list "subspace/one subspace/two" "--glob=refs/heads/subspace/*"
-- 
1.8.5-rc0-281-g8951339

Re: [PATCH 5/5] rev-parse: introduce --exclude=<glob> to tame wildcards

[Eric Sunshine]

On Fri, Nov 1, 2013 at 3:34 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Teach "rev-parse" the same "I'm going to glob, but omit the ones
> that match these patterns" feature as "rev-list".
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  Documentation/git-rev-parse.txt | 14 ++++++++++++++
>  builtin/rev-parse.c             | 17 +++++++++++++++++
>  t/t6018-rev-list-glob.sh        | 12 ++++++++++++
>  3 files changed, 43 insertions(+)
>
> diff --git a/Documentation/git-rev-parse.txt b/Documentation/git-rev-parse.txt
> index 2b126c0..d4639a2 100644
> --- a/Documentation/git-rev-parse.txt
> +++ b/Documentation/git-rev-parse.txt
> @@ -155,6 +155,20 @@ shown.  If the pattern does not contain a globbing character (`?`,
>         character (`?`, `*`, or `[`), it is turned into a prefix
>         match by appending `/*`.
>
> +--exclude=<glob-pattern>::
> +       Do not include refs matching '<glob-pattern>' that the next `--all`,
> +       `--branches`, `--tags`, `--remotes`, or `--glob` would otherwise
> +       consider. Repetitions of this option accumulate exclusion patterns
> +       up to the next `--all`, `--branches`, `--tags`, `--remotes`, or
> +       `--glob` option (other options or arguments do not clear
> +       accumlated patterns).
> ++
> +The patterns given should not begin with `refs/heads`, `refs/tags`, or
> +`refs/remotes` when applied to `--branches`, `--tags`, or `--remotes`,
> +restrictively, and they must begin with `refs/` when applied to `--glob`

Did you mean s/restrictively/respectively/ ?

> +or `--all`. If a trailing '/{asterisk}' is intended, it must be given
> +explicitly.

Re: [PATCH 5/5] rev-parse: introduce --exclude=<glob> to tame wildcards

[Junio C Hamano]

Eric Sunshine <sunshine@sunshineco.com> writes:

> On Fri, Nov 1, 2013 at 3:34 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Teach "rev-parse" the same "I'm going to glob, but omit the ones
>> that match these patterns" feature as "rev-list".
>>
>> Signed-off-by: Junio C Hamano <gitster@pobox.com>
>> ---
>>  Documentation/git-rev-parse.txt | 14 ++++++++++++++
>>  builtin/rev-parse.c             | 17 +++++++++++++++++
>>  t/t6018-rev-list-glob.sh        | 12 ++++++++++++
>>  3 files changed, 43 insertions(+)
>>
>> diff --git a/Documentation/git-rev-parse.txt b/Documentation/git-rev-parse.txt
>> index 2b126c0..d4639a2 100644
>> --- a/Documentation/git-rev-parse.txt
>> +++ b/Documentation/git-rev-parse.txt
>> @@ -155,6 +155,20 @@ shown.  If the pattern does not contain a globbing character (`?`,
>>         character (`?`, `*`, or `[`), it is turned into a prefix
>>         match by appending `/*`.
>>
>> +--exclude=<glob-pattern>::
>> +       Do not include refs matching '<glob-pattern>' that the next `--all`,
>> +       `--branches`, `--tags`, `--remotes`, or `--glob` would otherwise
>> +       consider. Repetitions of this option accumulate exclusion patterns
>> +       up to the next `--all`, `--branches`, `--tags`, `--remotes`, or
>> +       `--glob` option (other options or arguments do not clear
>> +       accumlated patterns).
>> ++
>> +The patterns given should not begin with `refs/heads`, `refs/tags`, or
>> +`refs/remotes` when applied to `--branches`, `--tags`, or `--remotes`,
>> +restrictively, and they must begin with `refs/` when applied to `--glob`
>
> Did you mean s/restrictively/respectively/ ?

I guess so; it was "cut&paste without thinking" from [Patch 2/5] ;-).

>
>> +or `--all`. If a trailing '/{asterisk}' is intended, it must be given
>> +explicitly.

Re: [PATCH 0/5] ref glob exclusion follow-up

[Johannes Sixt]

Am 01.11.2013 20:34, schrieb Junio C Hamano:
> In $gmane/234730, J6t pointed out that "rev-list --exclude" needs a
> matching updates to the globbing option "rev-parse" supports.
> 
> Here is a follow-up to do just that. They are meant to be applied on
> top of the two patch series that has been cooking in 'pu'.

Thanks a lot! I took a stab at the task the other day. I intended to
weave the exclusion matching into the do_for_each_* functions in refs.c.
But I quickly found that I would take me much more time than I was able
to spend.

-- Hannes

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

[Johannes Sixt]

Am 8/30/2013 8:32, schrieb Junio C Hamano:
> If you have a history where
> 
>  - branches "master" and "maint" point at commit A;
>  - branch "next" points at commit B that is a descendant of A; and
>  - there are tags X and Y pointing at commits that are ahead of B
>    or behind A
> 
> i.e.
> 
> 	----X----A----B----Y
> 
> what are the desired semantics for these?

I think the simplest were that --except trumps everything and means
"whatever else I say, do as if I did not mention the following".

>  (1) --branches --except maint

=> master next

>  (2) --all --not --branches --except maint

=> X Y --not master next

>  (3) ^master next --except maint

=> ^master next

What should the following mean? Does --not forget that --except was
earlier on the command line?

(4) Y next --except master next --not --branches

this => Y --not maint
or this => Y --not maint master next

What about this:

(5) --branches --except ^master

this => maint next
or this => maint master next ^master
or error("--except does not allow negated revisions")

-- Hannes

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

[Felipe Contreras]

On Fri, Aug 30, 2013 at 2:56 AM, Johannes Sixt <j.sixt@viscovery.net> wrote:
> Am 8/30/2013 8:32, schrieb Junio C Hamano:
>> If you have a history where
>>
>>  - branches "master" and "maint" point at commit A;
>>  - branch "next" points at commit B that is a descendant of A; and
>>  - there are tags X and Y pointing at commits that are ahead of B
>>    or behind A
>>
>> i.e.
>>
>>       ----X----A----B----Y
>>
>> what are the desired semantics for these?
>
> I think the simplest were that --except trumps everything and means
> "whatever else I say, do as if I did not mention the following".

Actually, my patch is almost there, I attach the necessary changed
below to make everything work. I've added debug prints to show what
it's actually doing:

>>  (1) --branches --except maint
>
> => master next

=> master next

>>  (2) --all --not --branches --except maint
>
> => X Y --not master next

=> ^master ^next X Y HEAD

>>  (3) ^master next --except maint
>
> => ^master next

=> ^master next

> (4) Y next --except master next --not --branches
>
> this => Y --not maint
> or this => Y --not maint master next

=> Y

Remember that maint (or rather ^maint) is after --except.

> (5) --branches --except ^master
>
> this => maint next
> or this => maint master next ^master
> or error("--except does not allow negated revisions")

=> maint next

Here's the diff:

--- a/revision.c
+++ b/revision.c
@@ -2578,7 +2578,11 @@ void reset_revision_walk(void)
 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);
 }

@@ -2594,13 +2598,14 @@ int prepare_revision_walk(struct rev_info *revs)
        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))
                                goto next;
                }
+               commit = handle_commit(revs, e->item, e->name);
                if (commit) {
                        if (!(commit->object.flags & SEEN)) {
                                commit->object.flags |= SEEN;

-- 
Felipe Contreras

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

[Johannes Sixt]

Am 8/31/2013 21:27, schrieb Felipe Contreras:
> On Fri, Aug 30, 2013 at 2:56 AM, Johannes Sixt <j.sixt@viscovery.net> wrote:
>> Am 8/30/2013 8:32, schrieb Junio C Hamano:
>>> If you have a history where
>>>
>>>  - branches "master" and "maint" point at commit A;
>>>  - branch "next" points at commit B that is a descendant of A; and
>>>  - there are tags X and Y pointing at commits that are ahead of B
>>>    or behind A
>>>
>>> i.e.
>>>
>>>       ----X----A----B----Y
>>>
>>> what are the desired semantics for these?
>>
>> I think the simplest were that --except trumps everything and means
>> "whatever else I say, do as if I did not mention the following".
> 
> Actually, my patch is almost there, I attach the necessary changed
> below to make everything work. I've added debug prints to show what
> it's actually doing:
> 
>>>  (1) --branches --except maint
>>
>> => master next
> 
> => master next
> 
>>>  (2) --all --not --branches --except maint
>>
>> => X Y --not master next
> 
> => ^master ^next X Y HEAD
> 
>>>  (3) ^master next --except maint
>>
>> => ^master next
> 
> => ^master next
> 
>> (4) Y next --except master next --not --branches
>>
>> this => Y --not maint
>> or this => Y --not maint master next
> 
> => Y
> 
> Remember that maint (or rather ^maint) is after --except.

Sure, but why is it not in the result? maint is not even mentioned under
--except. Confused...

Ah, are you treating the union of master, next, and --branches as --except
and ignore --not?

-- Hannes

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

[Felipe Contreras]

On Mon, Sep 2, 2013 at 1:25 AM, Johannes Sixt <j.sixt@viscovery.net> wrote:
> Am 8/31/2013 21:27, schrieb Felipe Contreras:
>> On Fri, Aug 30, 2013 at 2:56 AM, Johannes Sixt <j.sixt@viscovery.net> wrote:
>>> Am 8/30/2013 8:32, schrieb Junio C Hamano:
>>>> If you have a history where
>>>>
>>>>  - branches "master" and "maint" point at commit A;
>>>>  - branch "next" points at commit B that is a descendant of A; and
>>>>  - there are tags X and Y pointing at commits that are ahead of B
>>>>    or behind A
>>>>
>>>> i.e.
>>>>
>>>>       ----X----A----B----Y
>>>>
>>>> what are the desired semantics for these?
>>>
>>> I think the simplest were that --except trumps everything and means
>>> "whatever else I say, do as if I did not mention the following".
>>
>> Actually, my patch is almost there, I attach the necessary changed
>> below to make everything work. I've added debug prints to show what
>> it's actually doing:
>>
>>>>  (1) --branches --except maint
>>>
>>> => master next
>>
>> => master next
>>
>>>>  (2) --all --not --branches --except maint
>>>
>>> => X Y --not master next
>>
>> => ^master ^next X Y HEAD
>>
>>>>  (3) ^master next --except maint
>>>
>>> => ^master next
>>
>> => ^master next
>>
>>> (4) Y next --except master next --not --branches
>>>
>>> this => Y --not maint
>>> or this => Y --not maint master next
>>
>> => Y
>>
>> Remember that maint (or rather ^maint) is after --except.
>
> Sure, but why is it not in the result? maint is not even mentioned under
> --except. Confused...

It is mentioned under --except, by --branches.

> Ah, are you treating the union of master, next, and --branches as --except
> and ignore --not?

These are the same:

Y next --except master next --not --branches
Y next --except master next --not master next maint

If you add more positive branches:

They get removed anyway by --except:

Y next master maint --except master next --not master next maint
Y master maint --except master --not master maint
Y maint --except --not maint
Y

-- 
Felipe Contreras

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

[Johannes Sixt]

Am 8/30/2013 7:00, schrieb 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:
> 
>   * 374e8dd (crap) crap
>   * 4cbbf7b (test) two
>   * d025ae0 (HEAD, master) one
> 
> When using '--branches --except crap':
> 
>   * 4cbbf7b (test) two
>   * d025ae0 (HEAD, master) one
> 
> But when using '--branches --not crap' nothing will come out.

I like the idea to be able to exclude refs from listings. My use-case is
the following:

To unclutter 'git branch' output, I rename work-in-progress branches to
begin with "wip/", ready-to-merge branches do not carry this prefix. To
inspect unmerged work of the latter kind of branches I would like to
type... what?

  gitk --branches --except --branches=wip --not master
  gitk --branches --not master --except --branches=wip

Would one of these work with your current patch?

-- Hannes

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

[Felipe Contreras]

On Fri, Aug 30, 2013 at 2:11 AM, Johannes Sixt <j.sixt@viscovery.net> wrote:
> Am 8/30/2013 7:00, schrieb 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:
>>
>>   * 374e8dd (crap) crap
>>   * 4cbbf7b (test) two
>>   * d025ae0 (HEAD, master) one
>>
>> When using '--branches --except crap':
>>
>>   * 4cbbf7b (test) two
>>   * d025ae0 (HEAD, master) one
>>
>> But when using '--branches --not crap' nothing will come out.
>
> I like the idea to be able to exclude refs from listings. My use-case is
> the following:
>
> To unclutter 'git branch' output, I rename work-in-progress branches to
> begin with "wip/", ready-to-merge branches do not carry this prefix. To
> inspect unmerged work of the latter kind of branches I would like to
> type... what?
>
>   gitk --branches --except --branches=wip --not master
>   gitk --branches --not master --except --branches=wip
>
> Would one of these work with your current patch?

The first one should work, but mostly by accident. A proper fix is not
far though:

diff --git a/revision.c b/revision.c
index 25564c1..1380cd1 100644
--- a/revision.c
+++ b/revision.c
@@ -1983,9 +1983,9 @@ static int handle_revision_pseudo_opt(const char
*submodule,
        } else if (!strcmp(arg, "--reflog")) {
                handle_reflog(revs, *flags);
        } else if (!strcmp(arg, "--not")) {
-               *flags ^= UNINTERESTING | BOTTOM;
+               *flags = UNINTERESTING | BOTTOM;
        } else if (!strcmp(arg, "--except")) {
-               *flags ^= SKIP;
+               *flags = SKIP;
        } else if (!strcmp(arg, "--no-walk")) {
                revs->no_walk = REVISION_WALK_NO_WALK_SORTED;
        } else if (!prefixcmp(arg, "--no-walk=")) {

Then both should work.

-- 
Felipe Contreras