All of lore.kernel.org
 help / color / mirror / Atom feed
From: Felipe Contreras <felipe.contreras@gmail.com>
To: Elijah Newren <newren@gmail.com>,
	Felipe Contreras <felipe.contreras@gmail.com>
Cc: Eric Sunshine <sunshine@sunshineco.com>,
	Git List <git@vger.kernel.org>,
	Bagas Sanjaya <bagasdotme@gmail.com>,
	Junio C Hamano <gitster@pobox.com>
Subject: Re: [PATCH] doc: revisions: improve single range explanation
Date: Sun, 13 Jun 2021 12:09:23 -0500	[thread overview]
Message-ID: <60c63bc3f616_41f452086b@natae.notmuch> (raw)
In-Reply-To: <CABPp-BE4r=Nhw2sJS++7Eh1K5rpyWgg+f8vDTf92JBayt1B_cA@mail.gmail.com>

Elijah Newren wrote:
> On Sat, Jun 12, 2021 at 9:25 PM Felipe Contreras
> <felipe.contreras@gmail.com> wrote:
> >
> > Eric Sunshine wrote:
> > > On Sat, Jun 12, 2021 at 8:44 PM Felipe Contreras
> > > <felipe.contreras@gmail.com> wrote:
> > > > The original explanation didn't seem clear enough to some people.
> > > >
> > > > Signed-off-by: Felipe Contreras <felipe.contreras@gmail.com>
> > > > ---
> > > > diff --git a/Documentation/revisions.txt b/Documentation/revisions.txt
> > > > @@ -299,22 +299,22 @@ empty range that is both reachable and unreachable from HEAD.
> > > > +For example, if you have a linear history like this:
> > > >
> > > > +    ---A---B---C---D---E---F
> > > >
> > > > +Doing A..F will retrieve 5 commits, and doing B..E will retrieve 3
> > > > +commits, but doing A..F B..E will not retrieve two revision ranges
> > > > +totalling 8 commits. Instead the starting point A gets overriden by B,
> > > > +and the ending point of E by F, effectively becoming B..F, a single
> > > > +revision range.
> > >
> > > s/overriden/overridden/
> > >
> > > For what it's worth, as a person who is far from expert at revision
> > > ranges, I had to read this revised text five or six times and think
> > > about it quite a bit to understand what it is saying,
> >
> > Can you explain why?
> 
> I tend to agree with Eric.  I think the example you chose is likely to
> be misinterpreted and your wording magnifies it.  A..F B..E simplifies
> to B..F which is *almost* the union of A..F and B..E, it's only
> missing A.  Off-by-one errors are easy to miss.

Yes, but right before it's explained that the ending point is F.
Not E, F.

> You make it more likely that they'll miss it, because there are only 6
> commits total in the union, and you are trying to explain why listing
> A..F B..E while not be 8 commits, which readers can easily respond
> with, "Well, of course it's not 8 commits.  There's only 6.

If the reader understands that no more than 6 commits can be returned,
then the reader has understood the point that the operation is not
addition.

> When you do the union operation, of course the duplicates go away",
> and miss the actual point that A got excluded.

But that is not the point. This is the point:

  Unless otherwise noted, all git commands that operate on a set of
  commits work on a single revision range.

You are missing the forest for the trees.

In the context of gitrevisions(7) the user has just been told that:

  1. We are trying to specify a graph of commits reachable from a
     commit, or commits.

The user was shown this graph:

  G   H   I   J
   \ /     \ /
    D   E   F
     \  |  / \
      \ | /   |
       \|/    |
        B     C
         \   /
          \ /
           A

And that B is A^, therefore doing `git log A B` is redundant, as is
doing `git log A B D`.

  2. The caret notation `^r1 r2` means commits reachable from r2, but
     exclude commits reachable from r1 (r1 and it's ancestors)

That means '^D A' will exclude D G and H.

  3. The two-dot range notation `r1..r2` is the same as `^r1 r2`


Now, whith this context in mind, we are trying to hedge the corner-case
of `r1..r2 r3..r4` in other words: `^r1 r2 ^r3 r4`.

The user has been told already that C..A is the same as `^C A` (I'm
changing the order to be consistent with the graph above). And to make
my point clear I actually don't need two starting points.

So how about this:

  Commands that are specifically designed to take two distinct ranges
  (e.g. "git range-diff R1 R2" to compare two ranges) do exist, but
  they are exceptions.  Unless otherwise noted, all git commands
  that operate on a set of commits work on a single revision range. Just
  like 'A A' coalesces to 'A', 'B..A C..A' is the same as the
  single revision range '^B ^C A'.

-- 
Felipe Contreras

  reply	other threads:[~2021-06-13 17:10 UTC|newest]

Thread overview: 11+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-06-13  0:44 [PATCH] doc: revisions: improve single range explanation Felipe Contreras
2021-06-13  2:50 ` Bagas Sanjaya
2021-06-13  3:12   ` Felipe Contreras
2021-06-13  3:32 ` Eric Sunshine
2021-06-13  4:25   ` Felipe Contreras
2021-06-13  7:02     ` Elijah Newren
2021-06-13 17:09       ` Felipe Contreras [this message]
2021-06-14 14:39         ` Elijah Newren
2021-06-15 11:53           ` Felipe Contreras
2021-06-13  8:11     ` Eric Sunshine
2021-06-13 16:13       ` Felipe Contreras

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=60c63bc3f616_41f452086b@natae.notmuch \
    --to=felipe.contreras@gmail.com \
    --cc=bagasdotme@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=newren@gmail.com \
    --cc=sunshine@sunshineco.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.