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

2013-09-01 Thread Felipe Contreras
On Mon, Sep 2, 2013 at 1:25 AM, Johannes Sixt  wrote:
> Am 8/31/2013 21:27, schrieb Felipe Contreras:
>> On Fri, Aug 30, 2013 at 2:56 AM, Johannes Sixt  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.

   XABY

 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2013-09-01 Thread Johannes Sixt
Am 8/31/2013 21:27, schrieb Felipe Contreras:
> On Fri, Aug 30, 2013 at 2:56 AM, Johannes Sixt  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.
>>>
>>>   XABY
>>>
>>> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2013-08-31 Thread Felipe Contreras
On Fri, Aug 30, 2013 at 2:56 AM, Johannes Sixt  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.
>>
>>   XABY
>>
>> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2013-08-30 Thread Felipe Contreras
On Fri, Aug 30, 2013 at 11:48 AM, Junio C Hamano  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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2013-08-30 Thread Junio C Hamano
Felipe Contreras  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

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2013-08-30 Thread Johannes Sixt
Am 8/30/2013 9:32, schrieb Felipe Contreras:
> On Fri, Aug 30, 2013 at 2:26 AM, Junio C Hamano  wrote:
>> On Aug 30, 2013 12:19 AM, "Felipe Contreras" 
>> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2013-08-30 Thread 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.
> 
>   XABY
> 
> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2013-08-30 Thread Felipe Contreras
On Fri, Aug 30, 2013 at 2:26 AM, Junio C Hamano  wrote:
> Pardon terseness, typo and HTML from a tablet.
>
>
> On Aug 30, 2013 12:19 AM, "Felipe Contreras" 
> wrote:
>>
>> On Fri, Aug 30, 2013 at 1:32 AM, Junio C Hamano  wrote:
>> > Felipe Contreras  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.
>> >
>> > XABY
>> >
>> > 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2013-08-30 Thread Felipe Contreras
On Fri, Aug 30, 2013 at 2:11 AM, Johannes Sixt  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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2013-08-30 Thread Felipe Contreras
On Fri, Aug 30, 2013 at 1:32 AM, Junio C Hamano  wrote:
> Felipe Contreras  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.
>
> XABY
>
> 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2013-08-30 Thread 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2013-08-29 Thread Junio C Hamano
Felipe Contreras  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.

XABY

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 000..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.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2013-08-29 Thread Felipe Contreras
On Fri, Aug 30, 2013 at 12:00 AM, Felipe Contreras
 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
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH] revision: add --except option

2013-08-29 Thread 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 
---
 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_REFS1
 #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 000..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 -