[PATCH] Make git-diff-tree indicate when it flushes

[PATCH] Make git-diff-tree indicate when it flushes

[Paul Mackerras]

There are times when gitk needs to know that the commits it has sent
to git-diff-tree --stdin did not match, and it needs to know in a
timely fashion even if none of them match.  At the moment,
git-diff-tree outputs nothing for non-matching commits, so it is
impossible for gitk to distinguish between git-diff-tree being slow
and git-diff-tree saying no.

This makes git-diff-tree output a blank line in response to a blank
line in its input (which already causes git-diff-tree to flush its
output buffers).  Gitk, or other users of git-diff-tree --stdin, can
use the blank line to indicate that git-diff-tree has processed all
the commits on its input up to the input blank line, and any commits
that have not been output do not match.

Signed-off-by: Paul Mackerras <paulus@samba.org>
---
diff --git a/builtin-diff-tree.c b/builtin-diff-tree.c
index cc53b81..dbe5737 100644
--- a/builtin-diff-tree.c
+++ b/builtin-diff-tree.c
@@ -139,9 +139,10 @@ int cmd_diff_tree(int argc, const char *
 		opt->diffopt.setup |= (DIFF_SETUP_USE_SIZE_CACHE |
 				       DIFF_SETUP_USE_CACHE);
 	while (fgets(line, sizeof(line), stdin))
-		if (line[0] == '\n')
+		if (line[0] == '\n') {
+			putchar('\n');
 			fflush(stdout);
-		else
+		} else
 			diff_tree_stdin(line);
 
 	return 0;

Re: [PATCH] Make git-diff-tree indicate when it flushes

[Junio C Hamano]

Paul Mackerras <paulus@samba.org> writes:

> There are times when gitk needs to know that the commits it has sent
> to git-diff-tree --stdin did not match, and it needs to know in a
> timely fashion even if none of them match.  At the moment,
> git-diff-tree outputs nothing for non-matching commits, so it is
> impossible for gitk to distinguish between git-diff-tree being slow
> and git-diff-tree saying no.

Wouldn't this help?

	$ git-diff-tree --stdin --always

Re: [PATCH] Make git-diff-tree indicate when it flushes

[Paul Mackerras]

Junio C Hamano writes:

> Paul Mackerras <paulus@samba.org> writes:
> 
> > There are times when gitk needs to know that the commits it has sent
> > to git-diff-tree --stdin did not match, and it needs to know in a
> > timely fashion even if none of them match.  At the moment,
> > git-diff-tree outputs nothing for non-matching commits, so it is
> > impossible for gitk to distinguish between git-diff-tree being slow
> > and git-diff-tree saying no.
> 
> Wouldn't this help?
> 
> 	$ git-diff-tree --stdin --always

On the git.git tree:

$ cat revs
65aadb92a1ce9605fa2f412b51de91781a3ef3d6
cc189c2ca2c725c430f100f61e7c4a6849f93163
$ git diff-tree -r -s --stdin -- apply.c <revs
65aadb92a1ce9605fa2f412b51de91781a3ef3d6
$ git diff-tree -r -s --stdin --always -- apply.c <revs
65aadb92a1ce9605fa2f412b51de91781a3ef3d6
cc189c2ca2c725c430f100f61e7c4a6849f93163
$

With --always, how do I tell that 65aadb affects apply.c and cc189c
doesn't?

Paul.

Re: [PATCH] Make git-diff-tree indicate when it flushes

[Junio C Hamano]

Paul Mackerras <paulus@samba.org> writes:

>> Wouldn't this help?
>> 
>> 	$ git-diff-tree --stdin --always
>
> On the git.git tree:
>
> $ cat revs
> 65aadb92a1ce9605fa2f412b51de91781a3ef3d6
> cc189c2ca2c725c430f100f61e7c4a6849f93163
> $ git diff-tree -r -s --stdin -- apply.c <revs
> 65aadb92a1ce9605fa2f412b51de91781a3ef3d6
> $ git diff-tree -r -s --stdin --always -- apply.c <revs
> 65aadb92a1ce9605fa2f412b51de91781a3ef3d6
> cc189c2ca2c725c430f100f61e7c4a6849f93163
> $
>
> With --always, how do I tell that 65aadb affects apply.c and cc189c
> doesn't?

I am not quite sure exactly what you are trying to achieve, but
one trivial way is not giving -s perhaps?

Re: [PATCH] Make git-diff-tree indicate when it flushes

[Junio C Hamano]

Junio C Hamano <junkio@cox.net> writes:

> Paul Mackerras <paulus@samba.org> writes:
>
>> With --always, how do I tell that 65aadb affects apply.c and cc189c
>> doesn't?
>
> I am not quite sure exactly what you are trying to achieve, but
> one trivial way is not giving -s perhaps?

Having said that, I suspect this might be a better way.
Whatever you throw at it from stdin that are not a validly
looking object name, you will get them back, so you can use your
favorite markers.

diff --git a/builtin-diff-tree.c b/builtin-diff-tree.c
index cc53b81..7208c48 100644
--- a/builtin-diff-tree.c
+++ b/builtin-diff-tree.c
@@ -138,11 +138,15 @@ int cmd_diff_tree(int argc, const char *
 	if (opt->diffopt.detect_rename)
 		opt->diffopt.setup |= (DIFF_SETUP_USE_SIZE_CACHE |
 				       DIFF_SETUP_USE_CACHE);
-	while (fgets(line, sizeof(line), stdin))
-		if (line[0] == '\n')
+	while (fgets(line, sizeof(line), stdin)) {
+		unsigned char sha1[20];
+
+		if (get_sha1_hex(line, sha1)) {
+			fputs(line);
 			fflush(stdout);
+		}
 		else
 			diff_tree_stdin(line);
-
+	}
 	return 0;
 }

Re: [PATCH] Make git-diff-tree indicate when it flushes

[Paul Mackerras]

Junio C Hamano writes:

> I am not quite sure exactly what you are trying to achieve, but
> one trivial way is not giving -s perhaps?

I'm asking git-diff-tree which of a given set of commits affect any of
a set of paths, so that gitk can highlight the ones that do.
Furthermore I want to be able to use the git-diff-tree process for
multiple sets of commit IDs.

If I don't use -s, then I will get lines starting with a ":" after the
commit IDs of the commits that do affect the set of paths I specified.
That means I get a definite indication for all except the last commit
I send.  For the last commit I still don't know whether the absence of
any ":" lines means that the commit doesn't affect the set of paths,
or that git-diff-tree is being slow.  So I still need something like
the patch I sent.

I could get the indication I want (with or without -s) if I close the
pipe going to the git-diff-tree process.  But then the process will
exit, and I want it to stay around so that I don't have to pay the
fork/exec and startup time of git-diff-tree next time (which will be
when the user scrolls the commit list window or asks to move to the
next highlighted commit).

Thus, --always (with or without -s) doesn't quite do what I need.

Paul.

Re: [PATCH] Make git-diff-tree indicate when it flushes

[Paul Mackerras]

Junio C Hamano writes:

> Having said that, I suspect this might be a better way.
> Whatever you throw at it from stdin that are not a validly
> looking object name, you will get them back, so you can use your
> favorite markers.

That would be fine.

Thanks,
Paul.