From: Jeff King <peff@peff.net>
Date: Fri, 18 Mar 2011 19:48:02 GMT
Subject: Re: [PATCH 3/3] rev-list --min-parents,--max-parents: doc and test and completion
Message-ID: <20110318194802.GB27825@sigill.intra.peff.net>
In-Reply-To: <182c5ece22835559a9762e242adb701547cf6575.1300459017.git.git@drmicha.warpmail.net>

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.

> ++
> +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).

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

-Peff