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

2013-04-22 Thread Thomas Rast
Junio C Hamano gits...@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 gits...@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 tr...@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
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2013-04-22 Thread Junio C Hamano
Thomas Rast tr...@inf.ethz.ch writes:

 Thomas Rast tr...@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.));
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2013-04-21 Thread Junio C Hamano
Ramkumar Ramachandra artag...@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 artag...@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.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2013-04-21 Thread 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).
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2013-04-21 Thread Junio C Hamano
Ramkumar Ramachandra artag...@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.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2013-04-21 Thread 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.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2013-04-21 Thread Junio C Hamano
Junio C Hamano gits...@pobox.com writes:

 Ramkumar Ramachandra artag...@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.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2013-04-21 Thread Junio C Hamano
Ramkumar Ramachandra artag...@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.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2013-04-21 Thread 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.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2013-04-21 Thread 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).
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2013-04-21 Thread 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.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2013-04-21 Thread 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.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2013-04-21 Thread 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.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2013-04-21 Thread 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.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2013-04-21 Thread 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.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2013-04-21 Thread Junio C Hamano
Jonathan Nieder jrnie...@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.



--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2013-04-21 Thread 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?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2013-04-21 Thread 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.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2013-04-21 Thread Junio C Hamano
Junio C Hamano gits...@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 gits...@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
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2013-04-20 Thread 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 artag...@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

--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html