Re: [PATCH 3/3] rev-list --min-parents,--max-parents: doc and test and completion

From: Michael J Gruber <git@drmicha.warpmail.net>
Date: Mar 21, 2011, 09:01 UTC
Message-ID: <4D871401.6020900@drmicha.warpmail.net>
In-Reply-To: <20110318194802.GB27825@sigill.intra.peff.net>

(Working my way backwards through the responses.)

Jeff King venit, vidit, dixit 18.03.2011 20:48:

Show 11 quoted lines

> On Fri, Mar 18, 2011 at 03:50:25PM +0100, Michael J Gruber wrote:
> 
>> +--min-parents::
>> +--max-parents::
>> +
>> +	Show only commits which have at least resp. at most that many
>> +	commits, where `--max-parents=8` denotes infinity (i.e. no upper
>> +	limit). In fact, 7 (or any negative number) does, but 8 is
>> +	infinity sideways 8-)
> 
> I didn't quite parse this "resp." in the middle.

Well, there are two options (--min-parents, --max-parents) which we describe in one paragraph. Sooo...

Show 37 quoted lines

> 
>> ++
>> +In particular, `--max-parents=1` is `--no-merges`, `--min-parents=2` is
>> +`--merges` (only), `--max-parents=0` gives all root commits and
>> +`--min-parents=3` all octopusses.
> 
> Spelling nit: the plural of octopus is "octopuses" (or "octopi" or
> "octopodes", depending on which dictionary you consult).
> 
> I think it's good to equate --{no-,}merges with their
> --{max,min}-parents counterparts here. We should probably do the same
> for the reverse association, like:
> 
> diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt
> index fcc4c6c..f32509a 100644
> --- a/Documentation/rev-list-options.txt
> +++ b/Documentation/rev-list-options.txt
> @@ -226,11 +226,13 @@ endif::git-rev-list[]
>  
>  --merges::
>  
> -	Print only merge commits.
> +	Print only merge commits. This is equivalent to
> +	`--min-parents=2`.
>  
>  --no-merges::
>  
> -	Do not print commits with more than one parent.
> +	Do not print commits with more than one parent. This is
> +	equivalent to `--max-parents=1`.
>  
>  --min-parents::
>  --max-parents::
> 
> That way it is obvious that "--merges" cancels a previous --min-parents
> on the command line (maybe the text should be "this is an alias for..."
> to make it clear that doing it is exactly the same).

Yes, that is helpful. I have doubts about "alias" for. Without wanting to sound elitist or something, I have the impression that we start catering for users who understand "equivalent" more reliably than "alias".

> This provides a user-friendly form for the two common cases. Do we care
> about the other cases, or adding a single multi-state flag like
> --show-parents={all,merges,etc}? They other ones are rare enough and

I know that name is not a serious suggestion...

> particular enough that it is probably fine for people to just use
> {min,max}-parents directly. So I would say what you have is good.

Thanks :)

I would go for something multi-state like only If I were allowed to stamp on the existing --merges/--no-merges, which I am not. ;)

Michael