threads / patch / 29350

patchdiff --no-index: support more than one file pair

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

## tl;dr

7 messages between Jan 12, 2012 and Jan 12, 2012. Diffs are folded; open one to read it.

replies: 6people: 3as markdown or json

Nguyễn Thái Ngọc Duy· Jan 12, 2012, 09:09 UTC · lore
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(-)
Show changes to diff-no-index.c +21 −17
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
Matthieu Moy· Jan 12, 2012, 09:14 UTC · re: Nguyễn Thái Ngọc Duy · lore

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

Nguyễn Thái Ngọc Duy <pclouds@gmail.com> writes:
Show 7 quoted lines
> 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/
Nguyen Thai Ngoc Duy· Jan 12, 2012, 09:17 UTC · re: Matthieu Moy · lore

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

2012/1/12 Matthieu Moy <Matthieu.Moy@grenoble-inp.fr>:
Show 23 quoted lines
> 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
Matthieu Moy· Jan 12, 2012, 09:30 UTC · re: Nguyen Thai Ngoc Duy · lore

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

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/
Nguyen Thai Ngoc Duy· Jan 12, 2012, 17:40 UTC · re: Matthieu Moy · lore

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

On Thu, Jan 12, 2012 at 4:30 PM, Matthieu Moy <Matthieu.Moy@grenoble-inp.fr> wrote:

Show 5 quoted lines
>>  - 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.
Show 6 quoted lines
>>  - 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
Matthieu Moy· Jan 12, 2012, 18:26 UTC · re: Nguyen Thai Ngoc Duy · lore

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

Nguyen Thai Ngoc Duy <pclouds@gmail.com> writes:
Show 11 quoted lines
>>>  - 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/
Neal Kreitzinger· Jan 12, 2012, 16:37 UTC · re: Nguyễn Thái Ngọc Duy · lore

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

On 1/12/2012 3:09 AM, Nguyễn Thái Ngọc Duy wrote:
Show 8 quoted lines
> 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

← back to recent threads