[PATCH 0/5] Documentation/git-diff.txt improvements

[PATCH 0/5] Documentation/git-diff.txt improvements

[Ramkumar Ramachandra]

Hi again,

So I decided that builtin/diff.c is hardcoded for the
<commit>..<commit> and <commit>...<commit> forms, and we can do
nothing about it unless we want to break compatibility (maybe a git
2.0 candidate?).  The least we can do is document it properly in the
SYNOPSIS.  I've done this in [4/5].

The other patches are just general "while we're there" cleanups.

Ramkumar Ramachandra (5):
  git-diff.txt: reorder the <commit> <commit> form
  git-diff.txt: strip the leading "--" from options template
  git-diff.txt: group the [--] and [<path>...] templates
  git-diff.txt: document the .. and ... forms in SYNOPSIS
  git-diff.txt: fix ordering of the [--no-index] form

 Documentation/git-diff.txt | 42 ++++++++++++++++++++++--------------------
 1 file changed, 22 insertions(+), 20 deletions(-)

-- 
1.8.2.1.502.g2d60b5c

[PATCH 1/5] git-diff.txt: reorder the <commit> <commit> form

[Ramkumar Ramachandra]

In DESCRIPTION, the '<commit>..<commit>' form refers to "the previous
form", namely the '<commit> <commit>' form.  However, bd52900
(Documentation: Describe "git diff <blob> <blob>" separately,
2012-12-18) broke this by inserting a form in between these two forms.
Fix this by reordering a form.

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

diff --git a/Documentation/git-diff.txt b/Documentation/git-diff.txt
index a7b4620..8623867 100644
--- a/Documentation/git-diff.txt
+++ b/Documentation/git-diff.txt
@@ -11,8 +11,8 @@ SYNOPSIS
 [verse]
 'git diff' [options] [<commit>] [--] [<path>...]
 'git diff' [options] --cached [<commit>] [--] [<path>...]
-'git diff' [options] <commit> <commit> [--] [<path>...]
 'git diff' [options] <blob> <blob>
+'git diff' [options] <commit> <commit> [--] [<path>...]
 'git diff' [options] [--no-index] [--] <path> <path>
 
 DESCRIPTION
@@ -51,16 +51,16 @@ directories. This behavior can be forced by --no-index.
 	branch name to compare with the tip of a different
 	branch.
 
-'git diff' [--options] <commit> <commit> [--] [<path>...]::
-
-	This is to view the changes between two arbitrary
-	<commit>.
-
 'git diff' [options] <blob> <blob>::
 
 	This form is to view the differences between the raw
 	contents of two blob objects.
 
+'git diff' [--options] <commit> <commit> [--] [<path>...]::
+
+	This is to view the changes between two arbitrary
+	<commit>.
+
 'git diff' [--options] <commit>..<commit> [--] [<path>...]::
 
 	This is synonymous to the previous form.  If <commit> on
-- 
1.8.2.1.502.g2d60b5c

Re: [PATCH 1/5] git-diff.txt: reorder the <commit> <commit> form

[Jonathan Nieder]

Ramkumar Ramachandra wrote:

> In DESCRIPTION, the '<commit>..<commit>' form refers to "the previous
> form", namely the '<commit> <commit>' form.  However, bd52900

Good catch.

[...]
> --- a/Documentation/git-diff.txt
> +++ b/Documentation/git-diff.txt
> @@ -11,8 +11,8 @@ SYNOPSIS
>  [verse]
>  'git diff' [options] [<commit>] [--] [<path>...]
>  'git diff' [options] --cached [<commit>] [--] [<path>...]
> -'git diff' [options] <commit> <commit> [--] [<path>...]
>  'git diff' [options] <blob> <blob>
> +'git diff' [options] <commit> <commit> [--] [<path>...]
>  'git diff' [options] [--no-index] [--] <path> <path>

For the sake of exposition I think it makes more sense to put
<commit> <commit> *before* <blob> <blob>, since the former is used
more often.

Thanks,
Jonathan

[PATCH 2/5] git-diff.txt: strip the leading "--" from options template

[Ramkumar Ramachandra]

It is imperative to specify options with the [options] template.
[--options] is inconsistent as well as misleading (as there are short
options which can be specified with a single "-").  Fix this.

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

diff --git a/Documentation/git-diff.txt b/Documentation/git-diff.txt
index 8623867..47aa3fc 100644
--- a/Documentation/git-diff.txt
+++ b/Documentation/git-diff.txt
@@ -21,7 +21,7 @@ Show changes between the working tree and the index or a tree, changes
 between the index and a tree, changes between two trees, or changes
 between two files on disk.
 
-'git diff' [--options] [--] [<path>...]::
+'git diff' [options] [--] [<path>...]::
 
 	This form is to view the changes you made relative to
 	the index (staging area for the next commit).  In other
@@ -33,7 +33,7 @@ If exactly two paths are given and at least one points outside
 the current repository, 'git diff' will compare the two files /
 directories. This behavior can be forced by --no-index.
 
-'git diff' [--options] --cached [<commit>] [--] [<path>...]::
+'git diff' [options] --cached [<commit>] [--] [<path>...]::
 
 	This form is to view the changes you staged for the next
 	commit relative to the named <commit>.  Typically you
@@ -43,7 +43,7 @@ directories. This behavior can be forced by --no-index.
 	<commit> is not given, it shows all staged changes.
 	--staged is a synonym of --cached.
 
-'git diff' [--options] <commit> [--] [<path>...]::
+'git diff' [options] <commit> [--] [<path>...]::
 
 	This form is to view the changes you have in your
 	working tree relative to the named <commit>.  You can
@@ -56,18 +56,18 @@ directories. This behavior can be forced by --no-index.
 	This form is to view the differences between the raw
 	contents of two blob objects.
 
-'git diff' [--options] <commit> <commit> [--] [<path>...]::
+'git diff' [options] <commit> <commit> [--] [<path>...]::
 
 	This is to view the changes between two arbitrary
 	<commit>.
 
-'git diff' [--options] <commit>..<commit> [--] [<path>...]::
+'git diff' [options] <commit>..<commit> [--] [<path>...]::
 
 	This is synonymous to the previous form.  If <commit> on
 	one side is omitted, it will have the same effect as
 	using HEAD instead.
 
-'git diff' [--options] <commit>\...<commit> [--] [<path>...]::
+'git diff' [options] <commit>\...<commit> [--] [<path>...]::
 
 	This form is to view the changes on the branch containing
 	and up to the second <commit>, starting at a common ancestor
-- 
1.8.2.1.502.g2d60b5c

Re: [PATCH 2/5] git-diff.txt: strip the leading "--" from options template

[Jonathan Nieder]

Ramkumar Ramachandra wrote:

> It is imperative to specify options with the [options] template.

Why is it imperative?

Anyway, this looks like a good change.  With that sentence removed
or some other clarification,

Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>

Re: [PATCH 2/5] git-diff.txt: strip the leading "--" from options template

[Ramkumar Ramachandra]

Jonathan Nieder wrote:
> Why is it imperative?

Sorry about the thinko; s/imperative/idiomatic/

Re: [PATCH 2/5] git-diff.txt: strip the leading "--" from options template

[Junio C Hamano]

Ramkumar Ramachandra <artagnon@gmail.com> writes:

> Jonathan Nieder wrote:
>> Why is it imperative?
>
> Sorry about the thinko; s/imperative/idiomatic/

Yeah, I think this step looks reasonable.

[PATCH 3/5] git-diff.txt: group the [--] and [<path>...] templates

[Ramkumar Ramachandra]

Clarify that "--" is meant to disambiguate paths from the other
options.

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

diff --git a/Documentation/git-diff.txt b/Documentation/git-diff.txt
index 47aa3fc..a0fdfc2 100644
--- a/Documentation/git-diff.txt
+++ b/Documentation/git-diff.txt
@@ -9,11 +9,11 @@ git-diff - Show changes between commits, commit and working tree, etc
 SYNOPSIS
 --------
 [verse]
-'git diff' [options] [<commit>] [--] [<path>...]
-'git diff' [options] --cached [<commit>] [--] [<path>...]
+'git diff' [options] [<commit>] [[--] [<path>...]]
+'git diff' [options] --cached [<commit>] [[--] [<path>...]]
 'git diff' [options] <blob> <blob>
-'git diff' [options] <commit> <commit> [--] [<path>...]
-'git diff' [options] [--no-index] [--] <path> <path>
+'git diff' [options] <commit> <commit> [[--] [<path>...]]
+'git diff' [options] [--no-index] [[--] <path> <path>]
 
 DESCRIPTION
 -----------
@@ -21,7 +21,7 @@ Show changes between the working tree and the index or a tree, changes
 between the index and a tree, changes between two trees, or changes
 between two files on disk.
 
-'git diff' [options] [--] [<path>...]::
+'git diff' [options] [[--] [<path>...]]::
 
 	This form is to view the changes you made relative to
 	the index (staging area for the next commit).  In other
@@ -33,7 +33,7 @@ If exactly two paths are given and at least one points outside
 the current repository, 'git diff' will compare the two files /
 directories. This behavior can be forced by --no-index.
 
-'git diff' [options] --cached [<commit>] [--] [<path>...]::
+'git diff' [options] --cached [<commit>] [[--] [<path>...]]::
 
 	This form is to view the changes you staged for the next
 	commit relative to the named <commit>.  Typically you
@@ -43,7 +43,7 @@ directories. This behavior can be forced by --no-index.
 	<commit> is not given, it shows all staged changes.
 	--staged is a synonym of --cached.
 
-'git diff' [options] <commit> [--] [<path>...]::
+'git diff' [options] <commit> [[--] [<path>...]]::
 
 	This form is to view the changes you have in your
 	working tree relative to the named <commit>.  You can
@@ -56,18 +56,18 @@ directories. This behavior can be forced by --no-index.
 	This form is to view the differences between the raw
 	contents of two blob objects.
 
-'git diff' [options] <commit> <commit> [--] [<path>...]::
+'git diff' [options] <commit> <commit> [[--] [<path>...]]::
 
 	This is to view the changes between two arbitrary
 	<commit>.
 
-'git diff' [options] <commit>..<commit> [--] [<path>...]::
+'git diff' [options] <commit>..<commit> [[--] [<path>...]]::
 
 	This is synonymous to the previous form.  If <commit> on
 	one side is omitted, it will have the same effect as
 	using HEAD instead.
 
-'git diff' [options] <commit>\...<commit> [--] [<path>...]::
+'git diff' [options] <commit>\...<commit> [[--] [<path>...]]::
 
 	This form is to view the changes on the branch containing
 	and up to the second <commit>, starting at a common ancestor
-- 
1.8.2.1.502.g2d60b5c

Re: [PATCH 3/5] git-diff.txt: group the [--] and [<path>...] templates

[Junio C Hamano]

Ramkumar Ramachandra <artagnon@gmail.com> writes:

> -'git diff' [options] [--] [<path>...]::
> +'git diff' [options] [[--] [<path>...]]::

You can say

    "git diff A B --" without any path
    "git diff A B pathspec" without any double-dashes
    "git diff -- pathspec"

and all three of them are expressed by versions before or after the
patch, so there is no change in the semantics introduced by this
patch.

While the update might be logically more correct, it looks to me
that the only end-user visibile effect of it is to make the end
result harder to read.

Re: [PATCH 3/5] git-diff.txt: group the [--] and [<path>...] templates

[Ramkumar Ramachandra]

Junio C Hamano wrote:
> While the update might be logically more correct, it looks to me
> that the only end-user visibile effect of it is to make the end
> result harder to read.

Hm, I thought it improves readability.  I'm trying to say that -- is
used to separate [<path>...] from [everything else].

Re: [PATCH 3/5] git-diff.txt: group the [--] and [<path>...] templates

[Junio C Hamano]

Ramkumar Ramachandra <artagnon@gmail.com> writes:

> Junio C Hamano wrote:
>>> -'git diff' [options] [--] [<path>...]::
>>> +'git diff' [options] [[--] [<path>...]]::
>>
>> While the update might be logically more correct, it looks to me
>> that the only end-user visibile effect of it is to make the end
>> result harder to read.
>
> Hm, I thought it improves readability.  I'm trying to say that -- is
> used to separate [<path>...] from [everything else].

The same logic would apply to this semi-nonsense rewrite, no?

	git diff [[options] [--]] [<path>...]

Everything else comes before "--" (if exists) that separates it from
the list of pathspecs.

Re: [PATCH 3/5] git-diff.txt: group the [--] and [<path>...] templates

[Ramkumar Ramachandra]

Junio C Hamano wrote:
> The same logic would apply to this semi-nonsense rewrite, no?
>
>         git diff [[options] [--]] [<path>...]
>
> Everything else comes before "--" (if exists) that separates it from
> the list of pathspecs.

Yeah, except it's sometimes

    [[options] [revision range] [--]] [<path>...]

and other times

    [[options] [<commit>...] [--]] [<path>...]

and yet other times

    [[options] [--]] [<path>...]

Whereas mine is always

    ... [[--] [<path>]]

(everywhere in git)

Re: [PATCH 3/5] git-diff.txt: group the [--] and [<path>...] templates

[Junio C Hamano]

Ramkumar Ramachandra <artagnon@gmail.com> writes:

> Junio C Hamano wrote:
>> The same logic would apply to this semi-nonsense rewrite, no?
>>
>>         git diff [[options] [--]] [<path>...]
>>
>> Everything else comes before "--" (if exists) that separates it from
>> the list of pathspecs.
>
> Yeah, except it's sometimes
>
>     [[options] [revision range] [--]] [<path>...]
>
> and other times
>
>     [[options] [<commit>...] [--]] [<path>...]
>
> and yet other times
>
>     [[options] [--]] [<path>...]
>
> Whereas mine is always
>
>     ... [[--] [<path>]]
>
> (everywhere in git)

You are missing the entire point.  Neither [[everything else][--]][path]
or [everything else][[--][path]] is better than without the extra
level of [], which does not help clarifying _anything_.

Re: [PATCH 3/5] git-diff.txt: group the [--] and [<path>...] templates

[Ramkumar Ramachandra]

Junio C Hamano wrote:
> You are missing the entire point.

Yeah, okay: I'll drop this patch.  I was just saying; I don't feel
strongly about it at all.

Re: [PATCH 3/5] git-diff.txt: group the [--] and [<path>...] templates

[Jonathan Nieder]

Ramkumar Ramachandra wrote:

> Hm, I thought it improves readability.  I'm trying to say that -- is
> used to separate [<path>...] from [everything else].

I agree with the goal, but I don't think this change achieves it.
Maybe adding gitcli(7) to the SEE ALSO section would work?

Jonathan

[PATCH 4/5] git-diff.txt: document the .. and ... forms in SYNOPSIS

[Ramkumar Ramachandra]

The DESCRIPTION refers to the '<commit>..<commit>' and
'<commit>...<commit>' forms, but the SYNOPSIS does not list them at
all.  Fix this.

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

diff --git a/Documentation/git-diff.txt b/Documentation/git-diff.txt
index a0fdfc2..291e250 100644
--- a/Documentation/git-diff.txt
+++ b/Documentation/git-diff.txt
@@ -13,6 +13,8 @@ SYNOPSIS
 'git diff' [options] --cached [<commit>] [[--] [<path>...]]
 'git diff' [options] <blob> <blob>
 'git diff' [options] <commit> <commit> [[--] [<path>...]]
+'git diff' [options] <commit>..<commit> [[--] [<path>...]]
+'git diff' [options] <commit>\...<commit> [[--] [<path>...]]
 'git diff' [options] [--no-index] [[--] <path> <path>]
 
 DESCRIPTION
-- 
1.8.2.1.502.g2d60b5c

Re: [PATCH 4/5] git-diff.txt: document the .. and ... forms in SYNOPSIS

[Jonathan Nieder]

Ramkumar Ramachandra wrote:

> --- a/Documentation/git-diff.txt
> +++ b/Documentation/git-diff.txt
> @@ -13,6 +13,8 @@ SYNOPSIS
>  'git diff' [options] --cached [<commit>] [[--] [<path>...]]
>  'git diff' [options] <blob> <blob>
>  'git diff' [options] <commit> <commit> [[--] [<path>...]]
> +'git diff' [options] <commit>..<commit> [[--] [<path>...]]
> +'git diff' [options] <commit>\...<commit> [[--] [<path>...]]
>  'git diff' [options] [--no-index] [[--] <path> <path>]

This is preparation for removing <commit>..<commit> from the
documentation, right?  It makes sense to add the ...  form since
there's no other way to spell it but I can't say I'm excited about
adding the .. form.  Especially since <tree>..<tree> doesn't work,
that form's a little embarrassing. ;-)

*checks the manpage*

Ah, this is to make the SYNOPSIS a better table of contents for the
DESCRIPTION (except that the DESCRIPTION treats "git diff --no-index
<path> <path>" before "git diff [<commit>] -- <pathspec>" while the
SYNOPSIS doesn't).  Maybe it would make sense to go all the way and
make the two follow the same order?

My 2¢,
Jonathan

[PATCH 5/5] git-diff.txt: fix ordering of the [--no-index] form

[Ramkumar Ramachandra]

The SYNOPSIS lists the [--no-index] form as the last item, but the
DESCRIPTION lists it as a natural extension of the first form.  Fix
this with a reordering.  Also since the DESCRIPTION breaks up the
first form in the SYNOPSIS into two different forms (one without the
optional [<commit>], and the other with it), reorder to make these two
forms appear together.

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

diff --git a/Documentation/git-diff.txt b/Documentation/git-diff.txt
index 291e250..1f2f681 100644
--- a/Documentation/git-diff.txt
+++ b/Documentation/git-diff.txt
@@ -10,12 +10,12 @@ SYNOPSIS
 --------
 [verse]
 'git diff' [options] [<commit>] [[--] [<path>...]]
+'git diff' [options] [--no-index] [[--] <path> <path>]
 'git diff' [options] --cached [<commit>] [[--] [<path>...]]
 'git diff' [options] <blob> <blob>
 'git diff' [options] <commit> <commit> [[--] [<path>...]]
 'git diff' [options] <commit>..<commit> [[--] [<path>...]]
 'git diff' [options] <commit>\...<commit> [[--] [<path>...]]
-'git diff' [options] [--no-index] [[--] <path> <path>]
 
 DESCRIPTION
 -----------
@@ -35,6 +35,14 @@ If exactly two paths are given and at least one points outside
 the current repository, 'git diff' will compare the two files /
 directories. This behavior can be forced by --no-index.
 
+'git diff' [options] <commit> [[--] [<path>...]]::
+
+	This form is to view the changes you have in your
+	working tree relative to the named <commit>.  You can
+	use HEAD to compare it with the latest commit, or a
+	branch name to compare with the tip of a different
+	branch.
+
 'git diff' [options] --cached [<commit>] [[--] [<path>...]]::
 
 	This form is to view the changes you staged for the next
@@ -45,14 +53,6 @@ directories. This behavior can be forced by --no-index.
 	<commit> is not given, it shows all staged changes.
 	--staged is a synonym of --cached.
 
-'git diff' [options] <commit> [[--] [<path>...]]::
-
-	This form is to view the changes you have in your
-	working tree relative to the named <commit>.  You can
-	use HEAD to compare it with the latest commit, or a
-	branch name to compare with the tip of a different
-	branch.
-
 'git diff' [options] <blob> <blob>::
 
 	This form is to view the differences between the raw
-- 
1.8.2.1.502.g2d60b5c

Re: [PATCH 5/5] git-diff.txt: fix ordering of the [--no-index] form

[Junio C Hamano]

Ramkumar Ramachandra <artagnon@gmail.com> writes:

> The SYNOPSIS lists the [--no-index] form as the last item, but the
> DESCRIPTION lists it as a natural extension of the first form.

Perhaps either the description or your reading is wrong.

The "--no-index" mode was a hack to allow "git diff" goodies to be
used outside the context of "git", and a proper execution of it
should have been to send patches to GNU or BSD diff maintainers, not
to add the "--no-index" option that is unrelated to "git" to our
code.

I think it is more correct to list it as a footnote and list
separately from the main text, which is what the original SYNOPSIS
section does.

Re: [PATCH 5/5] git-diff.txt: fix ordering of the [--no-index] form

[Ramkumar Ramachandra]

Junio C Hamano wrote:
> The "--no-index" mode was a hack to allow "git diff" goodies to be
> used outside the context of "git", and a proper execution of it
> should have been to send patches to GNU or BSD diff maintainers, not
> to add the "--no-index" option that is unrelated to "git" to our
> code.

Yeah, I know.  I use it instead of diff all the time.

> I think it is more correct to list it as a footnote and list
> separately from the main text, which is what the original SYNOPSIS
> section does.

I was trying to be more "correct" here.  The form is a special case of
the first form where the number of paths are limited to two.  Besides,
isn't that how the DESCRIPTION section explains it now?

Re: [PATCH 5/5] git-diff.txt: fix ordering of the [--no-index] form

[Jonathan Nieder]

Ramkumar Ramachandra wrote:

>                                          The form is a special case of
> the first form where the number of paths are limited to two.  Besides,
> isn't that how the DESCRIPTION section explains it now?

Sort of.  It's a completely different form, but when --no-index is
left out the syntax is a special case of "git diff -- <path>...".

Maybe it would make sense to move towards eliminating the "implicit
--no-index for paths outside the repository" trick.  I use "git diff
--no-index" all the time, but I always spell it out to be careful.

Re: [PATCH 5/5] git-diff.txt: fix ordering of the [--no-index] form

[Ramkumar Ramachandra]

Jonathan Nieder wrote:
> Maybe it would make sense to move towards eliminating the "implicit
> --no-index for paths outside the repository" trick.  I use "git diff
> --no-index" all the time, but I always spell it out to be careful.

Huh?  Why do you want to endure the pain of spelling it out, when your
intent is perfectly clear?

What would make sense is a tristate diff.no-index:
inside-worktree|outside-worktree|always.  I don't know why we should
even have a "never".

Re: [PATCH 5/5] git-diff.txt: fix ordering of the [--no-index] form

[Jonathan Nieder]

Ramkumar Ramachandra wrote:
> Jonathan Nieder wrote:

>> Maybe it would make sense to move towards eliminating the "implicit
>> --no-index for paths outside the repository" trick.  I use "git diff
>> --no-index" all the time, but I always spell it out to be careful.
>
> Huh?  Why do you want to endure the pain of spelling it out, when your
> intent is perfectly clear?

Because typing paths does not make my intent perfectly clear.

> What would make sense is a tristate diff.no-index:

Oh please no.

Hope that helps,
Jonathan

Re: [PATCH 5/5] git-diff.txt: fix ordering of the [--no-index] form

[Ramkumar Ramachandra]

Jonathan Nieder wrote:
> Oh please no.

If I understand this correctly, you are horrified by this?
https://github.com/artagnon/dotfiles/blob/master/.gitconfig#L30

By the way, my zsh aliases git to g.

Re: [PATCH 5/5] git-diff.txt: fix ordering of the [--no-index] form

[Jonathan Nieder]

Ramkumar Ramachandra wrote:
> Jonathan Nieder wrote:

>> Oh please no.
>
> If I understand this correctly, you are horrified by this?
> https://github.com/artagnon/dotfiles/blob/master/.gitconfig#L30

Nope, that looks like a useful way to save typing, and "git help"
helpfully expands any of your custom commands when you want to
translate in order to help other people.

Re: [PATCH 5/5] git-diff.txt: fix ordering of the [--no-index] form

[Ramkumar Ramachandra]

Jonathan Nieder wrote:
> Because typing paths does not make my intent perfectly clear.

I'm not able to understand this.  Doesn't your prompt tell you which
directory you're in, and if you're in a git repository?  When you type
out paths, you know what is inside and what is outside your
repository.  By extension, you know when --no-index is implied and
when it isn't.

Can you explain what your problem is?

Re: [PATCH 5/5] git-diff.txt: fix ordering of the [--no-index] form

[Jonathan Nieder]

Ramkumar Ramachandra wrote:
> Jonathan Nieder wrote:

>> Because typing paths does not make my intent perfectly clear.
>
> I'm not able to understand this.  Doesn't your prompt tell you which
> directory you're in, and if you're in a git repository?  When you type
> out paths, you know what is inside and what is outside your
> repository.  By extension, you know when --no-index is implied and
> when it isn't.
>
> Can you explain what your problem is?

Are you wondering why I use --no-index or why I think we should
eventually stop moving into --no-index mode by default?

The answers are different.  I use --no-index because it means I don't
have to think about whether the files I am comparing are in a Git
repository.  It's relaxing.  I'm not advocating that you follow suit;
I'm just describing my own usage of the command.

I think git should eventually stop doing --no-index implicitly because
it is a very different mode using the same syntax triggered by
external conditions.  That is

 * hard to document
 * problematic for scripts calling "git diff", can create lurking bugs
 * unnecessarily complicated.  For example:
   * "git diff -h" output depends on whether I am in a git worktree.
   * git searches for a .git directory, possibly hitting filesystem
     automount points, just to decide whether its arguments are inside
     the current repository.

Re: [PATCH 5/5] git-diff.txt: fix ordering of the [--no-index] form

[Ramkumar Ramachandra]

Jonathan Nieder wrote:
>  * hard to document
> [...]

I completely disagree, but we don't have to agree: make it a
configuration variable.  Even if it's turned to "never" by default, I
don't mind having one extra line in my .gitconfig.  But you went all
"Oh please no" when I brought it up.  I thought you were claiming that
nobody finds it useful.  Now, you seem to be claiming that people who
find it useful must suffer.

Re: [PATCH 5/5] git-diff.txt: fix ordering of the [--no-index] form

[Jonathan Nieder]

Ramkumar Ramachandra wrote:

> I completely disagree, but we don't have to agree: make it a
> configuration variable.

I thought we had discussed before how every configuration variable
costs quite a lot in terms of Git's teachability.

What would that configuration variable even mean?  "Set this to make
other people's scripts work when they assume --no-index won't be
triggered automatically"?

Re: [PATCH 5/5] git-diff.txt: fix ordering of the [--no-index] form

[Ramkumar Ramachandra]

Jonathan Nieder wrote:
> What would that configuration variable even mean?  "Set this to make
> other people's scripts work when they assume --no-index won't be
> triggered automatically"?

No.  "Set this for interactive use".

[PATCH 6/5] git-diff.txt: banish the <commit>..<commit> form

[Ramkumar Ramachandra]

The '<commit>..<commit>' and '<commit>...<commit>' forms are confusing
as they are reminiscent of the corresponding forms in the "SPECIFYING
RANGES" section of revisions.txt.  We can remove the
'<commit>..<commit>' form now (hence discouraging its use), since it
is exactly equivalent to the clearer '<commit> <commit>' form.
However, we must keep the '<commit>...<commit>' form around until we
can find a different way to express the same thing.

Suggested-by: Junio C Hamano <gitster@pobox.com>
Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 Documentation/git-diff.txt | 21 ++++++---------------
 1 file changed, 6 insertions(+), 15 deletions(-)

diff --git a/Documentation/git-diff.txt b/Documentation/git-diff.txt
index 1f2f681..118b448 100644
--- a/Documentation/git-diff.txt
+++ b/Documentation/git-diff.txt
@@ -14,7 +14,6 @@ SYNOPSIS
 'git diff' [options] --cached [<commit>] [[--] [<path>...]]
 'git diff' [options] <blob> <blob>
 'git diff' [options] <commit> <commit> [[--] [<path>...]]
-'git diff' [options] <commit>..<commit> [[--] [<path>...]]
 'git diff' [options] <commit>\...<commit> [[--] [<path>...]]
 
 DESCRIPTION
@@ -63,12 +62,6 @@ directories. This behavior can be forced by --no-index.
 	This is to view the changes between two arbitrary
 	<commit>.
 
-'git diff' [options] <commit>..<commit> [[--] [<path>...]]::
-
-	This is synonymous to the previous form.  If <commit> on
-	one side is omitted, it will have the same effect as
-	using HEAD instead.
-
 'git diff' [options] <commit>\...<commit> [[--] [<path>...]]::
 
 	This form is to view the changes on the branch containing
@@ -79,14 +72,12 @@ directories. This behavior can be forced by --no-index.
 
 Just in case if you are doing something exotic, it should be
 noted that all of the <commit> in the above description, except
-in the last two forms that use ".." notations, can be any
-<tree>.
-
-For a more complete list of ways to spell <commit>, see
-"SPECIFYING REVISIONS" section in linkgit:gitrevisions[7].
-However, "diff" is about comparing two _endpoints_, not ranges,
-and the range notations ("<commit>..<commit>" and
-"<commit>\...<commit>") do not mean a range as defined in the
+in the last "..." form, can be any <tree>.
+
+For a more complete list of ways to spell <commit>, see "SPECIFYING
+REVISIONS" section in linkgit:gitrevisions[7].  However, "diff" is
+about comparing two _endpoints_, not ranges, and the range notation
+"<commit>...<commit>" does not mean a range as defined in the
 "SPECIFYING RANGES" section in linkgit:gitrevisions[7].
 
 OPTIONS
-- 
1.8.2.1.502.g2d60b5c

Re: [PATCH 6/5] git-diff.txt: banish the <commit>..<commit> form

[Junio C Hamano]

Ramkumar Ramachandra <artagnon@gmail.com> writes:

> The '<commit>..<commit>' and '<commit>...<commit>' forms are confusing
> as they are reminiscent of the corresponding forms in the "SPECIFYING
> RANGES" section of revisions.txt.  We can remove the
> '<commit>..<commit>' form now (hence discouraging its use), since it
> is exactly equivalent to the clearer '<commit> <commit>' form.
> However, we must keep the '<commit>...<commit>' form around until we
> can find a different way to express the same thing.
>
> Suggested-by: Junio C Hamano <gitster@pobox.com>
> Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
> ---
>  Documentation/git-diff.txt | 21 ++++++---------------
>  1 file changed, 6 insertions(+), 15 deletions(-)
>
> diff --git a/Documentation/git-diff.txt b/Documentation/git-diff.txt
> index 1f2f681..118b448 100644
> --- a/Documentation/git-diff.txt
> +++ b/Documentation/git-diff.txt
> @@ -14,7 +14,6 @@ SYNOPSIS
>  'git diff' [options] --cached [<commit>] [[--] [<path>...]]
>  'git diff' [options] <blob> <blob>
>  'git diff' [options] <commit> <commit> [[--] [<path>...]]
> -'git diff' [options] <commit>..<commit> [[--] [<path>...]]

I agree with the end result not to list .. form in the SYNOPSIS, but
you shouldn't have added it in the first place in the earlier patch.

>  'git diff' [options] <commit>\...<commit> [[--] [<path>...]]
>  
>  DESCRIPTION
> @@ -63,12 +62,6 @@ directories. This behavior can be forced by --no-index.
>  	This is to view the changes between two arbitrary
>  	<commit>.
>  
> -'git diff' [options] <commit>..<commit> [[--] [<path>...]]::
> -
> -	This is synonymous to the previous form.  If <commit> on
> -	one side is omitted, it will have the same effect as
> -	using HEAD instead.

I do not think it is a bad idea to drop this enumerated item, but I
would have expected a sidenote to be added for the description for
"git diff A B" form, so that people who saw "git diff A..B" somewhere
on the web and wonder what it means can look it up.

Re: [PATCH 6/5] git-diff.txt: banish the <commit>..<commit> form

[Ramkumar Ramachandra]

On Wed, Apr 24, 2013 at 11:59 PM, Junio C Hamano <gitster@pobox.com> wrote:
> I agree with the end result not to list .. form in the SYNOPSIS, but
> you shouldn't have added it in the first place in the earlier patch.

I'm expecting to re-roll anyway.  I just wanted to show it to you now.

> I do not think it is a bad idea to drop this enumerated item, but I
> would have expected a sidenote to be added for the description for
> "git diff A B" form, so that people who saw "git diff A..B" somewhere
> on the web and wonder what it means can look it up.

Yeah, good idea.  I'll put it in the section after the ... syntax.

Re: [PATCH 0/5] Documentation/git-diff.txt improvements

[Ramkumar Ramachandra]

So, I decided to drop everything and just submit the first two parts.
Until we can decide what to do about the '<commit>..<commit>' and
'<commit>...<commit>' forms (new rev spec? overload existing rev spec
to change the meaning? deprecate without a viable alternative?), let's
leave them as it is.

Nothing to resubmit.  Junio: can you just queue the first two parts?

Thanks.

Ramkumar Ramachandra (2):
  git-diff.txt: reorder the <commit> <commit> form
  git-diff.txt: strip the leading "--" from options template

 Documentation/git-diff.txt | 22 +++++++++++-----------
 1 file changed, 11 insertions(+), 11 deletions(-)

Re: [PATCH 0/5] Documentation/git-diff.txt improvements

[Junio C Hamano]

Ramkumar Ramachandra <artagnon@gmail.com> writes:

> So, I decided to drop everything and just submit the first two parts.
> Until we can decide what to do about the '<commit>..<commit>' and
> '<commit>...<commit>' forms (new rev spec? overload existing rev spec
> to change the meaning? deprecate without a viable alternative?), let's
> leave them as it is.

I had an impression that many of them, with the suggested fix-ups,
were improvements, with or without the A~B discussion.  It seems
that the entire review wasted many braincycles if you are to drop
the ball at this point, which is very dissapointing.

> Nothing to resubmit.  Junio: can you just queue the first two parts?

I do not appreciate "Nothing to resubmit".  [1/5] needs fixes to the
issue Jonathan pointed out, doesn't it?

Re: [PATCH 0/5] Documentation/git-diff.txt improvements

[Ramkumar Ramachandra]

Junio C Hamano wrote:
> I had an impression that many of them, with the suggested fix-ups,
> were improvements, with or without the A~B discussion.  It seems
> that the entire review wasted many braincycles if you are to drop
> the ball at this point, which is very dissapointing.

No, I don't meant to drop the ball at all.  I'm _very_ annoyed by this
inconsistency, and have a burning desire to fix it.  It's just that I
don't want to fixup something now and regret it later, so I'm putting
it off until the A~B issue gets resolved.  In the meantime, the first
two patches are fine and can be applied as is.

> I do not appreciate "Nothing to resubmit".  [1/5] needs fixes to the
> issue Jonathan pointed out, doesn't it?

Oh, I thought it was fairly trivial and that you'd fix it up locally.
Would you like me to resend with the fixup?

Re: [PATCH 0/5] Documentation/git-diff.txt improvements

[Junio C Hamano]

Ramkumar Ramachandra <artagnon@gmail.com> writes:

> Junio C Hamano wrote:
>> I had an impression that many of them, with the suggested fix-ups,
>> were improvements, with or without the A~B discussion.  It seems
>> that the entire review wasted many braincycles if you are to drop
>> the ball at this point, which is very dissapointing.
>
> No, I don't meant to drop the ball at all.  I'm _very_ annoyed by this
> inconsistency, and have a burning desire to fix it.  It's just that I
> don't want to fixup something now and regret it later, so I'm putting
> it off until the A~B issue gets resolved.  In the meantime, the first
> two patches are fine and can be applied as is.

As far as I am concerned, there is no A~B issue.

Please don't let it take good changes hostage.

Re: [PATCH 0/5] Documentation/git-diff.txt improvements

[Ramkumar Ramachandra]

Junio C Hamano wrote:
> As far as I am concerned, there is no A~B issue.

You don't have to remind me about how hostile you are towards new
proposals.  I already know.

> Please don't let it take good changes hostage.

Those are my changes, and I will not check them in until I'm happy
with them.  How did this turn into a hostage situation?  I've already
signed off on my changes: you or anyone else has every right to pick
them up and merge them (even if I don't like them).

Re: [PATCH 0/5] Documentation/git-diff.txt improvements

[Junio C Hamano]

Ramkumar Ramachandra <artagnon@gmail.com> writes:

> Junio C Hamano wrote:
>> As far as I am concerned, there is no A~B issue.
>
> You don't have to remind me about how hostile you are towards new
> proposals.  I already know.

That's not "hostile" or "new".  I only express strong doubt against
"ideas" (or non-"ideas") that do not make sense to me.

They may be valid and merely are not well explained, or they may be
total nonsense.  The latter does not deserve time from people on
this list, but the former needs to be better explained.