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

From: Junio C Hamano <gitster@pobox.com>
Date: Aug 30, 2013, 16:48 UTC
Message-ID: <xmqqeh9b15x6.fsf@gitster.dls.corp.google.com>
In-Reply-To: <CAMP44s0P=XF5C8+fU2cJ-Xuq57iqcAn674Upub6N=+iiMpQK0g@mail.gmail.com>

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

Show 16 quoted lines

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