All of lore.kernel.org
 help / color / mirror / Atom feed
From: Elijah Newren <newren@gmail.com>
To: 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 00:02:18 -0700	[thread overview]
Message-ID: <CABPp-BE4r=Nhw2sJS++7Eh1K5rpyWgg+f8vDTf92JBayt1B_cA@mail.gmail.com> (raw)
In-Reply-To: <60c588d452750_3d86c2085c@natae.notmuch>

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.  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.  When you do the union
operation, of course the duplicates go away", and miss the actual
point that A got excluded.

Junio's wording and example just seemed better to me here.

>
> This is the context: commands don't generally take two ranges:
>
>  1. Unless otherwise noted, all git commands that operate on a set of
>     commits work on a single revision range.
>
>  2. 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.
>
> At this point what isn't clear? Isn't it clear that `A..F B..E` aren't
> two revision ranges?
>
>  3. Instead the starting point A gets overridden by B, and the ending
>     point of E by F, effectively becoming B..F, a single revision range.
>
> What isn't clear about that? A gets superseded by B because it's higher
> in the graph. And if you do `git log D E F` it's clear that doing
> `git log F` will get you the same thing, isn't it?
>
> > Also, if this explanation is aimed at newcomers, then saying only
> > "doing A..F will retrieve 5 commits" without actually saying _which_
> > commits those are is perhaps not so helpful.
>
> It doesn't matter which specific commits are retrieved, the only thing
> that matters is that `X op Y` is not additive.
>
> --
> Felipe Contreras

  reply	other threads:[~2021-06-13  7:03 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 [this message]
2021-06-13 17:09       ` Felipe Contreras
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='CABPp-BE4r=Nhw2sJS++7Eh1K5rpyWgg+f8vDTf92JBayt1B_cA@mail.gmail.com' \
    --to=newren@gmail.com \
    --cc=bagasdotme@gmail.com \
    --cc=felipe.contreras@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.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.