[PATCH 0/5] Documentation/shortlog improvements

[PATCH 0/5] Documentation/shortlog improvements

[Ramkumar Ramachandra]

Hi,

I was going through the shortlog documentation and was saddened to see
that it was inaccurate and inconsistent with the log documentation.  I
use shortlog quite a lot, and like it very much.  So, here's a small
series fixing some problems.

[3/5] and [4/5] came out of my desire to copy out parts of git-log.txt
into git-shortlog.txt (I had to fix git-log.txt first).

Thanks.

Ramkumar Ramachandra (5):
  git-shortlog.txt: remove (-h|--help) from OPTIONS
  builtin/shortlog.c: make usage string consistent with log
  git-log.txt: fix description of <since>..<until>
  git-log.txt: rewrite note on why "--" may be required
  git-shortlog.txt: make SYNOPSIS match log, update OPTIONS

 Documentation/git-log.txt      | 10 +++++-----
 Documentation/git-shortlog.txt | 23 ++++++++++++++++++-----
 builtin/shortlog.c             |  4 +---
 3 files changed, 24 insertions(+), 13 deletions(-)

-- 
1.8.2.1.506.gbce9ff0

[PATCH 1/5] 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.506.gbce9ff0

[PATCH 2/5] 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..a2f54f2 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>] [<since>..<until>] [[--] [<path>...]]"),
 	NULL
 };
 
-- 
1.8.2.1.506.gbce9ff0

[PATCH 3/5] git-log.txt: fix description of <since>..<until>

[Ramkumar Ramachandra]

First, <since> and <until> are ways to specify "revisions", not
"commits", as gitrevisions.txt would indicate.  Second,
'<since>..<until>' is simply indicative of how users would normally
want to specify the rev spec: it need not conform to this form, and
can take any form that gitrevisions.txt lists.  A 'git log :/quuxery'
is perfectly valid, for instance.  Make these two corrections to the
description of '<since>..<until>' under OPTIONS.

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

diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt
index 69db578..f03ae74 100644
--- a/Documentation/git-log.txt
+++ b/Documentation/git-log.txt
@@ -25,11 +25,11 @@ OPTIONS
 -------
 
 <since>..<until>::
-	Show only commits between the named two commits.  When
+	Show only commits between the named two revisions.  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].
+	For a more complete list of ways to spell <since>..<until>,
+	see linkgit:gitrevisions[7].
 
 --follow::
 	Continue listing the history of a file beyond renames
-- 
1.8.2.1.506.gbce9ff0

[PATCH 4/5] 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 the rev spec need not be a set of branch names or ref
names.  Rewrite it to use the word "revisions".

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 f03ae74..1a9c3ca 100644
--- a/Documentation/git-log.txt
+++ b/Documentation/git-log.txt
@@ -75,8 +75,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 revisions, when confusion arises.
 
 include::rev-list-options.txt[]
 
-- 
1.8.2.1.506.gbce9ff0

[PATCH 5/5] 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.

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

diff --git a/Documentation/git-shortlog.txt b/Documentation/git-shortlog.txt
index c7f7f51..d02ac03 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>] [<since>..<until>] [[\--] <path>...]
 
 DESCRIPTION
 -----------
@@ -26,6 +26,13 @@ reference to the current repository.
 OPTIONS
 -------
 
+<since>..<until>::
+	Consider only commits between the named two revisions.  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>..<until>, see
+	linkgit:gitrevisions[7].
+
 -n::
 --numbered::
 	Sort output according to the number of commits per author instead
@@ -56,6 +63,16 @@ OPTIONS
 If width is `0` (zero) then indent the lines of the output without wrapping
 them.
 
+[\--] <path>...::
+	Consider 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 revisions, when confusion arises.
+
+include::rev-list-options.txt[]
 
 MAPPING AUTHORS
 ---------------
-- 
1.8.2.1.506.gbce9ff0

Re: [PATCH 3/5] git-log.txt: fix description of <since>..<until>

[Jonathan Nieder]

Ramkumar Ramachandra wrote:

> First, <since> and <until> are ways to specify "revisions", not
> "commits", as gitrevisions.txt would indicate.

What's the difference between a revision and a commit?  The definition
in gitglossary(7) only confuses me.

Re: [PATCH 3/5] git-log.txt: fix description of <since>..<until>

[Junio C Hamano]

Ramkumar Ramachandra <artagnon@gmail.com> writes:

> First, <since> and <until> are ways to specify "revisions", not
> "commits", as gitrevisions.txt would indicate.  Second,
> '<since>..<until>' is simply indicative of how users would normally
> want to specify the rev spec: it need not conform to this form, and
> can take any form that gitrevisions.txt lists.  A 'git log :/quuxery'
> is perfectly valid, for instance.  Make these two corrections to the
> description of '<since>..<until>' under OPTIONS.
>
> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
> ---
>  Documentation/git-log.txt | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt
> index 69db578..f03ae74 100644
> --- a/Documentation/git-log.txt
> +++ b/Documentation/git-log.txt
> @@ -25,11 +25,11 @@ OPTIONS
>  -------
>  
>  <since>..<until>::
> -	Show only commits between the named two commits.  When
> +	Show only commits between the named two revisions.  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].
> +	For a more complete list of ways to spell <since>..<until>,
> +	see linkgit:gitrevisions[7].

A bigger issue is if it is a good idea to single out A..B like this
in the enumeration of options and parameters.  We are listing this
among the optoins and pathspec, so this is where we _ought_ to be
saying that you can specify a revision range on the command line.
We use A..B only because it is the most commonly used form, asking
e.g. "show me the commit since v1.7.12 up to v1.8.0", but this
existing headline makes it as if it is the _only_ form, which is
somewhat misleading.

It might be better _not_ to say <since>..<until> here and on the
synopsis part, replace it with <revision range>, and update the text
to mention <since>..<until> as a more concrete "v1.0..v2.0" example.
Perhaps like

    <revision range>::

	Show only commits in the specified revision range.  When no
	<revision range> is specified, defaults to "HEAD" (i.e. the
	whole history leading to the current commit).  v1.0..v2.0
	asks for commits since the commit tagged as version 1.0 up
	to the commit tagged as version 2.0.  For a more complete
	list of ways to spell this parameter, see "Specifying
	Ranges" section of linkgit:gitrevisions[7].

or something.

Documentation/gitrevisions.txt lists <rev1>..<rev2>, but does not
mention that missing <rev1> (or <rev2>) defaults to HEAD, which is
worth adding there with or without the above suggested update.

Re: [PATCH 4/5] 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 the rev spec need not be a set of branch names or ref
> names.  Rewrite it to use the word "revisions".
>
> 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 f03ae74..1a9c3ca 100644
> --- a/Documentation/git-log.txt
> +++ b/Documentation/git-log.txt
> @@ -75,8 +75,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 revisions, when confusion arises.

I think "branch names" was an attempt to make it more newbie
friendly by sacrificing technical accuracy.  With the suggested
update (see the review for the previous one), it would be easier to
read if this part said "options or the revision range".

Two other things I noticed with the current text are:

 * As the synopsis section shows, on the command line, --options
   come first, then revision range and then pathspec.  The order of
   the description should follow that as well.  The current text
   shows <since>..<until> at the very beginning, which is wrong.

 * The part with the new "-L" option seems to be throwing the
   overall formatting off.  Its second paragraph "<start> and <end>
   can take..." is not indented to the same level as its first
   paragraph "Trace the evolution of...", and the following items,
   like "--full-line-diff" and "[--] <pathspec>..." are indented one
   level too deeply.

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

[Junio C Hamano]

Ramkumar Ramachandra <artagnon@gmail.com> writes:

> +<since>..<until>::
> +	Consider only commits between the named two revisions.  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>..<until>, see
> +	linkgit:gitrevisions[7].
> +

The same "revision range" comment, and also that this should be
after all options and before "-- <pathspec>", apply to this.

> @@ -56,6 +63,16 @@ OPTIONS
>  If width is `0` (zero) then indent the lines of the output without wrapping
>  them.
>  
> +[\--] <path>...::
> +	Consider 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 revisions, when confusion arises.
> +
> +include::rev-list-options.txt[]

While it is true that this page is missing many options, including
rev-list-options.txt as-is is more harmful than helpful and the
patch needs more work (hint: with "ifdef::git-shortlog[]" sprinkled
in rev-list-options.txt).

Some of the earlier one in rev-list-options are fine, but some
others are inappropriate in the context of shortlog.  For example,
--grep=<pattern>, while it would internally work as it is told to
do, is unlikely to be clear how well/correctly worked when viewing
the end result.  So are many others, such as --all-match,
--walk-reflogs, --sparse, etc.  Some e.g. --topo-order, do not make
sense at all.

I am not sure what the command would do if you give --stdin; the
command wants to read from the standard input to operate as a filter
for "git log" output, but the "--stdin" from rev-list family wants
to read revision range specifiers from the standard input.  I do not
think it is something any sane user would want to do.

Re: [PATCH 3/5] git-log.txt: fix description of <since>..<until>

[Ramkumar Ramachandra]

Jonathan Nieder wrote:
> What's the difference between a revision and a commit?  The definition
> in gitglossary(7) only confuses me.

What's so confusing about it?

commit:: As a noun: A single point in the Git history; the entire
history of a project is represented as a set of interrelated commits.
The word "commit" is often used by Git in the same places other
revision control systems use the words "revision" or "version". Also
used as a short hand for commit object.

revision:: A particular state of files and directories which was
stored in the object database. It is referenced by a commit object.

-- 8< --

There a reference to "revision" in the "commit" description which
might be misleading.

Otherwise, these are quite accurate.  master~3:README is a valid revision,
but not a commit, for instance.  Another problem with the word commit
is that it doesn't encapsulate ranges like 'master~3..HEAD'.  It would
probably help to add the terms "commitish" and "treeish" to the
glossary to refer to "rev specs" (those in revisions.txt) that resolve
to commits or trees.  I'll give you two simple examples:

1. In 'git show master:README', master is a treeish and master:README
is a blob.  If we were to say 'git blame master: -- README', it would
error out because master: is not a commitish, but a treeish.

2. In 'git log master~3.. -- README', master~3.. is a commitish.  In
general, log can take only rev specs that resolve to a commits.
Although 'git log master:' doesn't error out, it doesn't make any
sense either.  Perhaps we should tighten it?

Re: [PATCH 3/5] git-log.txt: fix description of <since>..<until>

[Junio C Hamano]

Jonathan Nieder <jrnieder@gmail.com> writes:

> Ramkumar Ramachandra wrote:
>
>> First, <since> and <until> are ways to specify "revisions", not
>> "commits", as gitrevisions.txt would indicate.
>
> What's the difference between a revision and a commit?  The definition
> in gitglossary(7) only confuses me.

They are the same, but I think it is irrelevant.  We do not want
revision or commit there, anyway.  We want a revision range there.
This is "git log" not "git show".

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

[Ramkumar Ramachandra]

Junio C Hamano wrote:
> I think "branch names" was an attempt to make it more newbie
> friendly by sacrificing technical accuracy.  With the suggested
> update (see the review for the previous one), it would be easier to
> read if this part said "options or the revision range".

Why does it have to be a range?  It might well be a list of revisions,
so I'm tempted to stick with the word "revisions".  As I wrote in the
previous email, I think "revision" can be referenced by any one of the
syntaxes specified in revisions.txt (this includes a "committish
range").

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

[Junio C Hamano]

Ramkumar Ramachandra <artagnon@gmail.com> writes:

> Junio C Hamano wrote:
>> I think "branch names" was an attempt to make it more newbie
>> friendly by sacrificing technical accuracy.  With the suggested
>> update (see the review for the previous one), it would be easier to
>> read if this part said "options or the revision range".
>
> Why does it have to be a range? It might well be a list of revisions,

Study "Specifying Ranges" section of gitrevisions and come back,
perhaps?

A list of revisions is merely a way to specify "revision range" that
are reachable from any of these revisions listed.  "log A B" won't
stop by just showing A and B (that would be "show A B"), but will
list those that are reachable from A B, so in the context of
discussing the arguments given to "git log" command, "A B" is still
"revision range".

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

[Ramkumar Ramachandra]

Junio C Hamano wrote:
> A list of revisions is merely a way to specify "revision range" that
> are reachable from any of these revisions listed.  "log A B" won't
> stop by just showing A and B (that would be "show A B"), but will
> list those that are reachable from A B, so in the context of
> discussing the arguments given to "git log" command, "A B" is still
> "revision range".

... and what about 'git log HEAD^!'?  Is that a range?  What about
'git log HEAD:README' (hint: it doesn't error out)? I would argue that
"A B" is not inherently a revision range, but rather two revisions.
It's upto different commands to interpret it differently.

Then again, in the context of log, we just want ranges (also
considering we had <since>..<until> for this long, and I didn't even
object to it).  So I'll go with your "revision range", but I won't
claim that it's technically accurate.

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

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Ramkumar Ramachandra <artagnon@gmail.com> writes:
>
>> Junio C Hamano wrote:
>>> I think "branch names" was an attempt to make it more newbie
>>> friendly by sacrificing technical accuracy.  With the suggested
>>> update (see the review for the previous one), it would be easier to
>>> read if this part said "options or the revision range".
>>
>> Why does it have to be a range? It might well be a list of revisions,
>
> Study "Specifying Ranges" section of gitrevisions and come back,
> perhaps?

A bit more specifically, there is a reason why we list "Specifying
Revisions" and "Specifying Ranges" separately in that manual page.

I think you are trying explain "git log --short A B ^C" as if it
takes "--short" (which is an option), "A", "B", and "^C" (all of
which are revisions).  And I am saying that is wrong.

It is "--short" (which is an option) and "A B ^C" (which is a
revision range).

"git log --short A" is taking "--short" (an option) and "A".  This
"A" is still a revision range and not a single revision.  It is a
single commit that is used to name a revision range which is the
entire history behind the commit "A".

Re: [PATCH 3/5] git-log.txt: fix description of <since>..<until>

[Jonathan Nieder]

Ramkumar Ramachandra wrote:

> What's so confusing about it?
[...]
> revision:: A particular state of files and directories which was
> stored in the object database. It is referenced by a commit object.

So a revision is a tree?  ("one of the things pointed to by a commit
object, representing a particular state of files and directories")

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

[Junio C Hamano]

Ramkumar Ramachandra <artagnon@gmail.com> writes:

> ... and what about 'git log HEAD^!'?  Is that a range?  

Of course it _is_ a range; didn't you read it in the "specifying
ranges" section?  It is a short-hand for "include HEAD, exclude its
parents", and you can further combine it with other starting points.

> What about 'git log HEAD:README' (hint: it doesn't error out)?

Sounds like a bug, if it doesn't.

Patches welcome---I suspect that it can be solved the same way as
the recent cherry-pick patch by Miklos.

> I would argue that "A B" is not inherently a revision range, but
> rather two revisions.  It's upto different commands to interpret
> it differently.

That is why I said this is "git log", not "git show".

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

[Ramkumar Ramachandra]

Junio C Hamano wrote:
> I think you are trying explain "git log --short A B ^C" as if it
> takes "--short" (which is an option), "A", "B", and "^C" (all of
> which are revisions).  And I am saying that is wrong.
>
> It is "--short" (which is an option) and "A B ^C" (which is a
> revision range).

Got it.  Your explanation makes more sense.

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

[Ramkumar Ramachandra]

Junio C Hamano wrote:
> Sounds like a bug, if it doesn't.
>
> Patches welcome---I suspect that it can be solved the same way as
> the recent cherry-pick patch by Miklos.

It's not as pressing as the glaring Documentation inaccuracies, so
I'll queue the task.

To exclude this case, I would say the most technically accurate
description of what 'git log' takes is a "committish range" (basically
a "revision range" that resolves to commits).

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

[Jonathan Nieder]

Ramkumar Ramachandra wrote:

>                       I would say the most technically accurate
> description of what 'git log' takes is a "committish range" (basically
> a "revision range" that resolves to commits).

What is a revision range that doesn't resolve to commits?  Am I wrong
in thinking revision is nothing more than a synonym for commit?

When gitrevisions(7) says "A revision parameter <rev> typically, but
not necessarily, names a commit object", I suspect it is residue from
3a45f625 trying to apologize for the extended SHA1 syntax parser being
called "git rev-parse" instead of "git object-name-parse".

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

[Ramkumar Ramachandra]

Junio C Hamano wrote:
> A revision range is a slice of history, so you don't need any further
> adjective.
>
> Please don't introduce new words that do not add any accuracies nor value.

I was just saying: I don't want to introduce new terms either.  I
think revisions.txt needs to be fixed.  Under the Specifying Ranges
section:

'<rev>'::
	Include commits that are reachable from (i.e. ancestors of)
	<rev>.

This doesn't make sense because <rev> might as well refer to a blob*.
So this should say s/<rev>/<rev> that names a commit object/.

* The first line in the Specifying Revisions section: A revision
parameter '<rev>' typically, but not necessarily, names a commit
object.

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

[Ramkumar Ramachandra]

Jonathan Nieder wrote:
> Am I wrong
> in thinking revision is nothing more than a synonym for commit?

Yes.  If master@{3}~2:README isn't a revision, what is it?  And it
fits into the Specifying Revisions section quite snugly: I see no
reason to mangle the meaning of "revision".

[PATCH/RFC] glossary: a revision is just a commit

[Jonathan Nieder]

The current definition of 'revision' sounds like it is saying that a
revision is a tree object.  In reality it is just a commit.

This should be especially useful for people used to other revision
control systems trying to see how familiar concepts translate into git
terms.

Reported-by: Ramkumar Ramachandra <artagnon@gmail.com>
Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
---
Junio C Hamano wrote:
> On Apr 21, 2013 12:41 AM, "Jonathan Nieder" <jrnieder@gmail.com> wrote:

>>> revision:: A particular state of files and directories which was
>>> stored in the object database. It is referenced by a commit object.
>>
>> So a revision is a tree?
[...]
> That's a historical misnomer. Some places in the documentation use revision
> as if the word were interchangeable with object name, and HEAD:path listed
> in specifying revision section is a prime example of it. What the section
> lists is historically called extended SHA-1 expression, essentially what
> get_sha1() can grok down to a single object name (as opposed to a slice of
> history, which is a revision range).

Thanks.

 Documentation/glossary-content.txt | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/Documentation/glossary-content.txt b/Documentation/glossary-content.txt
index eb7ba84f..521fceeb 100644
--- a/Documentation/glossary-content.txt
+++ b/Documentation/glossary-content.txt
@@ -430,9 +430,7 @@ should not be combined with other pathspec.
 	<<def_merge,merge>> left behind.
 
 [[def_revision]]revision::
-	A particular state of files and directories which was stored in the
-	<<def_object_database,object database>>. It is referenced by a
-	<<def_commit_object,commit object>>.
+	Synonym for <<def_commit,commit>> (the noun).
 
 [[def_rewind]]rewind::
 	To throw away part of the development, i.e. to assign the
-- 
1.8.2.1

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

[Jonathan Nieder]

Ramkumar Ramachandra wrote:

> Yes.  If master@{3}~2:README isn't a revision, what is it?

An extended SHA1 expression referring to a blob.

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

[Ramkumar Ramachandra]

Ramkumar Ramachandra wrote:
> '<rev>'::
>         Include commits that are reachable from (i.e. ancestors of)
>         <rev>.
>
> This doesn't make sense because <rev> might as well refer to a blob*.
> So this should say s/<rev>/<rev> that names a commit object/.

In other words, my fix is to change the meaning of "revision range" to
not literally mean "range of revisions", but rather "range of
(revisions that are commits)".  I think it is more sensible than
Jonathan's fix which breaks too many existing things; most
significantly, the names rev-parse/rev-list.

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

[Junio C Hamano]

Jonathan Nieder <jrnieder@gmail.com> writes:

> Ramkumar Ramachandra wrote:
>
>>                       I would say the most technically accurate
>> description of what 'git log' takes is a "committish range" (basically
>> a "revision range" that resolves to commits).
>
> What is a revision range that doesn't resolve to commits?  Am I wrong
> in thinking revision is nothing more than a synonym for commit?
>
> When gitrevisions(7) says "A revision parameter <rev> typically, but
> not necessarily, names a commit object", I suspect it is residue from
> 3a45f625 trying to apologize for the extended SHA1 syntax parser being
> called "git rev-parse" instead of "git object-name-parse".

 - A revision range is a slice of history.  There is no need for
   "committish revision range" as there is no other kinds of ranges.

 - A revision should be a synonym to a committish, as glossary
   defines.  We historically used "revision" more or less
   interchangeably with "object name", especially "an extended SHA-1
   expression that is used as an object name", to mean whatever
   get_sha1() can grok down to a single object name.  This must be
   rectified to avoid causing confusion to new readers of our
   documentation.

 - Documentation/revisions.txt needs to be cleaned up.  It lists
   "specifying revisions" and "specifying ranges", which is a good
   start primarily because some notations used in ranges are not
   revisions (e.g. "^C"), but it should have three (not two) sections.

   They are: "specifying revisions", "specifying object names", and
   "specifying ranges".  Move non committish specification such as
   <revision>:<path> from the current "specifying revisions" section
   and make the new "object names" section.  The "ranges" section is
   already written in terms of revisions, and does not need any
   update, once we clarify the definition of a "revision" as
   "committish".

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

[Ramkumar Ramachandra]

Junio C Hamano wrote:
>  - A revision range is a slice of history.  There is no need for
>    "committish revision range" as there is no other kinds of ranges.

For the record, 'git rev-parse master:README..HEAD@{3}~2' works just
fine.  I don't know whether you want to consider it "sensible" or not,
but the current revisions.txt is consistent with this.

>  - A revision should be a synonym to a committish, as glossary
>    defines.  We historically used "revision" more or less
>    interchangeably with "object name", especially "an extended SHA-1
>    expression that is used as an object name", to mean whatever
>    get_sha1() can grok down to a single object name.  This must be
>    rectified to avoid causing confusion to new readers of our
>    documentation.

The question is: who is the authority on this?  The combination of
rev-parse, rev-list, and revisions.txt, or gitglossary.txt.

>    They are: "specifying revisions", "specifying object names", and
>    "specifying ranges".

Personally, I don't like giving commit objects a special status, and
like things just the way they currently are.  Why do you want to
separate "revisions" (which are really just "extended SHA-1
expressions" that incidentally resolve to commit objects) and
"extended SHA-1 expressions that resolve to objects other than
commits"?  Is the current hand-wavy unclear gitglossary.txt the only
basis for your argument?

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

[Ramkumar Ramachandra]

Ramkumar Ramachandra wrote:
> For the record, 'git rev-parse master:README..HEAD@{3}~2' works just
> fine.  I don't know whether you want to consider it "sensible" or not,
> but the current revisions.txt is consistent with this.

rev-list errors out though:

  error: Object 15bfea is a blob, not a commit
  fatal: Invalid revision range master:README..HEAD@{3}~2

Hence my suggestion to change the meaning of what a valid "revision
range" is (using rev-list as the guideline) , and be done with it.

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

[Jonathan Nieder]

Ramkumar Ramachandra wrote:

>                                               Why do you want to
> separate "revisions" (which are really just "extended SHA-1
> expressions" that incidentally resolve to commit objects) and
> "extended SHA-1 expressions that resolve to objects other than
> commits"?

No one I've met actually uses the word "revisions" to mean anything
other than the English-language meaning of "revisions", which in a Git
context is "commits".  There's value in matching common usage.

Re: [PATCH/RFC] glossary: a revision is just a commit

[Junio C Hamano]

Jonathan Nieder <jrnieder@gmail.com> writes:

> The current definition of 'revision' sounds like it is saying that a
> revision is a tree object.  In reality it is just a commit.
>
> This should be especially useful for people used to other revision
> control systems trying to see how familiar concepts translate into git
> terms.
>
> Reported-by: Ramkumar Ramachandra <artagnon@gmail.com>
> Signed-off-by: Jonathan Nieder <jrnieder@gmail.com>
> ---

I think the existing glossary text is just confusing (it does wants
to refer to a commit (noun) as a concept, as opposed to its concrete
realization, i.e. a commit object, and tries to do so but does a bad
job).

Your update is an improvement, but there may need a note that says
that various web pages and documents, especially historical ones
[*1*], may loosely use this word to refer to any single object that
is not necessarily a commit at times.

> diff --git a/Documentation/glossary-content.txt b/Documentation/glossary-content.txt
> index eb7ba84f..521fceeb 100644
> --- a/Documentation/glossary-content.txt
> +++ b/Documentation/glossary-content.txt
> @@ -430,9 +430,7 @@ should not be combined with other pathspec.
>  	<<def_merge,merge>> left behind.
>  
>  [[def_revision]]revision::
> -	A particular state of files and directories which was stored in the
> -	<<def_object_database,object database>>. It is referenced by a
> -	<<def_commit_object,commit object>>.
> +	Synonym for <<def_commit,commit>> (the noun).
>  
>  [[def_rewind]]rewind::
>  	To throw away part of the development, i.e. to assign the


[Footnote]

*1* The "rev-parse" command started as a tool to parse "revisions" aka
"commit", "revision range" e.g. "A..B" and non-revisions (pathspecs)
and flags.  It was primarily designed as a way to sift arguments
given to "git mylog -p A..B Makefile", so that a scripted "mylog"
can turn it into a pipeline that feeds "git rev-list A..B Makefile"
output as the input to "git diff-tree -p --stdin Makefile".

This was back when we did not even have non-committish extended
SHA-1 object name notation.  From the very start, "rev" in the name
"rev-parse" did not even mean that it is limited to "revision" aka
"commit".  Later we added tree:path form of extended SHA-1 notation
and "rev-parse --verify $it" has become its primary usage, while its
role as argument sifter has diminished as we made "log" a built-in,
not a pipeline.  Around that time, we loosely started calling
anything that get_sha1() can turn into an object name a "revision".

Also 99.9% of the time people use it to parse committish in real
life.

For these reasons, I do not think it is wise to rename "rev-parse"
to "object-name-parse".  "rev" in "rev-parse" may be related to
"revisions", and while it technically can operate on "revisions" (in
the colloquial "object name" sense), it is not limited to revisions.
And when it is used for revisions it mostly is used for "revisions"
(in the "nothing but committish" sense).  The name that most
faithfully reflects the use of the command is "git kitchen-sink"
these days.

The user still needs to know that in some context, "revision" may
loosely refer to "object name" when talking about a single object,
if only to understand the name of that command, let alone various
tutorials, screencasts and web resources people made over time.

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

[Junio C Hamano]

Junio C Hamano <gitster@pobox.com> writes:

> Two other things I noticed with the current text are:
> ...
>  * The part with the new "-L" option seems to be throwing the
>    overall formatting off.  Its second paragraph "<start> and <end>
>    can take..." is not indented to the same level as its first
>    paragraph "Trace the evolution of...", and the following items,
>    like "--full-line-diff" and "[--] <pathspec>..." are indented one
>    level too deeply.

This is a minimalistic patch to fix the formatting.  I removed the
extra sentence after the enumeration and moved it to the end of the
main text, but somebody may have a better idea to persuade AsciiDoc
to format it in a more reasonable way while keeping the sentence
there.

-- >8 --
Subject: line-log: fix documentation formatting

The second paragraph of the added description for the -L option
"<start> and <end> can take one of these forms:", and the list of
forms that follow the headline, were indented one level too short,
due to the missing "+" to signal that the next paragraph continues
the previous one.

Also "You can specify this option more than once" is about the -L
option, not about its various forms of starting and ending points.
Move it to the end of the main text.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---
 Documentation/git-log.txt | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt
index 4850226..0959f9d 100644
--- a/Documentation/git-log.txt
+++ b/Documentation/git-log.txt
@@ -76,12 +76,11 @@ produced by --stat etc.
 	not give any pathspec limiters.  This is currently limited to
 	a walk starting from a single revision, i.e., you may only
 	give zero or one positive revision arguments.
-
+	You can specify this option more than once.
++
 <start> and <end> can take one of these forms:
 
 include::line-range-format.txt[]
-You can specify this option more than once.
-
 
 --full-line-diff::
 	Always print the interesting range even if the current commit

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

[Thomas Rast]

Junio C Hamano <gitster@pobox.com> writes:

> This is a minimalistic patch to fix the formatting.  I removed the
> extra sentence after the enumeration and moved it to the end of the
> main text, but somebody may have a better idea to persuade AsciiDoc
> to format it in a more reasonable way while keeping the sentence
> there.
>
> -- >8 --
> Subject: line-log: fix documentation formatting
>
> The second paragraph of the added description for the -L option
> "<start> and <end> can take one of these forms:", and the list of
> forms that follow the headline, were indented one level too short,
> due to the missing "+" to signal that the next paragraph continues
> the previous one.
>
> Also "You can specify this option more than once" is about the -L
> option, not about its various forms of starting and ending points.
> Move it to the end of the main text.
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  Documentation/git-log.txt | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt
> index 4850226..0959f9d 100644
> --- a/Documentation/git-log.txt
> +++ b/Documentation/git-log.txt
> @@ -76,12 +76,11 @@ produced by --stat etc.
>  	not give any pathspec limiters.  This is currently limited to
>  	a walk starting from a single revision, i.e., you may only
>  	give zero or one positive revision arguments.
> -
> +	You can specify this option more than once.
> ++
>  <start> and <end> can take one of these forms:
>  
>  include::line-range-format.txt[]
> -You can specify this option more than once.
> -
>  

Sorry for being a bit late to this.  I think it's a good solution;
putting "You can specify this option more than once" after all the other
text was probably worse because it gets lost down there.

As for

>  --full-line-diff::
>  	Always print the interesting range even if the current commit

That's just stale and not currently implemented.  Sigh.  We should
remove it.

-- >8 --
Subject: [PATCH] git-log(1): remove --full-line-diff description

This option is a remnant of an earlier log -L version, and not
currently implemented.  Remove it until (if at all) it is implemented
again.

Signed-off-by: Thomas Rast <trast@inf.ethz.ch>
---
 Documentation/git-log.txt | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt
index 0959f9d..65707ce 100644
--- a/Documentation/git-log.txt
+++ b/Documentation/git-log.txt
@@ -82,10 +82,6 @@ produced by --stat etc.
 
 include::line-range-format.txt[]
 
---full-line-diff::
-	Always print the interesting range even if the current commit
-	does not change any line of the range.
-
 [\--] <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.844.g59e84de.dirty

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

[Thomas Rast]

Thomas Rast <trast@inf.ethz.ch> writes:

> -- >8 --
> Subject: [PATCH] git-log(1): remove --full-line-diff description

BTW, I generated this with your jc/format-patch, but it stopped working
after fc/send-email-annotate made it into next; I need this on top.  Am
I missing something?

-- >8 --
Subject: [PATCH] FIXUP jc/format-patch: adapt for fc/send-email-annotate

2a4c260 (format-patch: add format.coverLetter configuration variable,
2013-04-07) changed the coverletter variable to -1 by default, so the
die("... incompatible") always triggers.  Test if it is >0 instead.
---
 builtin/log.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/log.c b/builtin/log.c
index 4804229..c972e62 100644
--- a/builtin/log.c
+++ b/builtin/log.c
@@ -1247,7 +1247,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
 	/* Set defaults and check incompatible options */
 	if (rev.inline_single) {
 		use_stdout = 1;
-		if (cover_letter)
+		if (cover_letter > 0)
 			die(_("inline-single and cover-letter are incompatible."));
 		if (thread)
 			die(_("inline-single and thread are incompatible."));
-- 
1.8.2.1.844.g59e84de.dirty

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

[Junio C Hamano]

Thomas Rast <trast@inf.ethz.ch> writes:

> Thomas Rast <trast@inf.ethz.ch> writes:
>
>> -- >8 --
>> Subject: [PATCH] git-log(1): remove --full-line-diff description
>
> BTW, I generated this with your jc/format-patch, but it stopped working
> after fc/send-email-annotate made it into next; I need this on top.  Am
> I missing something?

No, the topic has been stalled and left behind and needs to be
rebased on top of that other topic with your patch.  Thanks.

It also needs a lot more work to de-mime its output to be eligible
for 'next', though.

> -- >8 --
> Subject: [PATCH] FIXUP jc/format-patch: adapt for fc/send-email-annotate
>
> 2a4c260 (format-patch: add format.coverLetter configuration variable,
> 2013-04-07) changed the coverletter variable to -1 by default, so the
> die("... incompatible") always triggers.  Test if it is >0 instead.
> ---
>  builtin/log.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/builtin/log.c b/builtin/log.c
> index 4804229..c972e62 100644
> --- a/builtin/log.c
> +++ b/builtin/log.c
> @@ -1247,7 +1247,7 @@ int cmd_format_patch(int argc, const char **argv, const char *prefix)
>  	/* Set defaults and check incompatible options */
>  	if (rev.inline_single) {
>  		use_stdout = 1;
> -		if (cover_letter)
> +		if (cover_letter > 0)
>  			die(_("inline-single and cover-letter are incompatible."));
>  		if (thread)
>  			die(_("inline-single and thread are incompatible."));