[PATCH] diff --no-index: support more than one file pair

[PATCH] diff --no-index: support more than one file pair

[Nguyễn Thái Ngọc Duy]

This allows you to do

git diff --no-index file1.old file1.new file2.old file2.new...

It could be seen as an abuse of "git --no-index", but it's very
tempting considering many bells and whistles git's diff machinery
provides.

Signed-off-by: A Clearcase user who has had enough with "ct diff"
---
 Sorry I used git@vger as a personal archive, but this might benefit
 others as well, I think.

 diff-no-index.c |   38 +++++++++++++++++++++-----------------
 1 files changed, 21 insertions(+), 17 deletions(-)

diff --git a/diff-no-index.c b/diff-no-index.c
index 3a36144..b4f6d06 100644
--- a/diff-no-index.c
+++ b/diff-no-index.c
@@ -199,12 +199,8 @@ void diff_no_index(struct rev_info *revs,
 		     !path_outside_repo(argv[i+1])))
 			return;
 	}
-	if (argc != i + 2)
-		usagef("git diff %s <path> <path>",
-		       no_index ? "--no-index" : "[--no-index]");
-
 	diff_setup(&revs->diffopt);
-	for (i = 1; i < argc - 2; ) {
+	for (i = 1; i < argc; ) {
 		int j;
 		if (!strcmp(argv[i], "--no-index"))
 			i++;
@@ -214,13 +210,19 @@ void diff_no_index(struct rev_info *revs,
 		}
 		else if (!strcmp(argv[i], "--"))
 			i++;
-		else {
+		else if (argv[i][0] == '-') {
 			j = diff_opt_parse(&revs->diffopt, argv + i, argc - i);
 			if (!j)
 				die("invalid diff option/value: %s", argv[i]);
 			i += j;
 		}
+		else
+			break;
 	}
+	if ((argc - i) % 2)
+		usagef("git diff %s <path> <path>%s",
+		       no_index ? "--no-index" : "[--no-index]",
+		       no_index ? "[ <path> <path>...]" : "");
 
 	/*
 	 * If the user asked for our exit code then don't start a
@@ -229,13 +231,15 @@ void diff_no_index(struct rev_info *revs,
 	if (!DIFF_OPT_TST(&revs->diffopt, EXIT_WITH_STATUS))
 		setup_pager();
 
+	/* argv now only contains paths */
+	argv += i;
+	argc -= i;
+
 	if (prefix) {
 		int len = strlen(prefix);
-		const char *paths[3];
-		memset(paths, 0, sizeof(paths));
 
-		for (i = 0; i < 2; i++) {
-			const char *p = argv[argc - 2 + i];
+		for (i = 0; i < argc; i++) {
+			const char *p = argv[i];
 			/*
 			 * stdin should be spelled as '-'; if you have
 			 * path that is '-', spell it as ./-.
@@ -243,12 +247,10 @@ void diff_no_index(struct rev_info *revs,
 			p = (strcmp(p, "-")
 			     ? xstrdup(prefix_filename(prefix, len, p))
 			     : p);
-			paths[i] = p;
+			argv[i] = p;
 		}
-		diff_tree_setup_paths(paths, &revs->diffopt);
 	}
-	else
-		diff_tree_setup_paths(argv + argc - 2, &revs->diffopt);
+
 	revs->diffopt.skip_stat_unmatch = 1;
 	if (!revs->diffopt.output_format)
 		revs->diffopt.output_format = DIFF_FORMAT_PATCH;
@@ -260,9 +262,11 @@ void diff_no_index(struct rev_info *revs,
 	if (diff_setup_done(&revs->diffopt) < 0)
 		die("diff_setup_done failed");
 
-	if (queue_diff(&revs->diffopt, revs->diffopt.pathspec.raw[0],
-		       revs->diffopt.pathspec.raw[1]))
-		exit(1);
+	while (argv[0] && argv[1]) {
+		if (queue_diff(&revs->diffopt, argv[0], argv[1]))
+			exit(1);
+		argv += 2;
+	}
 	diff_set_mnemonic_prefix(&revs->diffopt, "1/", "2/");
 	diffcore_std(&revs->diffopt);
 	diff_flush(&revs->diffopt);
-- 
1.7.3.1.256.g2539c.dirty

Re: [PATCH] diff --no-index: support more than one file pair

[Matthieu Moy]

Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:

> This allows you to do
>
> git diff --no-index file1.old file1.new file2.old file2.new...
>
> It could be seen as an abuse of "git --no-index", but it's very
> tempting considering many bells and whistles git's diff machinery
> provides.

I find this very, very unintuitive. I tried with GNU diff:

diff 1 2 3 4

and it complained with "diff: extra operand `3'". I find

git diff --no-index file1.old file1.new
git diff --no-index file2.old file2.new

far more intuitive, less error prone (when you start having a
non-trivial list of arguments, it's hard to tell which is the new and
which is the old visually), ... So I'm curious why you prefer your
syntax.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [PATCH] diff --no-index: support more than one file pair

[Nguyen Thai Ngoc Duy]

2012/1/12 Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>:
> Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
>
>> This allows you to do
>>
>> git diff --no-index file1.old file1.new file2.old file2.new...
>>
>> It could be seen as an abuse of "git --no-index", but it's very
>> tempting considering many bells and whistles git's diff machinery
>> provides.
>
> I find this very, very unintuitive. I tried with GNU diff:
>
> diff 1 2 3 4
>
> and it complained with "diff: extra operand `3'". I find
>
> git diff --no-index file1.old file1.new
> git diff --no-index file2.old file2.new
>
> far more intuitive, less error prone (when you start having a
> non-trivial list of arguments, it's hard to tell which is the new and
> which is the old visually), ... So I'm curious why you prefer your
> syntax.

Single operation has its advantages:

 - one pager
 - one stat and summary
 - might be easier to script (just throw them all to xargs)
 - hell, i might even benefit from git copy/modify detection
-- 
Duy

Re: [PATCH] diff --no-index: support more than one file pair

[Matthieu Moy]

Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:

> Single operation has its advantages:
>
>  - one pager
>  - one stat and summary

I buy these, and I understand why they apply to "git diff" and not GNU
diff.

>  - might be easier to script (just throw them all to xargs)

I don't see a use-case where a command produces old1 new1 old2 new2, but
if there is one, then "| xargs -n 2 diff" is the solution. You don't
need your patch.

>  - hell, i might even benefit from git copy/modify detection

I don't see how, if you specify explicitely the pairs (old, new). You
may have such benefit if you let the command-line express "here's a
bunch of old files, and a bunch of new ones", but not with your proposed
syntax.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [PATCH] diff --no-index: support more than one file pair

[Nguyen Thai Ngoc Duy]

On Thu, Jan 12, 2012 at 4:30 PM, Matthieu Moy
<Matthieu.Moy@grenoble-inp.fr> wrote:
>>  - might be easier to script (just throw them all to xargs)
>
> I don't see a use-case where a command produces old1 new1 old2 new2, but
> if there is one, then "| xargs -n 2 diff" is the solution. You don't
> need your patch.

Unthorough thought. I agree with you.

>>  - hell, i might even benefit from git copy/modify detection
>
> I don't see how, if you specify explicitely the pairs (old, new). You
> may have such benefit if you let the command-line express "here's a
> bunch of old files, and a bunch of new ones", but not with your proposed
> syntax.

That's what git gives to diff machinery: a set of file pairs, and the
diff machinery has to figure out copy/modify pairs, shuffling them up
if necessary. I simply cut of tree traversal part out and feed file
pairs directly to diff machinery. I remember long long time ago Junio
asked for assistance about code moving support within a file. It has
not come up (at least in public), but one can hope it'll come someday.
-- 
Duy

Re: [PATCH] diff --no-index: support more than one file pair

[Matthieu Moy]

Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:

>>>  - hell, i might even benefit from git copy/modify detection
>>
>> I don't see how, if you specify explicitely the pairs (old, new). You
>> may have such benefit if you let the command-line express "here's a
>> bunch of old files, and a bunch of new ones", but not with your proposed
>> syntax.
>
> That's what git gives to diff machinery: a set of file pairs, and the
> diff machinery has to figure out copy/modify pairs, shuffling them up
> if necessary. I simply cut of tree traversal part out and feed file
> pairs directly to diff machinery.

If you want to benefit from copy detection, you cannot hardcode the fact
that you have as many source and destination files. And even to benefit
from rename detection, I find the user interface really weird. If I
provide files in pairs, I really don't expect Git to shuffle them like

git diff --no-index A1 B1 A2 B2
--- A1
+++ B2
..
--- A2
+++ B1
..

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [PATCH] diff --no-index: support more than one file pair

[Neal Kreitzinger]

On 1/12/2012 3:09 AM, Nguyễn Thái Ngọc Duy wrote:
> This allows you to do
>
> git diff --no-index file1.old file1.new file2.old file2.new...
>
> It could be seen as an abuse of "git --no-index", but it's very
> tempting considering many bells and whistles git's diff machinery
> provides.
>
I see that git-diff can be used in place of linux diff for totally 
untracked file pairs (which is kind of neat, I guess, if you're partial 
to git like I am and would probably prefer to use it as your primary 
file-system interface if you could).  I assume this new syntax implies 
manual usage since scripting this input is less straightforward than 
iterating thru a single pair via xargs, etc.  In that context, I also 
see that git-difftool doesn't bring up kdiff3 (or whatever) but just 
does a text diff (git 1.7.1) which is mildly disappointing for mere 
mortals like myself who prefer to read side-by-side gui diffs over text 
diffS.  This, of course, is also preference for someone like me who 
wouldn't mind prefixing all of my commands with "git " ;-)

v/r,
neal