Re: [PATCH] doc: log takes multiple revision ranges

From: Taylor Blau <me@ttaylorr.com>
Date: Mar 31, 2020, 21:53 UTC
Message-ID: <20200331215356.GC95875@syl.local>
In-Reply-To: <xmqqd08wc5bd.fsf@gitster.c.googlers.com>

On Sat, Mar 28, 2020 at 06:34:14PM -0700, Junio C Hamano wrote:

Show 32 quoted lines

> "Philippe Blain via GitGitGadget" <gitgitgadget@gmail.com> writes:
>
> > From: Philippe Blain <levraiphilippeblain@gmail.com>
> >
> > `git log` accepts multiple revision ranges, but makes no mention of that
> > in its documentation. Currently readers have to go to the `git
> > rev-list` documentation to discover this useful feature.
>
> I have a mixed feeling about this change.  Surely you can write
>
> 	git log   v1.0..v2.0   v4.0..v5.0 -- Makefile
>
> and you can explain that the command appears to take two "ranges",
> but I do not think we want to encourage it to those who do not
> understand what the above actually *means*.
>
> Specifially, it does *NOT* list commits between v1.0 and v2.0, or
> between v4.0 and v5.0, that touch the path Makefile.  In other
> words, you didn't really give two ranges with the syntax.  What the
> A..B notation means could be called a range, only when it appears
> alone.
>
> And we have *no* intention of changing the semantics.
>
> "A..B" is a mere short-hand for ^A B, and it may appear to be a
> range, but "A..B C..D" does not make "two ranges".  It still merely
> is a short-hand for ^A B ^C D, and if C reaches A and B (in the
> above example, v4.0 is very likely to reach both v1.0 and v2.0), it
> means the same thing as "C..D", i.e. "git log v4.0..v5.0 -- Makefile".
>
> So I have a mildly strong opinion against the change to the synopsys
> and the short-help; it is a bad change that does not help users.

I second this opinion. I think that reading:

  [<revision range> [...]]

is very misleading. It guides the reader to think that they can specify multiple ranges, but does not provide clear guidance about how multiple ranges are interpreted. I think most readers will naturally assume that the interpretation is "take the union of each provided range", but this is not the case as Junio points out.

Show 7 quoted lines

> If there are not sufficient explanation on the equivalence between
> A..B and ^A B on "git log" documentation but there is one elsewhere,
> adding a reference to help users learn is very much appreciated,
> though, and I initially was hoping that the second hunk of this patch
> was such a change, but it appears that it is not (it just adds the
> misleading "you can have more than one" without making readers aware
> of what it means to write "A..B C..D").

I think the documentation is probably clearer as it is. Having this extra explanation about the difference between the two doesn't guarantee that it will be read, so it may just be adding additional complexity that isn't warranted ;).

> Thanks.

Thanks, Taylor