git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
From: Philip Oakley <philipoakley@iee.email>
To: Philippe Blain <levraiphilippeblain@gmail.com>,
	Junio C Hamano <gitster@pobox.com>
Cc: "Philippe Blain via GitGitGadget" <gitgitgadget@gmail.com>,
	git <git@vger.kernel.org>,
	"Martin Ågren" <martin.agren@gmail.com>,
	"Alex Henrie" <alexhenrie24@gmail.com>
Subject: Re: [PATCH] doc: log takes multiple revision ranges
Date: Sun, 5 Jul 2020 21:42:41 +0100	[thread overview]
Message-ID: <f55e0109-af9a-7c21-1cbf-41b2531560e4@iee.email> (raw)
In-Reply-To: <51E0F312-E132-4E8E-AFE6-570CB02B4590@gmail.com>

Hi Philippe,
This had been sitting in my inbox..

On 30/03/2020 13:52, Philippe Blain wrote:
> Hi Junio, 
>> Le 28 mars 2020 à 21:34, Junio C Hamano <gitster@pobox.com> a écrit :
>>
>> "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'd concur that sometime the documentation is not as educationally
informative  as it could be. Often it leaves readers to second guess
what is meant, or redirects to other reference sections with similar
presumptions that the reader already understands.  Documentation is
harder than coding because of these differences in writing style. Often
Git appears to prefer the normative standard style.

>> 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.
> Of course, I understand.
>
>> "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".
> Yes, you are right.
>
>> 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.  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,
> So the 'git log' documentation states: 
>
>     origin..HEAD specifies all the commits reachable from the current commit 
>     (i.e. HEAD), but not from origin. For a complete list of ways to spell <revision range>, 
>     see the Specifying Ranges section of gitrevisions[7].
>
> What I wanted to achieve with this patch is to make readers aware that 
> they can write e.g.
>
>     git log branch1 branch2 branch3 branch4 --not master
>
> to see commits on each of branch1-4, that are not on master, 
> since this is not immediately obvious (at least to me)
> in the 'git log' man page. 

The whole revision walk stuff is tricky. Finding the right starting
point for explanations is key, along with picking out where 'normal'
errors are made, as these do help users who aren't simply 'checking the
manual'. (the 20-20 hindsight problem)

For example, your "that are not on master", and what that means, needs
to be brought out to the new reader rather than being buried
mid-sentence. Most readers will miss those mid-sentence key points and
they can't learn until they have some significant problem that points it
out to them. Sometime we need to point at the 'wrong' things, not just
the right things. (e.g. if branch3 had ben merged a couple of revisions
ago...)

> Even following the link to gitrevisions[7], 
> it is not explicitly mentioned in the "Specifying Ranges" section 
> that a range denotes the set of all commits reachable by all given refs, 
> minus the ones reachable by all given refs prefixed by '^' or '--not'.
> However, this is really clear in the 'git rev-list' documentation, 
> which uses  '<commit>...' in the synopsis
> and clearly talks about the "set of commits" point of view:
>
>     You can think of this as a set operation. Commits given on the command line form 
>     a set of commits that are reachable from any of them, and then commits reachable 
>     from any of the ones given with ^ in front are subtracted from that set. The remaining 
>     commits are what comes out in the command’s output. Various other options and 
>     paths parameters can be used to further limit the result.
>
Set operations don't help everyone, just sayin'.

In some cases: The fact that we exclude any commit reachable from any of
the "^" commits should be mentioned first, and then we select any left
that are reachable from the named tips. This puts a different spin on
the  understanding, and helps catch those that have not understood the
other descriptions. It can then be easily linked to the 'two ranges'
case producing otherwise unexpected results.
> Do you think it would be appropriate that this explanation could somehow also appear 
> in the 'git log' (and maybe also 'gitrevisions') documentation ?
>
> Thanks for the quick feedback,
> Philippe.
Philip

  parent reply	other threads:[~2020-07-05 20:43 UTC|newest]

Thread overview: 23+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-03-28 21:11 [PATCH] doc: log takes multiple revision ranges Philippe Blain via GitGitGadget
2020-03-29  1:34 ` Junio C Hamano
2020-03-30 12:52   ` Philippe Blain
2020-03-30 17:09     ` Junio C Hamano
2020-07-05 20:42     ` Philip Oakley [this message]
2020-07-07 13:08       ` Philippe Blain
2020-03-31 21:53   ` Taylor Blau
2020-07-03  3:38 ` [PATCH v2 0/4] doc: include git rev-list description in git log doc Philippe Blain via GitGitGadget
2020-07-03  3:38   ` [PATCH v2 1/4] git-log.txt: add links to 'rev-list' and 'diff' docs Philippe Blain via GitGitGadget
2020-07-07  0:55     ` Junio C Hamano
2020-07-07 13:11       ` Philippe Blain
2020-07-03  3:38   ` [PATCH v2 2/4] git-rev-list.txt: move description to separate file Philippe Blain via GitGitGadget
2020-07-03  3:38   ` [PATCH v2 3/4] git-log.txt: include rev-list-description.txt Philippe Blain via GitGitGadget
2020-07-03  3:38   ` [PATCH v2 4/4] rev-list-description.txt: fix Asciidoc syntax Philippe Blain via GitGitGadget
2020-07-07  0:59     ` Junio C Hamano
2020-07-07 13:12       ` Philippe Blain
2020-07-09  2:16   ` [PATCH v3 0/6] doc: include git rev-list description in git log doc Philippe Blain via GitGitGadget
2020-07-09  2:16     ` [PATCH v3 1/6] git-log.txt: add links to 'rev-list' and 'diff' docs Philippe Blain via GitGitGadget
2020-07-09  2:16     ` [PATCH v3 2/6] revisions.txt: describe 'rev1 rev2 ...' meaning for ranges Philippe Blain via GitGitGadget
2020-07-09  2:16     ` [PATCH v3 3/6] git-rev-list.txt: fix Asciidoc syntax Philippe Blain via GitGitGadget
2020-07-09  2:16     ` [PATCH v3 4/6] git-rev-list.txt: tweak wording in set operations Philippe Blain via GitGitGadget
2020-07-09  2:16     ` [PATCH v3 5/6] git-rev-list.txt: move description to separate file Philippe Blain via GitGitGadget
2020-07-09  2:16     ` [PATCH v3 6/6] git-log.txt: include rev-list-description.txt Philippe Blain via GitGitGadget

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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to=f55e0109-af9a-7c21-1cbf-41b2531560e4@iee.email \
    --to=philipoakley@iee.email \
    --cc=alexhenrie24@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=gitster@pobox.com \
    --cc=levraiphilippeblain@gmail.com \
    --cc=martin.agren@gmail.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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).