Re: [PATCH] specifying ranges: we did not mean to make ".." an empty set

[Jeff King <peff@peff.net>]

On Wed, Aug 22, 2012 at 03:59:43PM -0700, Junio C Hamano wrote:

> Date: Mon, 2 May 2011 13:39:16 -0700
> 
> Either end of revision range operator can be omitted to default to HEAD,
> as in "origin.." (what did I do since I forked) or "..origin" (what did
> they do since I forked).  But the current parser interprets ".."  as an
> empty range "HEAD..HEAD", and worse yet, because ".." does exist on the
> filesystem, we get this annoying output:
> 
>   $ cd Documentation/howto
>   $ git log .. ;# give me recent commits that touch Documentation/ area.
>   fatal: ambiguous argument '..': both revision and filename
>   Use '--' to separate filenames from revisions
> 
> Surely we could say "git log ../" or even "git log -- .." to disambiguate,
> but we shouldn't have to.
> 
> Helped-by: Jeff King <peff@peff.net>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Hmm, for some reason I had no recollection of the original thread at
all. And yet reading the archives, I apparently had quite a bit to say.
Reading again with fresh eyes, I still think this is sane.

I don't think assigning any revision magic to ".." besides "the empty
range" makes sense at all for the reasons you gave in the original
thread. And the empty range is a pointless no-op. So I don't see any
real argument in favor of disambiguating towards the revision.

So the only reasonable choices are to leave it as an error, or to
disambiguate towards the pathspec.  I suppose some script could stupidly
be doing "git log $a..$b" and would prefer to error out when both are
blank. But that seems unlikely, and making ".." work is actually useful.

> --- a/Documentation/revisions.txt
> +++ b/Documentation/revisions.txt
> @@ -213,6 +213,13 @@ of 'r1' and 'r2' and is defined as
>  It is the set of commits that are reachable from either one of
>  'r1' or 'r2' but not from both.
>  
> +In these two shorthands, you can omit one end and let it default to HEAD.
> +For example, 'origin..' is a shorthand for 'origin..HEAD' and asks "What
> +did I do since I forked from the origin branch?"  Similarly, '..origin'
> +is a shorthand for 'HEAD..origin' and asks "What did the origin do since
> +I forked from them?"  Note that '..' would mean 'HEAD..HEAD' which is an
> +empty range that is both reachable and unreachable from HEAD.

This last sentence confuses me. Now we are documenting that "yes, ..
really means HEAD..HEAD, which is the empty range". But isn't the point
of this patch to say "sure, it would be the empty range, but because
that is stupid and pointless, we do not consider it valid and treat ..
as a pathspec"?

I think that may be what you are trying to say with the "would" in that
sentence, but perhaps this would be a good point to expand and mention
that we special-case "..".

> +test_expect_success 'dotdot is not an empty set' '
> +	( H=$(git rev-parse HEAD) && echo $H ; echo ^$H ) >expect &&

It almost certainly doesn't matter in practice, but the ';' here would
break the &&-chain from rev-parse.