git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [PATCH] Improve the "diff --git" format documentation
@ 2010-10-06 16:23 Andreas Gruenbacher
  2010-10-06 17:22 ` Junio C Hamano
  0 siblings, 1 reply; 10+ messages in thread
From: Andreas Gruenbacher @ 2010-10-06 16:23 UTC (permalink / raw)
  To: git

Hello,

here is a small improvement to the documentation of git's extended diff
format.  Can this please be included?

Thanks,
Andreas

Signed-off-by: Andreas Gruenbacher <agruen@suse.de>
---
 Documentation/diff-generate-patch.txt |   23 ++++++++++++++++++++++-
 1 files changed, 22 insertions(+), 1 deletions(-)

diff --git a/Documentation/diff-generate-patch.txt b/Documentation/diff-
generate-patch.txt
index 8f9a241..05f2164 100644
--- a/Documentation/diff-generate-patch.txt
+++ b/Documentation/diff-generate-patch.txt
@@ -18,7 +18,8 @@ diff format.
 +
 The `a/` and `b/` filenames are the same unless rename/copy is
 involved.  Especially, even for a creation or a deletion,
-`/dev/null` is _not_ used in place of `a/` or `b/` filenames.
+`/dev/null` is _not_ used in place of `a/` or `b/` filenames in the
+`diff --git` line.
 +
 When rename/copy is involved, `file1` and `file2` show the
 name of the source file of the rename/copy and the name of
@@ -38,11 +39,31 @@ the file that rename/copy produces, respectively.
        dissimilarity index <number>
        index <hash>..<hash> <mode>
 
+    Path names in extended header lines do not include the `a/` and `b/`
+    prefixes.  The index header includes the <mode> only if the file
+    mode does not change; otherwise, explicit mode headers are included.
+
 3.  TAB, LF, double quote and backslash characters in pathnames
     are represented as `\t`, `\n`, `\"` and `\\`, respectively.
     If there is need for such substitution then the whole
     pathname is put in double quotes.
 
+    Space characters are not quoted and so when files are copied or
+    renamed, the file names in the "diff --git" line can be
+    ambiguous.
+
+4.  All the `a/` files refer to files before the commit, and all the `b/`
+    files refer to files after the commit; it is incorrect to apply the
+    changes to each file sequentially.  For example, this patch will
+    swap a and b:
+
+      diff --git a/a b/b
+      rename from a
+      rename to b
+      diff --git a/b b/a
+      rename from b
+      rename to a
+
 The similarity index is the percentage of unchanged lines, and
 the dissimilarity index is the percentage of changed lines.  It
 is a rounded down integer, followed by a percent sign.  The

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] Improve the "diff --git" format documentation
  2010-10-06 16:23 [PATCH] Improve the "diff --git" format documentation Andreas Gruenbacher
@ 2010-10-06 17:22 ` Junio C Hamano
  2010-10-06 23:03   ` Andreas Gruenbacher
  0 siblings, 1 reply; 10+ messages in thread
From: Junio C Hamano @ 2010-10-06 17:22 UTC (permalink / raw)
  To: Andreas Gruenbacher; +Cc: git

Andreas Gruenbacher <agruen@suse.de> writes:

> Hello,
>
> here is a small improvement to the documentation of git's extended diff
> format.  Can this please be included?
>
> Thanks,
> Andreas
>
> Signed-off-by: Andreas Gruenbacher <agruen@suse.de>
> ---

I thought you have been here long enough to send a patch with some more
meaningful log message than that.  Could you objectively describe in what
way is it an "improvement" on the subject line?

>  Documentation/diff-generate-patch.txt |   23 ++++++++++++++++++++++-
>  1 files changed, 22 insertions(+), 1 deletions(-)
>
> diff --git a/Documentation/diff-generate-patch.txt b/Documentation/diff-
> generate-patch.txt
> index 8f9a241..05f2164 100644
> --- a/Documentation/diff-generate-patch.txt
> +++ b/Documentation/diff-generate-patch.txt
> @@ -18,7 +18,8 @@ diff format.
>  +
>  The `a/` and `b/` filenames are the same unless rename/copy is
>  involved.  Especially, even for a creation or a deletion,
> -`/dev/null` is _not_ used in place of `a/` or `b/` filenames.
> +`/dev/null` is _not_ used in place of `a/` or `b/` filenames in the
> +`diff --git` line.

With a bit more context, the original reads like this:

    What the -p option produces is slightly different from the traditional
    diff format.

    1.   It is preceded with a "git diff" header, that looks like
         this:

           diff --git a/file1 b/file2
    +
    The `a/` and `b/` filenames are the same unless rename/copy is
    involved.  Especially, even for a creation or a deletion,
    `/dev/null` is _not_ used in place of `a/` or `b/` filenames.

I think the first sentence makes it clear that this section is about '"git
diff" header', without repeating it like your patch does.

> @@ -38,11 +39,31 @@ the file that rename/copy produces, respectively.
>         dissimilarity index <number>
>         index <hash>..<hash> <mode>
>  
> +    Path names in extended header lines do not include the `a/` and `b/`
> +    prefixes.  The index header includes the <mode> only if the file
> +    mode does not change; otherwise, explicit mode headers are included.
> +

Have you looked at generated output in man and html formats?  I suspect
that it needs some asciidoc formatting magic, similar to what we already
have for the first section (namely, no indent, but the new block is marked
as a continuation with a lone plus sign at the beginning, instead of being
separated by a blank line).

>  3.  TAB, LF, double quote and backslash characters in pathnames
>      are represented as `\t`, `\n`, `\"` and `\\`, respectively.
>      If there is need for such substitution then the whole
>      pathname is put in double quotes.
>  
> +    Space characters are not quoted and so when files are copied or
> +    renamed, the file names in the "diff --git" line can be
> +    ambiguous.

Why do you even need to say that, especially after you made it clear that
rename information is available in the extended header section in an
unambiguous form?

> +4.  All the `a/` files refer to files before the commit, and all the `b/`
> +    files refer to files after the commit; it is incorrect to apply the
> +    changes to each file sequentially.  For example, this patch will
> +    swap a and b:
> +
> +      diff --git a/a b/b
> +      rename from a
> +      rename to b
> +      diff --git a/b b/a
> +      rename from b
> +      rename to a
> +
>  The similarity index is the percentage of unchanged lines, and
>  the dissimilarity index is the percentage of changed lines.  It
>  is a rounded down integer, followed by a percent sign.  The

The new section is a worthwhile addition; I however think this addition
makes the description of the similarity/dissimilarity indices further from
the section it relates to (it logically is part of section 2), so perhaps
it should move 3. down and add 4. while at it.

Thanks.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Improve the "diff --git" format documentation
  2010-10-06 17:22 ` Junio C Hamano
@ 2010-10-06 23:03   ` Andreas Gruenbacher
  2010-10-11 13:14     ` Andreas Gruenbacher
  2010-10-14  1:55     ` Junio C Hamano
  0 siblings, 2 replies; 10+ messages in thread
From: Andreas Gruenbacher @ 2010-10-06 23:03 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

On Wednesday 06 October 2010 19:22:11 Junio C Hamano wrote:
> [...] Could you objectively describe in what way is it an "improvement" on
> the subject line?

Okay.

> >  Documentation/diff-generate-patch.txt |   23 ++++++++++++++++++++++-
> >  1 files changed, 22 insertions(+), 1 deletions(-)
> > 
> > diff --git a/Documentation/diff-generate-patch.txt b/Documentation/diff-
> > generate-patch.txt
> > index 8f9a241..05f2164 100644
> > --- a/Documentation/diff-generate-patch.txt
> > +++ b/Documentation/diff-generate-patch.txt
> > @@ -18,7 +18,8 @@ diff format.
> > 
> >  +
> >  The `a/` and `b/` filenames are the same unless rename/copy is
> >  involved.  Especially, even for a creation or a deletion,
> > 
> > -`/dev/null` is _not_ used in place of `a/` or `b/` filenames.
> > +`/dev/null` is _not_ used in place of `a/` or `b/` filenames in the
> > +`diff --git` line.
> 
> With a bit more context, the original reads like this:
> 
>     What the -p option produces is slightly different from the traditional
>     diff format.
> 
>     1.   It is preceded with a "git diff" header, that looks like
>          this:
> 
>            diff --git a/file1 b/file2
>     +
>     The `a/` and `b/` filenames are the same unless rename/copy is
>     involved.  Especially, even for a creation or a deletion,
>     `/dev/null` is _not_ used in place of `a/` or `b/` filenames.
> 
> I think the first sentence makes it clear that this section is about '"git
> diff" header', without repeating it like your patch does.

The text made me wonder how git handles file names in the unified diff
headers, which is not mentioned in this section.  Also, the way how /dev/null
is used in unified diff headers may not be known to some users.  I have tried
to clarify both in the attached version.

> > @@ -38,11 +39,31 @@ the file that rename/copy produces, respectively.
> > 
> >         dissimilarity index <number>
> >         index <hash>..<hash> <mode>
> > 
> > +    Path names in extended header lines do not include the `a/` and `b/`
> > +    prefixes.  The index header includes the <mode> only if the file
> > +    mode does not change; otherwise, explicit mode headers are included.
> > +
> 
> Have you looked at generated output in man and html formats?  I suspect
> that it needs some asciidoc formatting magic, similar to what we already
> have for the first section (namely, no indent, but the new block is marked
> as a continuation with a lone plus sign at the beginning, instead of being
> separated by a blank line).

Fixed, thanks.

> >  3.  TAB, LF, double quote and backslash characters in pathnames
> >  
> >      are represented as `\t`, `\n`, `\"` and `\\`, respectively.
> >      If there is need for such substitution then the whole
> >      pathname is put in double quotes.
> > 
> > +    Space characters are not quoted and so when files are copied or
> > +    renamed, the file names in the "diff --git" line can be
> > +    ambiguous.
> 
> Why do you even need to say that, especially after you made it clear that
> rename information is available in the extended header section in an
> unambiguous form?

This fact was *extremely* surprising to me (and I still think it is a
mistake; spaces should at least be quoted in the "git diff" line). 
But documenting it may at least save others this surprise.

I have tried to reword this; could you please check?

> > +4.  All the `a/` files refer to files before the commit, and all the
> > `b/` +    files refer to files after the commit; it is incorrect to
> > apply the +    changes to each file sequentially.  For example, this
> > patch will +    swap a and b:
> > +
> > +      diff --git a/a b/b
> > +      rename from a
> > +      rename to b
> > +      diff --git a/b b/a
> > +      rename from b
> > +      rename to a
> > +
> > 
> >  The similarity index is the percentage of unchanged lines, and
> >  the dissimilarity index is the percentage of changed lines.  It
> >  is a rounded down integer, followed by a percent sign.  The
> 
> The new section is a worthwhile addition; I however think this addition
> makes the description of the similarity/dissimilarity indices further from
> the section it relates to (it logically is part of section 2), so perhaps
> it should move 3. down and add 4. while at it.

Done.

Thanks,
Andreas

--

[PATCH] Clarify and extend the "git diff" format documentation

Make it more clear that the unified diff header lines *do* use /dev/null
for nonexisting files.

Move the similarity and dissimilarity index header description closer to
where those extended headers are described.

Describe and/or clarify the format used for file modes, pathnames, and
the index header.  Make it clear that spaces in pathnames are not
quoted.

Document that all "old" files refer to the state before applying the
*entire* output, and all "new" files refer to the state thereafter.

Signed-off-by: Andreas Gruenbacher <agruen@suse.de>
---
 Documentation/diff-generate-patch.txt |   44 +++++++++++++++++++++++++-------
 1 files changed, 34 insertions(+), 10 deletions(-)

diff --git a/Documentation/diff-generate-patch.txt b/Documentation/diff-generate-patch.txt
index 8f9a241..e5beef1 100644
--- a/Documentation/diff-generate-patch.txt
+++ b/Documentation/diff-generate-patch.txt
@@ -9,16 +9,16 @@ patch file.  You can customize the creation of such patches via the
 GIT_EXTERNAL_DIFF and the GIT_DIFF_OPTS environment variables.
 
 What the -p option produces is slightly different from the traditional
-diff format.
+diff format:
 
-1.   It is preceded with a "git diff" header, that looks like
-     this:
+1.   It is preceded with a "git diff" header that looks like this:
 
        diff --git a/file1 b/file2
 +
 The `a/` and `b/` filenames are the same unless rename/copy is
 involved.  Especially, even for a creation or a deletion,
-`/dev/null` is _not_ used in place of `a/` or `b/` filenames.
+`/dev/null` is _not_ used in place of the `a/` or `b/` filenames
+for nonexisting files (unlike in the unified diff headers).
 +
 When rename/copy is involved, `file1` and `file2` show the
 name of the source file of the rename/copy and the name of
@@ -37,18 +37,42 @@ the file that rename/copy produces, respectively.
        similarity index <number>
        dissimilarity index <number>
        index <hash>..<hash> <mode>
-
-3.  TAB, LF, double quote and backslash characters in pathnames
-    are represented as `\t`, `\n`, `\"` and `\\`, respectively.
-    If there is need for such substitution then the whole
-    pathname is put in double quotes.
-
++
+File modes are printed as 6-digit octal numbers including the file type
+and file permission bits.
++
+Path names in extended headers do not include the `a/` and `b/` prefixes.
++
 The similarity index is the percentage of unchanged lines, and
 the dissimilarity index is the percentage of changed lines.  It
 is a rounded down integer, followed by a percent sign.  The
 similarity index value of 100% is thus reserved for two equal
 files, while 100% dissimilarity means that no line from the old
 file made it into the new one.
++
+The index line includes the SHA-1 checksum before and after the change.
+The <mode> is included if the file mode does not change; otherwise,
+separate lines indicate the old and the new mode.
+
+3.  TAB, LF, double quote and backslash characters in pathnames
+    are represented as `\t`, `\n`, `\"` and `\\`, respectively.
+    If there is need for such substitution then the whole
+    pathname is put in double quotes.
++
+Space characters in pathnames are _not_ quoted, neither in the "git
+diff" header nor in extended header lines.
+
+4.  All the `file1` files in the output refer to files before the
+    commit, and all the `file2` files refer to files after the commit.
+    It is incorrect to apply each change to each file sequentially.  For
+    example, this patch will swap a and b:
+
+      diff --git a/a b/b
+      rename from a
+      rename to b
+      diff --git a/b b/a
+      rename from b
+      rename to a
 
 
 combined diff format
-- 
1.7.3.1.50.g1e633.dirty

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] Improve the "diff --git" format documentation
  2010-10-06 23:03   ` Andreas Gruenbacher
@ 2010-10-11 13:14     ` Andreas Gruenbacher
  2010-10-14  1:55       ` Junio C Hamano
  2010-10-14  1:55     ` Junio C Hamano
  1 sibling, 1 reply; 10+ messages in thread
From: Andreas Gruenbacher @ 2010-10-11 13:14 UTC (permalink / raw)
  To: git; +Cc: Junio C Hamano

Hello,

what's the verdict on my second attempt?

Thanks,
Andreas

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Improve the "diff --git" format documentation
  2010-10-11 13:14     ` Andreas Gruenbacher
@ 2010-10-14  1:55       ` Junio C Hamano
  0 siblings, 0 replies; 10+ messages in thread
From: Junio C Hamano @ 2010-10-14  1:55 UTC (permalink / raw)
  To: Andreas Gruenbacher; +Cc: git

Andreas Gruenbacher <agruen@suse.de> writes:

> what's the verdict on my second attempt?

Sorry, but I didn't notice the "second attempt" that was buried in a
response message.

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Improve the "diff --git" format documentation
  2010-10-06 23:03   ` Andreas Gruenbacher
  2010-10-11 13:14     ` Andreas Gruenbacher
@ 2010-10-14  1:55     ` Junio C Hamano
  2010-10-14 10:53       ` Andreas Gruenbacher
  2010-10-14 12:39       ` Andreas Gruenbacher
  1 sibling, 2 replies; 10+ messages in thread
From: Junio C Hamano @ 2010-10-14  1:55 UTC (permalink / raw)
  To: Andreas Gruenbacher; +Cc: git

Andreas Gruenbacher <agruen@suse.de> writes:

>  The `a/` and `b/` filenames are the same unless rename/copy is
>  involved.  Especially, even for a creation or a deletion,
> -`/dev/null` is _not_ used in place of `a/` or `b/` filenames.
> +`/dev/null` is _not_ used in place of the `a/` or `b/` filenames
> +for nonexisting files (unlike in the unified diff headers).

The description in the parentheses is wrong, unless you qualify whose
"unified diff headers" you are talking about.  For example:

 http://www.opengroup.org/onlinepubs/9699919799/utilities/diff.html#tag_20_34_10_07

does not mention anything about file creation/deletion events.  Perhaps
you are referring to cvs or svn output, but I think we can safely drop the
parenthesized part without losing clarity.

> @@ -37,18 +37,42 @@ the file that rename/copy produces, respectively.
>         similarity index <number>
>         dissimilarity index <number>
>         index <hash>..<hash> <mode>
> -
> -3.  TAB, LF, double quote and backslash characters in pathnames
> -    are represented as `\t`, `\n`, `\"` and `\\`, respectively.
> -    If there is need for such substitution then the whole
> -    pathname is put in double quotes.
> -
> ++
> +File modes are printed as 6-digit octal numbers including the file type
> +and file permission bits.
> ++
> +Path names in extended headers do not include the `a/` and `b/` prefixes.
> ++
>  The similarity index is the percentage of unchanged lines, and
>  the dissimilarity index is the percentage of changed lines.  It
>  is a rounded down integer, followed by a percent sign.  The
>  similarity index value of 100% is thus reserved for two equal
>  files, while 100% dissimilarity means that no line from the old
>  file made it into the new one.
> ++
> +The index line includes the SHA-1 checksum before and after the change.
> +The <mode> is included if the file mode does not change; otherwise,
> +separate lines indicate the old and the new mode.
> +
> +3.  TAB, LF, double quote and backslash characters in pathnames
> +    are represented as `\t`, `\n`, `\"` and `\\`, respectively.
> +    If there is need for such substitution then the whole
> +    pathname is put in double quotes.
> ++
> +Space characters in pathnames are _not_ quoted, neither in the "git
> +diff" header nor in extended header lines.

I am not sure if there is a particular need to spend an extra paragraph to
special case the SP [*1*].  On the other hand, we quote bytes with
high-bit set in \octal [*2*], unless core.quotepath is set to false, too,
which should probably be described here.


[References]

*1* 28fba29 (Do not quote SP., 2005-10-17)
*2* http://marc.info/?l=git&m=112927316408690&w=2

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Improve the "diff --git" format documentation
  2010-10-14  1:55     ` Junio C Hamano
@ 2010-10-14 10:53       ` Andreas Gruenbacher
  2010-10-14 12:39       ` Andreas Gruenbacher
  1 sibling, 0 replies; 10+ messages in thread
From: Andreas Gruenbacher @ 2010-10-14 10:53 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

On Thursday 14 October 2010 03:55:48 Junio C Hamano wrote:
> [some more objections]

Okay, here are the changes we seem to be able to agree on.  Let's address the 
rest separately.

Andreas

--

[PATCH] Clarify and extend the "git diff" format documentation

Move the similarity and dissimilarity index header description closer to
where those extended headers are described.

Describe and/or clarify the format used for file modes, pathnames, and
the index header.

Document that all "old" files refer to the state before applying the
*entire* output, and all "new" files refer to the state thereafter.

Signed-off-by: Andreas Gruenbacher <agruen@suse.de>
---
 Documentation/diff-generate-patch.txt |   40 ++++++++++++++++++++++++--------
 1 files changed, 30 insertions(+), 10 deletions(-)

diff --git a/Documentation/diff-generate-patch.txt b/Documentation/diff-
generate-patch.txt
index 8f9a241..3ac2bea 100644
--- a/Documentation/diff-generate-patch.txt
+++ b/Documentation/diff-generate-patch.txt
@@ -9,16 +9,15 @@ patch file.  You can customize the creation of such patches 
via the
 GIT_EXTERNAL_DIFF and the GIT_DIFF_OPTS environment variables.
 
 What the -p option produces is slightly different from the traditional
-diff format.
+diff format:
 
-1.   It is preceded with a "git diff" header, that looks like
-     this:
+1.   It is preceded with a "git diff" header that looks like this:
 
        diff --git a/file1 b/file2
 +
 The `a/` and `b/` filenames are the same unless rename/copy is
 involved.  Especially, even for a creation or a deletion,
-`/dev/null` is _not_ used in place of `a/` or `b/` filenames.
+`/dev/null` is _not_ used in place of the `a/` or `b/` filenames.
 +
 When rename/copy is involved, `file1` and `file2` show the
 name of the source file of the rename/copy and the name of
@@ -37,18 +36,39 @@ the file that rename/copy produces, respectively.
        similarity index <number>
        dissimilarity index <number>
        index <hash>..<hash> <mode>
-
-3.  TAB, LF, double quote and backslash characters in pathnames
-    are represented as `\t`, `\n`, `\"` and `\\`, respectively.
-    If there is need for such substitution then the whole
-    pathname is put in double quotes.
-
++
+File modes are printed as 6-digit octal numbers including the file type
+and file permission bits.
++
+Path names in extended headers do not include the `a/` and `b/` prefixes.
++
 The similarity index is the percentage of unchanged lines, and
 the dissimilarity index is the percentage of changed lines.  It
 is a rounded down integer, followed by a percent sign.  The
 similarity index value of 100% is thus reserved for two equal
 files, while 100% dissimilarity means that no line from the old
 file made it into the new one.
++
+The index line includes the SHA-1 checksum before and after the change.
+The <mode> is included if the file mode does not change; otherwise,
+separate lines indicate the old and the new mode.
+
+3.  TAB, LF, double quote and backslash characters in pathnames
+    are represented as `\t`, `\n`, `\"` and `\\`, respectively.
+    If there is need for such substitution then the whole
+    pathname is put in double quotes.
+
+4.  All the `file1` files in the output refer to files before the
+    commit, and all the `file2` files refer to files after the commit.
+    It is incorrect to apply each change to each file sequentially.  For
+    example, this patch will swap a and b:
+
+      diff --git a/a b/b
+      rename from a
+      rename to b
+      diff --git a/b b/a
+      rename from b
+      rename to a
 
 
 combined diff format

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] Improve the "diff --git" format documentation
  2010-10-14  1:55     ` Junio C Hamano
  2010-10-14 10:53       ` Andreas Gruenbacher
@ 2010-10-14 12:39       ` Andreas Gruenbacher
  2010-10-14 16:16         ` Jonathan Nieder
  1 sibling, 1 reply; 10+ messages in thread
From: Andreas Gruenbacher @ 2010-10-14 12:39 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git

On Thursday 14 October 2010 03:55:48 Junio C Hamano wrote:
> Andreas Gruenbacher <agruen@suse.de> writes:
> 
> >  The `a/` and `b/` filenames are the same unless rename/copy is
> >  involved.  Especially, even for a creation or a deletion,
> > -`/dev/null` is _not_ used in place of `a/` or `b/` filenames.
> > +`/dev/null` is _not_ used in place of the `a/` or `b/` filenames
> > +for nonexisting files (unlike in the unified diff headers).
> 
> The description in the parentheses is wrong, unless you qualify whose
> "unified diff headers" you are talking about.

I was referring to the unified diff headers that git emits, not any other
utility's output.

POSIX does not say anything about nonexisting files, and GNU diff with -rN uses an
Epoch timestamp instead of /dev/null to distinguish between missing and empty
files:

	http://www.gnu.org/software/diffutils/manual/html_node/Creating-and-Removing.html

> For example:
> 
>  http://www.opengroup.org/onlinepubs/9699919799/utilities/diff.html#tag_20_34_10_07
> 
> does not mention anything about file creation/deletion events.

Fine, let's make this clear in the "combined diff" format description too
then.

> Perhaps you are referring to cvs or svn output, but I think we can safely
> drop the parenthesized part without losing clarity.

I still believe that the documentation should make it very clear how it
handles created and deleted files; it really is not obvious to everyone.

Can you live with the attached patch?

Thanks,
Andreas

--

[PATCH] Clarify how /dev/null is used in diffs

Say where the unified diff header comes in the "git diff" format, and
make it clear that /dev/null is used there.

In the description of the "combined diff" format, do not claim that
traditional unified diffs use /dev/null to signal created or deleted files.

Signed-off-by: Andreas Gruenbacher <agruen@suse.de>
---
 Documentation/diff-generate-patch.txt |   26 ++++++++++++++++++--------
 1 files changed, 18 insertions(+), 8 deletions(-)

diff --git a/Documentation/diff-generate-patch.txt b/Documentation/diff-generate-patch.txt
index 3ac2bea..21c4923 100644
--- a/Documentation/diff-generate-patch.txt
+++ b/Documentation/diff-generate-patch.txt
@@ -53,12 +53,22 @@ The index line includes the SHA-1 checksum before and after the change.
 The <mode> is included if the file mode does not change; otherwise,
 separate lines indicate the old and the new mode.
 
-3.  TAB, LF, double quote and backslash characters in pathnames
+3.  It is followed by a 'unified' diff which starts with a two-line
+    from-file/to-file header:
+
+      --- a/file1
+      +++ b/file2
++
+This header is omitted if the contents of `file1` and `file2` are identical.
+To signal created or deleted files, `/dev/null` is used instead of `a/file1`
+or `b/file2`.
+
+4.  TAB, LF, double quote and backslash characters in pathnames
     are represented as `\t`, `\n`, `\"` and `\\`, respectively.
     If there is need for such substitution then the whole
     pathname is put in double quotes.
 
-4.  All the `file1` files in the output refer to files before the
+5.  All the `file1` files in the output refer to files before the
     commit, and all the `file2` files refer to files after the commit.
     It is incorrect to apply each change to each file sequentially.  For
     example, this patch will swap a and b:
@@ -133,14 +143,14 @@ information about detected contents movement (renames and
 copying detection) are designed to work with diff of two
 <tree-ish> and are not used by combined diff format.
 
-3.   It is followed by two-line from-file/to-file header
+3.   It is followed by a two-line from-file/to-file header similar to the
+     traditional 'unified' diff format header:
 
-       --- a/file
-       +++ b/file
+       --- a/file1
+       +++ b/file2
 +
-Similar to two-line header for traditional 'unified' diff
-format, `/dev/null` is used to signal created or deleted
-files.
+To signal created or deleted files, `/dev/null` is used instead of `a/file1`
+or `b/file2`.
 
 4.   Chunk header format is modified to prevent people from
      accidentally feeding it to `patch -p1`. Combined diff format

^ permalink raw reply related	[flat|nested] 10+ messages in thread

* Re: [PATCH] Improve the "diff --git" format documentation
  2010-10-14 12:39       ` Andreas Gruenbacher
@ 2010-10-14 16:16         ` Jonathan Nieder
  2010-10-17  4:43           ` Junio C Hamano
  0 siblings, 1 reply; 10+ messages in thread
From: Jonathan Nieder @ 2010-10-14 16:16 UTC (permalink / raw)
  To: Andreas Gruenbacher; +Cc: Junio C Hamano, git

Andreas Gruenbacher wrote:

> I still believe that the documentation should make it very clear how it
> handles created and deleted files;

Yes - thanks for doing this.

> --- a/Documentation/diff-generate-patch.txt
> +++ b/Documentation/diff-generate-patch.txt
> @@ -53,12 +53,22 @@ The index line includes the SHA-1 checksum before and after the change.
>  The <mode> is included if the file mode does not change; otherwise,
>  separate lines indicate the old and the new mode.
>  
> -3.  TAB, LF, double quote and backslash characters in pathnames
> +3.  It is followed by a 'unified' diff which starts with a two-line

So afterwards the section would say:

	What the -p option produces is slightly different from the traditional
	diff format.

	1.   It is preceded with a "git diff" header, that looks like
	     this:

	       diff --git a/file1 b/file2
	[...]

	2.   It is followed by one or more extended header lines:

	       old mode <mode>
	       new mode <mode>
	[...]

	3.   It is followed by a 'unified' diff which starts with a two-line
	[...]

At some point, the reader starts to wonder what "it" is. :)   (2)
already has this problem, since the extended header lines actually
precede the traditional diff rather than following it.

How about:

	What the -p option produces is slightly different[...]

	1. It is preceded with a "git diff" header[...]
	2. Next comes one or more extended header lines[...]
	3. The from-file/to-file header that follows uses filenames
	of the form a/file1 and b/file2 (where "a/" and "b/" can be
	replaced with some other string or removed depending on
	options used):

		--- a/file1
		+++ b/file2

	This header is omitted if[...]
	4. TAB, LF, double quote, and [...]

Jonathan

^ permalink raw reply	[flat|nested] 10+ messages in thread

* Re: [PATCH] Improve the "diff --git" format documentation
  2010-10-14 16:16         ` Jonathan Nieder
@ 2010-10-17  4:43           ` Junio C Hamano
  0 siblings, 0 replies; 10+ messages in thread
From: Junio C Hamano @ 2010-10-17  4:43 UTC (permalink / raw)
  To: Jonathan Nieder; +Cc: Andreas Gruenbacher, git

Jonathan Nieder <jrnieder@gmail.com> writes:

> ...
> At some point, the reader starts to wonder what "it" is. :)   (2)
> already has this problem, since the extended header lines actually
> precede the traditional diff rather than following it.
>
> How about:
> 
> 	What the -p option produces is slightly different[...]
>
> 	1. It is preceded with a "git diff" header[...]
> 	2. Next comes one or more extended header lines[...]
> 	3. The from-file/to-file header that follows uses filenames
> 	of the form a/file1 and b/file2 (where "a/" and "b/" can be
> 	replaced with some other string or removed depending on
> 	options used):
>
> 		--- a/file1
> 		+++ b/file2
>
> 	This header is omitted if[...]
> 	4. TAB, LF, double quote, and [...]

Quite sensible point to raise, which I completely missed.  Thanks.

^ permalink raw reply	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2010-10-17  4:43 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-10-06 16:23 [PATCH] Improve the "diff --git" format documentation Andreas Gruenbacher
2010-10-06 17:22 ` Junio C Hamano
2010-10-06 23:03   ` Andreas Gruenbacher
2010-10-11 13:14     ` Andreas Gruenbacher
2010-10-14  1:55       ` Junio C Hamano
2010-10-14  1:55     ` Junio C Hamano
2010-10-14 10:53       ` Andreas Gruenbacher
2010-10-14 12:39       ` Andreas Gruenbacher
2010-10-14 16:16         ` Jonathan Nieder
2010-10-17  4:43           ` Junio C Hamano

Code repositories for project(s) associated with this public inbox

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

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