[PATCH v3 0/7] Documentation/shortlog improvements

[PATCH v3 0/7] Documentation/shortlog improvements

[Ramkumar Ramachandra]

Hi,

The re-roll was necessary due to minor/silly mistakes in v2, as
pointed out by Junio.

The builtin/shortlog.c patch has moved to the end until we change
'<since>..<until>' to '<revision range>' everywhere else.  We use
'<revision range>' here too.

[4/7] has been replaced with the version Junio fixed up and put in
`pu`.

[6/7] has been updated to copy sections from log.txt properly, and
omit the "see below for History Simplification" part.

Thanks.

Ramkumar Ramachandra (7):
  git-shortlog.txt: remove (-h|--help) from OPTIONS
  revisions.txt: clarify the .. and ... syntax
  git-log.txt: order OPTIONS properly; move <since>..<until>
  git-log.txt: generalize <since>..<until>
  git-log.txt: rewrite note on why "--" may be required
  git-shortlog.txt: make SYNOPSIS match log, update OPTIONS
  builtin/shortlog.c: make usage string consistent with log

 Documentation/git-log.txt      | 22 ++++++++++++----------
 Documentation/git-shortlog.txt | 23 +++++++++++++++++------
 Documentation/revisions.txt    |  6 ++++--
 builtin/shortlog.c             |  4 +---
 4 files changed, 34 insertions(+), 21 deletions(-)

-- 
1.8.2.1.501.gd2949c7

[PATCH 1/7] git-shortlog.txt: remove (-h|--help) from OPTIONS

[Ramkumar Ramachandra]

To be consistent with the documentation of all the other commands,
remove (-h|--help) from the OPTIONS section.

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 Documentation/git-shortlog.txt | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/Documentation/git-shortlog.txt b/Documentation/git-shortlog.txt
index c308e91..c7f7f51 100644
--- a/Documentation/git-shortlog.txt
+++ b/Documentation/git-shortlog.txt
@@ -26,10 +26,6 @@ reference to the current repository.
 OPTIONS
 -------
 
--h::
---help::
-	Print a short usage message and exit.
-
 -n::
 --numbered::
 	Sort output according to the number of commits per author instead
-- 
1.8.2.1.501.gd2949c7

[PATCH 2/7] revisions.txt: clarify the .. and ... syntax

[Ramkumar Ramachandra]

In <rev1>..<rev2> and <rev1>...<rev2>, if either <rev1> or <rev2> is
omitted, it defaults to 'HEAD'.  Add this detail to the document.

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 Documentation/revisions.txt | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/Documentation/revisions.txt b/Documentation/revisions.txt
index 8855b1a..0a23129 100644
--- a/Documentation/revisions.txt
+++ b/Documentation/revisions.txt
@@ -244,11 +244,13 @@ To summarize:
 
 '<rev1>..<rev2>'::
 	Include commits that are reachable from <rev2> but exclude
-	those that are reachable from <rev1>.
+	those that are reachable from <rev1>.  When either <rev1> or
+	<rev2> is omitted, it defaults to 'HEAD'.
 
 '<rev1>\...<rev2>'::
 	Include commits that are reachable from either <rev1> or
-	<rev2> but exclude those that are reachable from both.
+	<rev2> but exclude those that are reachable from both.  When
+	either <rev1> or <rev2> is omitted, it defaults to 'HEAD'.
 
 '<rev>{caret}@', e.g. 'HEAD{caret}@'::
   A suffix '{caret}' followed by an at sign is the same as listing
-- 
1.8.2.1.501.gd2949c7

[PATCH 3/7] git-log.txt: order OPTIONS properly; move <since>..<until>

[Ramkumar Ramachandra]

The OPTIONS section lists <since>..<until> as the first item, but this
is inconsistent with the ordering in SYNOPSIS.  Move it down until it
appears just before [[--] <path>...].

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 Documentation/git-log.txt | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt
index 69db578..64cc337 100644
--- a/Documentation/git-log.txt
+++ b/Documentation/git-log.txt
@@ -24,13 +24,6 @@ each commit introduces are shown.
 OPTIONS
 -------
 
-<since>..<until>::
-	Show only commits between the named two commits.  When
-	either <since> or <until> is omitted, it defaults to
-	`HEAD`, i.e. the tip of the current branch.
-	For a more complete list of ways to spell <since>
-	and <until>, see linkgit:gitrevisions[7].
-
 --follow::
 	Continue listing the history of a file beyond renames
 	(works only for a single file).
@@ -69,6 +62,13 @@ produced by --stat etc.
 	Note that only message is considered, if also a diff is shown
 	its size is not included.
 
+<since>..<until>::
+	Show only commits between the named two commits.  When
+	either <since> or <until> is omitted, it defaults to
+	`HEAD`, i.e. the tip of the current branch.
+	For a more complete list of ways to spell <since>
+	and <until>, see linkgit:gitrevisions[7].
+
 [\--] <path>...::
 	Show only commits that are enough to explain how the files
 	that match the specified paths came to be.  See "History
-- 
1.8.2.1.501.gd2949c7

[PATCH 4/7] git-log.txt: generalize <since>..<until>

[Ramkumar Ramachandra]

'<since>..<until>' is misleading, as there are many other forms that
'git log' can accept as an argument.  Replace it with <revision range>,
referring to the section "Specifying Ranges" in revisions.txt, and
rewrite the section appropriately.

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 Documentation/git-log.txt | 16 +++++++++-------
 1 file changed, 9 insertions(+), 7 deletions(-)

diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt
index 64cc337..9576695 100644
--- a/Documentation/git-log.txt
+++ b/Documentation/git-log.txt
@@ -9,7 +9,7 @@ git-log - Show commit logs
 SYNOPSIS
 --------
 [verse]
-'git log' [<options>] [<since>..<until>] [[\--] <path>...]
+'git log' [<options>] [<revision range>] [[\--] <path>...]
 
 DESCRIPTION
 -----------
@@ -62,12 +62,14 @@ produced by --stat etc.
 	Note that only message is considered, if also a diff is shown
 	its size is not included.
 
-<since>..<until>::
-	Show only commits between the named two commits.  When
-	either <since> or <until> is omitted, it defaults to
-	`HEAD`, i.e. the tip of the current branch.
-	For a more complete list of ways to spell <since>
-	and <until>, see linkgit:gitrevisions[7].
+<revision range>::
+	Show only commits in the specified revision range.  When no
+	<revision range> is specified, it defaults to `HEAD` (i.e. the
+	whole history leading to the current commit).  `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 linkgit:gitrevisions[7].
 
 [\--] <path>...::
 	Show only commits that are enough to explain how the files
-- 
1.8.2.1.501.gd2949c7

[PATCH 5/7] git-log.txt: rewrite note on why "--" may be required

[Ramkumar Ramachandra]

In its current form, the note talks about separating options from
"branch names" and "refnames" in the same sentence.  This is entirely
inaccurate, as <revision range> need not be a set of branch names or
ref names.  Rewrite it to use the word "revision range", to be
consistent with the SYNOPSIS.

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 Documentation/git-log.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt
index 9576695..a976534 100644
--- a/Documentation/git-log.txt
+++ b/Documentation/git-log.txt
@@ -77,8 +77,8 @@ produced by --stat etc.
 	Simplification" below for details and other simplification
 	modes.
 +
-To prevent confusion with options and branch names, paths may need to
-be prefixed with "\-- " to separate them from options or refnames.
+Paths may need to be prefixed with "\-- " to separate them from
+options or the revision range, when confusion arises.
 
 include::rev-list-options.txt[]
 
-- 
1.8.2.1.501.gd2949c7

Re* [PATCH 5/7] git-log.txt: rewrite note on why "--" may be required

[Junio C Hamano]

Ramkumar Ramachandra <artagnon@gmail.com> writes:

> In its current form, the note talks about separating options from
> "branch names" and "refnames" in the same sentence.  This is entirely
> inaccurate, as <revision range> need not be a set of branch names or
> ref names.  Rewrite it to use the word "revision range", to be
> consistent with the SYNOPSIS.
>
> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
> ---
>  Documentation/git-log.txt | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt
> index 9576695..a976534 100644
> --- a/Documentation/git-log.txt
> +++ b/Documentation/git-log.txt
> @@ -77,8 +77,8 @@ produced by --stat etc.
>  	Simplification" below for details and other simplification
>  	modes.
>  +
> -To prevent confusion with options and branch names, paths may need to
> -be prefixed with "\-- " to separate them from options or refnames.
> +Paths may need to be prefixed with "\-- " to separate them from
> +options or the revision range, when confusion arises.
>  
>  include::rev-list-options.txt[]

This is an explanation about an enumeration item:

	[\--] <path>...::

which corresponds to this in the synopsis:

        'git log' [<options>] [<revision range>] [[\--] <path>...]

But the synopsis looks incorrect to me.  It insists that if you were
to write "--", you have to give at least one <path>.

It should be more like this [*1*]:

	'git log' [<options>] [<revision range>] [--] [<path>...]

It may be better to split the item into two, keep the current
description of <path> under "<path>...::" heading, and have another
heading for "--::" that talks about the disambiguation.  The "--"
disambiguator is not solely for "your path might be mistaken as a
revision without it", but also for "your revision might be mistaken
as a path", but both the old and the new text are fuzzy about the
reason what "confusion" it is cautioning the reader.

Formatting doubts aside, one thing I am not so happy about the
suggested patch below (would apply on top of the patches from this
series) is that the explanation of a double-dash requires
understanding of _both_ what <revision range> and <path>... means to
the command, but for a reader who has read this page from top, the
latter has not been explained yet.  We could of course swap the
order and describe <path>... first and then "--", but that does not
sound quite right, either.  It has to be between these two.


[Footnote]

*1* We have many instances like "[--quiet]" in Documentation; do we
    need the backslash before these double-dash like "[\--]"?


 Documentation/git-log.txt | 22 +++++++++++++++++-----
 1 file changed, 17 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt
index a976534..f5746e4 100644
--- a/Documentation/git-log.txt
+++ b/Documentation/git-log.txt
@@ -9,7 +9,7 @@ git-log - Show commit logs
 SYNOPSIS
 --------
 [verse]
-'git log' [<options>] [<revision range>] [[\--] <path>...]
+'git log' [<options>] [<revision range>] [--] [<path>...]
 
 DESCRIPTION
 -----------
@@ -71,14 +71,26 @@ produced by --stat etc.
 	ways to spell <revision range>, see the "Specifying Ranges"
 	section of linkgit:gitrevisions[7].
 
-[\--] <path>...::
+--::
+	On the command line, arguments that specify revision range
+	come first and then paths (see the next item).  A
+	double-dash can be used to mark the end of revision range
+	arguments when an argument can be otherwise ambiguous.
++
+For example, when there are a file called `master` and a branch with
+the same name, what `git log master` means is unclear.  It may be
+asking for the whole history leading to the current commit (i.e. no
+<revision range> is specified, defaulting to `HEAD`), limited to
+commits that touch a single path `master`.  Or it may be asking for
+the whole history leading to the tip of `master`, without any path
+limiting.  The former can be spelled `git log -- master`, while the
+latter can be spelled `git log master --`.
+
+<path>...::
 	Show only commits that are enough to explain how the files
 	that match the specified paths came to be.  See "History
 	Simplification" below for details and other simplification
 	modes.
-+
-Paths may need to be prefixed with "\-- " to separate them from
-options or the revision range, when confusion arises.
 
 include::rev-list-options.txt[]
 

Re: Re* [PATCH 5/7] git-log.txt: rewrite note on why "--" may be required

[Ramkumar Ramachandra]

Junio C Hamano wrote:
> It should be more like this [*1*]:
>
>         'git log' [<options>] [<revision range>] [--] [<path>...]

Agreed.  The backslash is unnecessary (I suspect it's something
carried over from earlier versions of asciidoc requiring this
escaping).

> It may be better to split the item into two, keep the current
> description of <path> under "<path>...::" heading, and have another
> heading for "--::" that talks about the disambiguation.  The "--"
> disambiguator is not solely for "your path might be mistaken as a
> revision without it", but also for "your revision might be mistaken
> as a path", but both the old and the new text are fuzzy about the
> reason what "confusion" it is cautioning the reader.

I agree that the "confusion" paragraph after [--] [<path>] can be
improved, but putting [--] in a paragraph of its own sounds like an
overkill.  Apart from other things, it means that every single git
command would need an identical [--] paragraph for consistency.  We
can just keep the footnote following [--] [<path>] in some commands,
using discretion.

Re: Re* [PATCH 5/7] git-log.txt: rewrite note on why "--" may be required

[Junio C Hamano]

Ramkumar Ramachandra <artagnon@gmail.com> writes:

> I agree that the "confusion" paragraph after [--] [<path>] can be
> improved, but putting [--] in a paragraph of its own sounds like an
> overkill.  Apart from other things, it means that every single git
> command would need an identical [--] paragraph for consistency.

That is not a problem.  We need to say what command line argument X
(e.g. <path>, but not limited to it) does in all pages for commands
that pay attention to X anyway.

A more important reason to have them as separate entries is to avoid
giving a wrong impression that "--" is somehow related to <path>,
which is where the mistake in the synopsis "[[--] <path>...]" that
started this thread comes from.

Re: Re* [PATCH 5/7] git-log.txt: rewrite note on why "--" may be required

[Ramkumar Ramachandra]

Junio C Hamano wrote:
> That is not a problem.
> [...]
> A more important reason to have them as separate entries is to avoid
> giving a wrong impression that "--" is somehow related to <path>,

Okay, makes sense then.  Queue it for this and shortlog; we can write
it for the other manpages gradually.

[PATCH 6/7] git-shortlog.txt: make SYNOPSIS match log, update OPTIONS

[Ramkumar Ramachandra]

There are broadly two problems with the current SYNOPSIS.  First, it
completely omits the detail that paths can be specified.  Second, it
attempts to list all the options: this is futile as, in addition to
the options unique to it, it accepts all the options that git-rev-list
accepts.  In fixing these problems, make the SYNOPSIS consistent with
that in git-log.txt.  Also add the corresponding sections to OPTIONS.
Save adding the options from rev-list-options.txt for a later patch,
as it requires some work to pick out the options that are relevant to
shortlog.

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 Documentation/git-shortlog.txt | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-shortlog.txt b/Documentation/git-shortlog.txt
index c7f7f51..31af7f2 100644
--- a/Documentation/git-shortlog.txt
+++ b/Documentation/git-shortlog.txt
@@ -8,8 +8,8 @@ git-shortlog - Summarize 'git log' output
 SYNOPSIS
 --------
 [verse]
-git log --pretty=short | 'git shortlog' [-h] [-n] [-s] [-e] [-w]
-'git shortlog' [-n|--numbered] [-s|--summary] [-e|--email] [-w[<width>[,<indent1>[,<indent2>]]]] <commit>...
+git log --pretty=short | 'git shortlog' [<options>]
+'git shortlog' [<options>] [<revision range>] [[\--] <path>...]
 
 DESCRIPTION
 -----------
@@ -56,6 +56,21 @@ OPTIONS
 If width is `0` (zero) then indent the lines of the output without wrapping
 them.
 
+<revision range>::
+	Show only commits in the specified revision range.  When no
+	<revision range> is specified, it defaults to `HEAD` (i.e. the
+	whole history leading to the current commit).  `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 linkgit:gitrevisions[7].
+
+[\--] <path>...::
+	Consider only commits that are enough to explain how the files
+	that match the specified paths came to be.
++
+Paths may need to be prefixed with "\-- " to separate them from
+options or the revision range, when confusion arises.
 
 MAPPING AUTHORS
 ---------------
-- 
1.8.2.1.501.gd2949c7

[PATCH 7/7] builtin/shortlog.c: make usage string consistent with log

[Ramkumar Ramachandra]

"--" is used to separate pathspecs from the rev specs, and not rev
specs from the options, as the shortlog_usage string currently
indicates.  In correcting this usage string, make it consistent with
the log_usage string.

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 builtin/shortlog.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/builtin/shortlog.c b/builtin/shortlog.c
index 240bff3..1fd6f8a 100644
--- a/builtin/shortlog.c
+++ b/builtin/shortlog.c
@@ -10,9 +10,7 @@
 #include "parse-options.h"
 
 static char const * const shortlog_usage[] = {
-	N_("git shortlog [-n] [-s] [-e] [-w] [rev-opts] [--] [<commit-id>... ]"),
-	"",
-	N_("[rev-opts] are documented in git-rev-list(1)"),
+	N_("git shortlog [<options>] [<revision range>] [[--] [<path>...]]"),
 	NULL
 };
 
-- 
1.8.2.1.501.gd2949c7

[PATCH 6.5/7] builtin/log.c: make usage string consistent with doc

[Ramkumar Ramachandra]

Replace '<since>..<until>' with '<revision range>', in accordance with
the documentation.

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 Junio: sorry I missed this detail.  Can you squeeze this patch
 between [6/7] and [7/7] so that the commit message in [7/7] makes
 sense?

 Thanks.

 builtin/log.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/log.c b/builtin/log.c
index ad46f72..6e56a50 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -37,7 +37,7 @@ static const char *fmt_patch_subject_prefix = "PATCH";
 static const char *fmt_pretty;
 
 static const char * const builtin_log_usage[] = {
-	N_("git log [<options>] [<since>..<until>] [[--] <path>...]\n")
+	N_("git log [<options>] [<revision range>] [[--] <path>...]\n")
 	N_("   or: git show [options] <object>..."),
 	NULL
 };
-- 
1.8.2.1.423.g9a53c75.dirty