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

> 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?