[PATCH 2/4] Tweak diff output further to make it a bit less distracting.

[PATCH 2/4] Tweak diff output further to make it a bit less distracting.

[Junio C Hamano]

Adds an newline between each diff.  Also change "#mode : "
string, which was misleading in that we are not showing just
mode when we talk about a file changing into a symlink.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---

diff.c                 |   18 ++++++++++--------
t/t4000-diff-format.sh |    6 ++++--
2 files changed, 14 insertions(+), 10 deletions(-)

--- a/diff.c
+++ b/diff.c
@@ -83,7 +83,7 @@
 			 struct diff_tempfile *temp)
 {
 	int i, next_at;
-	const char *git_prefix = "# mode: ";
+	const char *git_prefix = "\n@. ";
 	const char *diff_cmd = "diff -L'%s%s' -L'%s%s'";
 	const char *diff_arg  = "'%s' '%s'||:"; /* "||:" is to return 0 */
 	const char *input_name_sq[2];
@@ -128,15 +128,17 @@
 	else if (!path1[1][0])
 		printf("%s%s . %s\n", git_prefix, temp[0].mode, name);
 	else {
-		if (strcmp(temp[0].mode, temp[1].mode))
+		if (strcmp(temp[0].mode, temp[1].mode)) {
 			printf("%s%s %s %s\n", git_prefix,
 			       temp[0].mode, temp[1].mode, name);
-
-		if (strncmp(temp[0].mode, temp[1].mode, 3))
-			/* we do not run diff between different kind
-			 * of objects.
-			 */
-			exit(0);
+			if (strncmp(temp[0].mode, temp[1].mode, 3))
+				/* we do not run diff between different kind
+				 * of objects.
+				 */
+				exit(0);
+		}
+		else
+			putchar('\n');
 	}
 	fflush(NULL);
 	execlp("/bin/sh","sh", "-c", cmd, NULL);
--- a/t/t4000-diff-format.sh
+++ b/t/t4000-diff-format.sh
@@ -26,7 +26,8 @@
     'git-diff-files -p after editing work tree.' \
     'git-diff-files -p >current'
 cat >expected <<\EOF
-# mode: 100644 100755 path0
+
+@. 100644 100755 path0
 --- a/path0
 +++ b/path0
 @@ -1,3 +1,3 @@
@@ -34,7 +35,8 @@
  Line 2
 -line 3
 +Line 3
-# mode: 100755 . path1
+
+@. 100755 . path1
 --- a/path1
 +++ /dev/null
 @@ -1,3 +0,0 @@

Re: [PATCH 2/4] Tweak diff output further to make it a bit less distracting.

[Petr Baudis]

Dear diary, on Sun, May 15, 2005 at 11:19:50PM CEST, I got a letter
where Junio C Hamano <junkio@cox.net> told me that...
> Adds an newline between each diff.  Also change "#mode : "
> string, which was misleading in that we are not showing just
> mode when we talk about a file changing into a symlink.
> 
> Signed-off-by: Junio C Hamano <junkio@cox.net>

So, I've been looking at the output, and I have to admit that I'm still
not too happy with it (I know I'm horrible). It turned out to be rather
confusing, since there are normally no blank lines in the middle of the
diffs, so it looked as the blank lines were actually part of the diffed
files.

What about just throwing away the newlines and just passing '@.'?

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
C++: an octopus made by nailing extra legs onto a dog. -- Steve Taylor

Re: [PATCH 2/4] Tweak diff output further to make it a bit less distracting.

[Junio C Hamano]

>>>>> "PB" == Petr Baudis <pasky@ucw.cz> writes:

PB> What about just throwing away the newlines and just passing '@.'?

Let's just drop the patch altogether unless anybody else has
better justification and pressing needs.

The current one is tolerable, except I do not like the word
"mode" very much.

    --- a/frotz
    +++ b/frotz
    @@ xxx @@
    + asdfasdf
    # mode: 100644 100755 nitfol
    --- a/nitfol
    +++ b/nitfol
    @@ yyy @@
    - asdfasdf
    + asdfasdfasdf
    --- a/rezrov
    +++ b/rezrov
    @@ zzz @@
     ...

This is what we would have got with the patch, which as you say
gives an illusion as if there should exist an empty line in
"frotz" and "nitfol", after the lines the hunks are applied to.
I should not have pushed it to begin with.

    --- a/frotz
    +++ b/frotz
    @@ xxx @@
    + asdfasdf

    @. 100644 100755 nitfol
    --- a/nitfol
    +++ b/nitfol
    @@ yyy @@
    - asdfasdf
    + asdfasdfasdf

    --- a/rezrov
    +++ b/rezrov
    @@ zzz @@
     ...

Re: [PATCH 2/4] Tweak diff output further to make it a bit less distracting.

[Linus Torvalds]

On Mon, 16 May 2005, Junio C Hamano wrote:

>     # mode: 100644 100755 nitfol
>     --- a/nitfol
>     +++ b/nitfol

>     @. 100644 100755 nitfol
>     --- a/nitfol
>     +++ b/nitfol

I have to say, I muct prefer the first over the second.

I don't know why people think "line noise" means "computer readable".  To
me, "@." look slike line noise, and worse, it's clearly _less_
disambiguous than spelling out "mode". The fact that it (on purpose, I
assume) looks somewhat like the "@@" beginning of a patch hunk for the
_previous_ file makes it even worse.

Terseness is filne, and thus maybe it could be just

	# 100644 100755 nitfol

but on the other hand that doesn't really have any advantages either. 
Except being better than the inexplicable "@."

Me, I'd prefer making it clear whether the file is "new", "removed" or
"changed". That's really what matters, and at least the "new" case does
need the mode (while the "removed" case does not). So instead of talking
about "mode", which is largely irrelevant (the important thing is to
indicate whether it's new or old), we should talk about how the file has
changed.

There's another issue I noticed with the current default 'diff' output: I 
often (almost always) want to have a way to go to "next file", which in 
traditional diffs I just do with "/^diff" when paging with 'less'. The 
current one doesn't have a way to do that - searching for '^--- ' comes 
closest, but is ambiguous, since it might be a part of a _patch_ that 
contains a line that got removed and that started with "-- ".

So I'd actually prefer some output format that fixed that thing too, by 
having a header for each file.

Making the header be "diff a/file b/file" would make the thing look the 
same as a regular diff, and that also makes it disambiguous (and thus easy 
for machines to parse) to have a _second_ or third line for things like 
mode change information.

So my preferred format would actually be

	diff -git a/filename b/filename
	<optional extended header lines>
	--- a/filename
	+++ b/filename
	@@ -xx ....

where the "optional extended header lines" would be very plain, like

	old mode 100644
	new mode 100755

or

	new file mode 100644

or

	deleted file mode 100644

or something like that.

In fact anything _except_ for something that starts with "-" "+" " " or
"@"  which have special meanings inside diffs (whether you want the "mode"
for the deleted file case or not is up to you - it could be an added
sanity check that the diff actually matches, but on the other hand there's
really nothing you could do anyway except warn if the mode didn't match,
so..)

This makes it very easy to parse mechanically: look for a line that starts
with "diff -git " (which cannot be part of the "meat" of the patch), and
then you know that you've found the start of an extended patch.

Why the "-git"? A normal patch header already looks something like the 
following (head of the pre-git 2.6.12-rc1 patch):

	diff -Nru a/CREDITS b/CREDITS
	--- a/CREDITS   2005-03-17 17:35:10 -08:00
	+++ b/CREDITS   2005-03-17 17:35:10 -08:00
	@@ -34,8 +34,9 @@
	 E: airlied@linux.ie
	 W: http://www.csn.ul.ie/~airlied
	 D: NFS over TCP patches
	-S: University of Limerick
	-S: Ireland
	+D: in-kernel DRM Maintainer

so the "-git" thing there is equivalent to the "-Nru" part, telling how
the diff was generated, and being an added hint that we may have extended
headers before the actual patch (which wouldn't be sensible in a non-git
patch).

One final note: I actually think that "rename patches" make a ton of 
sense, even if git itself doesn't track renames. If we ever have a "smart 
diff" thing that can generate inter-file diffs, I'd like to eventually see

	diff -git a/kernel/sched.c b/kernel/sched.c.old
	rename kernel/sched.c kernel/sched.c.old
	old mode 100644
	new mode 100755
	--- a/kernel/sched.c
	+++ b/kernel/sched.c.old
	@@ -1,5 +1,5 @@
	 /*
	- *  kernel/sched.c
	+ *  kernel/sched.c.old
	  *
	  *  Kernel scheduler and related syscalls
	  *

Notice? We could have a mode change, a rename _and_ a content change, all
at the same time under the same header. That's obviously a totally idiotic
example, but the point is that if we have a nice "extended diff header"
setup, the format is very easily able to accomodate things like this.

And it's both human-readable _and_ automatically parseable. And my old 
"/^diff " thing would still work, and still find each new entry, so I'd 
not have to teach my old fingers new tricks.

(This is, btw, the reason for the format of "git-diff-tree -v --stdin", in 
case anybody wondered. Take a look, and notice how piping the output to 
"less" and then doing "/^diff-tree " gives the expected results. Same 
deal. Human-readable _and_ machine-parseable at the same time!).

		Linus

Re: [PATCH 2/4] Tweak diff output further to make it a bit less distracting.

[Junio C Hamano]

>>>>> "LT" == Linus Torvalds <torvalds@osdl.org> writes:

LT> I have to say, I muct prefer the first over the second.

Likewise.

LT> ... (whether you want the "mode"
LT> for the deleted file case or not is up to you - it could be an added
LT> sanity check that the diff actually matches, but on the other hand there's
LT> really nothing you could do anyway except warn if the mode didn't match,
LT> so..)

Applying patch in reverse comes to mind...

I'd agree what you said about "diff -git" in the rest of your
message makes the most sense.

Re: [PATCH 2/4] Tweak diff output further to make it a bit less distracting.

[Matthias Urlichs]

Hi, Junio C Hamano wrote:
> 
> I'd agree what you said about "diff -git" in the rest of your
> message makes the most sense.
> 
... except, please use "diff --git".

-- 
Matthias Urlichs   |   {M:U} IT Design @ m-u-it.de   |  smurf@smurf.noris.de

Re: [PATCH 2/4] Tweak diff output further to make it a bit less distracting.

[Linus Torvalds]

On Wed, 18 May 2005, Matthias Urlichs wrote:

> Hi, Junio C Hamano wrote:
> > 
> > I'd agree what you said about "diff -git" in the rest of your
> > message makes the most sense.
> > 
> ... except, please use "diff --git".

Yes, that makes sense. It's not three flags "g" "i" and "t", it's the 
"git" flag.

		Linus

Re: [PATCH 2/4] Tweak diff output further to make it a bit less distracting.

[Matthias Urlichs]

Hi,

Linus Torvalds:
>  It's not three flags "g" "i" and "t",

Or the (hitherto nonexisting, at least in GNU diff) '-g' flag with an
"it" argument. ;-)

-- 
Matthias Urlichs   |   {M:U} IT Design @ m-u-it.de   |  smurf@smurf.noris.de

[PATCH] Fix diff output take #4.

[Junio C Hamano]

>>>>> "LT" == Linus Torvalds <torvalds@osdl.org> writes:

LT> Yes, that makes sense. It's not three flags "g" "i" and "t", it's the 
LT> "git" flag.

Concurred.  This is against the tip of your tree.  Pasky already
has a version with '-git' in his tree but I trust he can deal
with that single byte change locally.

------------
[PATCH] Fix diff output take #4.

This implements the output format suggested by Linus in
<Pine.LNX.4.58.0505161556260.18337@ppc970.osdl.org>, except the
imaginary diff option is spelled "diff --git" with double
dashes.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---

diff.c                 |   14 +++++++-------
t/t4000-diff-format.sh |    7 +++++--
2 files changed, 12 insertions(+), 9 deletions(-)

diff -git a/diff.c b/diff.c
--- a/diff.c
+++ b/diff.c
@@ -83,7 +83,6 @@
 			 struct diff_tempfile *temp)
 {
 	int i, next_at;
-	const char *git_prefix = "# mode: ";
 	const char *diff_cmd = "diff -L'%s%s' -L'%s%s'";
 	const char *diff_arg  = "'%s' '%s'||:"; /* "||:" is to return 0 */
 	const char *input_name_sq[2];
@@ -123,15 +122,16 @@
 	next_at += snprintf(cmd+next_at, cmd_size-next_at,
 			    diff_arg, input_name_sq[0], input_name_sq[1]);
 
+	printf("diff --git a/%s b/%s\n", name, name);
 	if (!path1[0][0])
-		printf("%s. %s %s\n", git_prefix, temp[1].mode, name);
+		printf("new file mode %s\n", temp[1].mode);
 	else if (!path1[1][0])
-		printf("%s%s . %s\n", git_prefix, temp[0].mode, name);
+		printf("deleted file mode %s\n", temp[0].mode);
 	else {
-		if (strcmp(temp[0].mode, temp[1].mode))
-			printf("%s%s %s %s\n", git_prefix,
-			       temp[0].mode, temp[1].mode, name);
-
+		if (strcmp(temp[0].mode, temp[1].mode)) {
+			printf("old mode %s\n", temp[0].mode);
+			printf("new mode %s\n", temp[1].mode);
+		}
 		if (strncmp(temp[0].mode, temp[1].mode, 3))
 			/* we do not run diff between different kind
 			 * of objects.
diff -git a/t/t4000-diff-format.sh b/t/t4000-diff-format.sh
--- a/t/t4000-diff-format.sh
+++ b/t/t4000-diff-format.sh
@@ -26,7 +26,9 @@
     'git-diff-files -p after editing work tree.' \
     'git-diff-files -p >current'
 cat >expected <<\EOF
-# mode: 100644 100755 path0
+diff --git a/path0 b/path0
+old mode 100644
+new mode 100755
 --- a/path0
 +++ b/path0
 @@ -1,3 +1,3 @@
@@ -34,7 +36,8 @@
  Line 2
 -line 3
 +Line 3
-# mode: 100755 . path1
+diff --git a/path1 b/path1
+deleted file mode 100755
 --- a/path1
 +++ /dev/null
 @@ -1,3 +0,0 @@
------------------------------------------------

Re: [PATCH 2/4] Tweak diff output further to make it a bit less distracting.

[Daniel Barkalow]

On Mon, 16 May 2005, Linus Torvalds wrote:

> One final note: I actually think that "rename patches" make a ton of 
> sense, even if git itself doesn't track renames. If we ever have a "smart 
> diff" thing that can generate inter-file diffs, I'd like to eventually see
> 
> 	diff -git a/kernel/sched.c b/kernel/sched.c.old
> 	rename kernel/sched.c kernel/sched.c.old
> 	old mode 100644
> 	new mode 100755

I'd like something like:

diff -git a/kernel/sched.c b/kernel/sched.c.old
filename -- kernel/sched.c
filename ++ kernel/sched.c.old
mode -- 100644
mode ++ 100755
--- a/kernel/sched.c
+++ b/kernel/sched.c.old
@@ -1,5 +1,5 @@
(etc.)

because I actually start thinking of the two sides as "-" and "+", and I'd
actually have to think about which is "old" and which is "new", and which
way the "rename" line goes, and so forth. I'd actually be happier with
just a "mode -- 100644" line for a deleted file, also. If I'm looking at a
patch, and I read Makefile with '-' and '+' versions of the lists of
objects, and then get to a "new file" line, I have to think about it to
associate the '+' side with having the file and the '-' side with not
having it.

	-Daniel
*This .sig left intentionally blank*

Re: [PATCH 2/4] Tweak diff output further to make it a bit less distracting.

[Petr Baudis]

Dear diary, on Tue, May 17, 2005 at 02:10:35AM CEST, I got a letter
where Daniel Barkalow <barkalow@iabervon.org> told me that...
> On Mon, 16 May 2005, Linus Torvalds wrote:
> 
> > One final note: I actually think that "rename patches" make a ton of 
> > sense, even if git itself doesn't track renames. If we ever have a "smart 
> > diff" thing that can generate inter-file diffs, I'd like to eventually see
> > 
> > 	diff -git a/kernel/sched.c b/kernel/sched.c.old
> > 	rename kernel/sched.c kernel/sched.c.old
> > 	old mode 100644
> > 	new mode 100755
> 
> I'd like something like:
> 
> diff -git a/kernel/sched.c b/kernel/sched.c.old
> filename -- kernel/sched.c
> filename ++ kernel/sched.c.old
> mode -- 100644
> mode ++ 100755
> --- a/kernel/sched.c
> +++ b/kernel/sched.c.old
> @@ -1,5 +1,5 @@
> (etc.)
> 
> because I actually start thinking of the two sides as "-" and "+", and I'd
> actually have to think about which is "old" and which is "new", and which
> way the "rename" line goes, and so forth. I'd actually be happier with
> just a "mode -- 100644" line for a deleted file, also. If I'm looking at a
> patch, and I read Makefile with '-' and '+' versions of the lists of
> objects, and then get to a "new file" line, I have to think about it to
> associate the '+' side with having the file and the '-' side with not
> having it.

Oops, I've somehow completely missed this mail, but I like this idea a
lot. What do you think, Linus and Junio?

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
C++: an octopus made by nailing extra legs onto a dog. -- Steve Taylor

Re: [PATCH 2/4] Tweak diff output further to make it a bit less distracting.

[Junio C Hamano]

>>>>> "PB" == Petr Baudis <pasky@ucw.cz> writes:

PB> Oops, I've somehow completely missed this mail, but I like this idea a
PB> lot. What do you think, Linus and Junio?

I find the original Linus one easier to read.

[PATCH] Fix diff output take #3.

[Junio C Hamano]

This implements the output format suggested by Linus in
<Pine.LNX.4.58.0505161556260.18337@ppc970.osdl.org>

Signed-off-by: Junio C Hamano <junkio@cox.net>
---

diff.c                 |   14 +++++++-------
t/t4000-diff-format.sh |    7 +++++--
2 files changed, 12 insertions(+), 9 deletions(-)

diff -git a/diff.c b/diff.c
--- a/diff.c
+++ b/diff.c
@@ -83,7 +83,6 @@
 			 struct diff_tempfile *temp)
 {
 	int i, next_at;
-	const char *git_prefix = "# mode: ";
 	const char *diff_cmd = "diff -L'%s%s' -L'%s%s'";
 	const char *diff_arg  = "'%s' '%s'||:"; /* "||:" is to return 0 */
 	const char *input_name_sq[2];
@@ -123,15 +122,16 @@
 	next_at += snprintf(cmd+next_at, cmd_size-next_at,
 			    diff_arg, input_name_sq[0], input_name_sq[1]);
 
+	printf("diff -git a/%s b/%s\n", name, name);
 	if (!path1[0][0])
-		printf("%s. %s %s\n", git_prefix, temp[1].mode, name);
+		printf("new file mode %s\n", temp[1].mode);
 	else if (!path1[1][0])
-		printf("%s%s . %s\n", git_prefix, temp[0].mode, name);
+		printf("deleted file mode %s\n", temp[0].mode);
 	else {
-		if (strcmp(temp[0].mode, temp[1].mode))
-			printf("%s%s %s %s\n", git_prefix,
-			       temp[0].mode, temp[1].mode, name);
-
+		if (strcmp(temp[0].mode, temp[1].mode)) {
+			printf("old mode %s\n", temp[0].mode);
+			printf("new mode %s\n", temp[1].mode);
+		}
 		if (strncmp(temp[0].mode, temp[1].mode, 3))
 			/* we do not run diff between different kind
 			 * of objects.
diff -git a/t/t4000-diff-format.sh b/t/t4000-diff-format.sh
--- a/t/t4000-diff-format.sh
+++ b/t/t4000-diff-format.sh
@@ -26,7 +26,9 @@
     'git-diff-files -p after editing work tree.' \
     'git-diff-files -p >current'
 cat >expected <<\EOF
-# mode: 100644 100755 path0
+diff -git a/path0 b/path0
+old mode 100644
+new mode 100755
 --- a/path0
 +++ b/path0
 @@ -1,3 +1,3 @@
@@ -34,7 +36,8 @@
  Line 2
 -line 3
 +Line 3
-# mode: 100755 . path1
+diff -git a/path1 b/path1
+deleted file mode 100755
 --- a/path1
 +++ /dev/null
 @@ -1,3 +0,0 @@
------------------------------------------------

Re: [PATCH 2/4] Tweak diff output further to make it a bit less distracting.

[Petr Baudis]

Dear diary, on Tue, May 17, 2005 at 01:28:31AM CEST, I got a letter
where Linus Torvalds <torvalds@osdl.org> told me that...
> 
> 
> On Mon, 16 May 2005, Junio C Hamano wrote:
> 
> >     # mode: 100644 100755 nitfol
> >     --- a/nitfol
> >     +++ b/nitfol
> 
> >     @. 100644 100755 nitfol
> >     --- a/nitfol
> >     +++ b/nitfol
> 
> I have to say, I muct prefer the first over the second.

Glad. :-)

> One final note: I actually think that "rename patches" make a ton of 
> sense, even if git itself doesn't track renames. If we ever have a "smart 
> diff" thing that can generate inter-file diffs, I'd like to eventually see
> 
> 	diff -git a/kernel/sched.c b/kernel/sched.c.old
> 	rename kernel/sched.c kernel/sched.c.old
> 	old mode 100644
> 	new mode 100755
> 	--- a/kernel/sched.c
> 	+++ b/kernel/sched.c.old
> 	@@ -1,5 +1,5 @@
> 	 /*
> 	- *  kernel/sched.c
> 	+ *  kernel/sched.c.old
> 	  *
> 	  *  Kernel scheduler and related syscalls
> 	  *
> 
> Notice? We could have a mode change, a rename _and_ a content change, all
> at the same time under the same header. That's obviously a totally idiotic
> example, but the point is that if we have a nice "extended diff header"
> setup, the format is very easily able to accomodate things like this.

Actually, if the git diff format is fixed, do we even need the explicit
rename line? It could be enough if the filenames on the diff line would
be just different. Or you want it because of clarity?

-- 
				Petr "Pasky" Baudis
Stuff: http://pasky.or.cz/
C++: an octopus made by nailing extra legs onto a dog. -- Steve Taylor

Re: [PATCH 2/4] Tweak diff output further to make it a bit less distracting.

[Linus Torvalds]

On Tue, 17 May 2005, Petr Baudis wrote:
> > 
> > 	diff -git a/kernel/sched.c b/kernel/sched.c.old
> > 	rename kernel/sched.c kernel/sched.c.old
> 
> Actually, if the git diff format is fixed, do we even need the explicit
> rename line? It could be enough if the filenames on the diff line would
> be just different. Or you want it because of clarity?

Yes, it's something we can glean from the header itself (or the ---/+++
lines), but I'd prefer it just to make things really obvious. Especially
as all the other pathnames involved (both on the "diff" header line and on
the ---/+++ lines) are in non-canonical -p1 format. So the "rename" line
would be the only one that is actually in canonical form.

There's also a real technical reason for this: since the rename format
would not be a valid patch for a traditional "patch" program, and if we
ever want to actually teach "patch" to handle it, we really need to be
explicit. There are tons of traditional patches around that say

	diff -Nur a/kernel/sched.c.old b/kernel/sched.c
	--- a/kernel/sched.c.old
	+++ b/kernel/sched.c
	...

and clearly the above is _not_ a rename from "sched.c.old" to "sched.c",
so if we want to teach "patch" about the magic git rules, we'd have to
have something unambiguous that a GNU patch maintainer might be willing to
trigger on. The combination of the "diff -git " and "rename" markers might
be such a thing.

So it's a combination of clarity, canonical names, and "patch" issues.

		Linus

Re: [PATCH 2/4] Tweak diff output further to make it a bit less distracting.

[Junio C Hamano]

>>>>> "LT" == Linus Torvalds <torvalds@osdl.org> writes:

LT> There's also a real technical reason for this: since the rename format
LT> would not be a valid patch for a traditional "patch" program, and if we
LT> ever want to actually teach "patch" to handle it, we really need to be
LT> explicit. There are tons of traditional patches around that say

LT> 	diff -Nur a/kernel/sched.c.old b/kernel/sched.c
LT> 	--- a/kernel/sched.c.old
LT> 	+++ b/kernel/sched.c
LT> 	...

LT> and clearly the above is _not_ a rename from "sched.c.old" to "sched.c",
LT> so if we want to teach "patch" about the magic git rules, we'd have to
LT> have something unambiguous that a GNU patch maintainer might be willing to
LT> trigger on. The combination of the "diff -git " and "rename" markers might
LT> be such a thing.

LT> So it's a combination of clarity, canonical names, and "patch" issues.

I've been thinking about doing some rename detection in
diff-helper for some time.  Here is what that would produce in
your proposed file format (BTW, wouldn't the earlier patch ready
for merge already?), if you move file frotz to file nitfol and
at the same time do some edits:

    diff -git a/frotz b/frotz
    rename old frotz
    rename new nitfol
    delete file mode 100644
    --- a/frotz
    +++ /dev/null
    @@ -1,2 +0,0 @@
    -xyzzy
    -rezrov
    diff -git a/nitfol b/nitfol
    rename old frotz
    rename new nitfol
    new file mode 100644
    --- /dev/null
    +++ b/nitfol
    @@ -0,0 +1,2 @@
    +xyzzy
    +rezrov
    diff -git a/nitfol b/nitfol
    rename old frotz
    rename new nitfol
    --- a/nitfol
    +++ b/nitfol
    @@ -1,2 +1,3 @@
     xyzzy
     rezrov
    +gnusto

The basic idea is to express the pure rename with traditional
two patches against /dev/null, plus optionally contents patch on
top after pure rename patches.

I am still debating myself where rename lines should be, though.
I cannot decide so I placed them in all three in the above
example.

Re: [PATCH 2/4] Tweak diff output further to make it a bit less distracting.

[Linus Torvalds]

On Tue, 17 May 2005, Junio C Hamano wrote:
> 
> I've been thinking about doing some rename detection in
> diff-helper for some time.  Here is what that would produce in
> your proposed file format (BTW, wouldn't the earlier patch ready
> for merge already?), if you move file frotz to file nitfol and
> at the same time do some edits:

This has the advantage of working with any old "patch" version, but it has 
the disadvantage of being human-unreadable, and big. 

To me, there really are only two reasons to do rename diffs:
 - smaller diffs
 - human readability (you can actually see what changed)

and if you want to have compatibility with a "patch" program that doesn't
support the feature (like your example), you basically lose both of those
advantages. You have _some_ human-readability, but it basically boils down
to "ignore all those deletes/creates".

So I'd really suggest having just a flag that says "pure old diff format"  
or "new diff format with renames", and if the latter is selected, then do
_just_ the changes, ie the rename+change case would really boil down to
getting just

>     diff -git a/nitfol b/nitfol
>     rename old frotz
>     rename new nitfol
>     --- a/nitfol
>     +++ b/nitfol
>     @@ -1,2 +1,3 @@
>      xyzzy
>      rezrov
>     +gnusto

(except I think it would be nice to have the renamed names show up in the 
"diff" and "---/+++" lines too)

		Linus

Re: [PATCH 2/4] Tweak diff output further to make it a bit less distracting.

[Junio C Hamano]

>>>>> "LT" == Linus Torvalds <torvalds@osdl.org> writes:

LT> So I'd really suggest having just a flag that says "pure old diff format"  
LT> or "new diff format with renames", and if the latter is selected, then do
LT> _just_ the changes, ie the rename+change case would really boil down to
LT> getting just

That is sensible.  So the with --detect-rename flag, we do
rename detection and show only the changes, otherwise we do not
do rename detection and give pure old diff (two diffs against
/dev/null, that is).  I do not personally think --detect-rename
with --output-old-style-diff is useful.

Now, in the new diff format, if the rename is really a pure
rename, then we would have:

     diff -git a/nitfol b/nitfol
     rename old frotz
     rename new nitfol
     diff -git a/rezrov b/rezrov
     --- a/rezrov
     +++ b/rezrov
     @@ ...

that is, nothing until the patch for the next file or EOF.  Is
this acceptable?

Re: [PATCH 2/4] Tweak diff output further to make it a bit less distracting.

[Linus Torvalds]

On Tue, 17 May 2005, Junio C Hamano wrote:
> 
> Now, in the new diff format, if the rename is really a pure
> rename, then we would have:
> 
>      diff -git a/nitfol b/nitfol
>      rename old frotz
>      rename new nitfol
>      diff -git a/rezrov b/rezrov
>      --- a/rezrov
>      +++ b/rezrov
>      @@ ...
> 
> that is, nothing until the patch for the next file or EOF.  Is
> this acceptable?

I think that's exactly what we want. At least it does exactly the right 
thing for me, when I do '/^diff ' in less, with nice highlighting of the 
headers.

With people inevitably adding some nice coloration support in gitweb etc,
and it will be outstanding.

		Linus