Re: [PATCH 2/3 v2] Teach git diff-tree --stdin to diff trees
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Aug 9, 2008, 20:41 UTC
- Message-ID
- <7vwsip53bg.fsf@gitster.siamese.dyndns.org>
- In-Reply-To
- <20080809120816.11085.66578.stgit@yoghurt>
Karl Hasselström <kha@treskal.com> writes:
Show 8 quoted lines
> When feeding trees on the command line, you can give exactly two > trees, not three nor one; --stdin now supports this "two tree" form on > its input, in addition to accepting lines with one or more commits. > > When diffing trees (either specified on the command line or from the > standard input), the -m, -s, -v, --pretty, --abbrev-commit, > --encoding, --no-commit-id, -c, --cc, and --always options are > ignored, since they do not apply to trees.
I've commented on this part already; -m, -c, --cc are excluded because they make sense only when you are dealing with three or more trees.
Show 17 quoted lines
> diff --git a/builtin-diff-tree.c b/builtin-diff-tree.c
> index ebbd631..0bdb1cf 100644
> --- a/builtin-diff-tree.c
> +++ b/builtin-diff-tree.c
> @@ -42,21 +42,48 @@ static int stdin_diff_commit(struct commit *commit, char *line, int len)
> return log_tree_commit(&log_tree_opt, commit);
> }
>
> +/* Diff two trees. */
> +static int stdin_diff_trees(struct tree *tree1, char *line, int len)
> +{
> + unsigned char sha1[20];
> + struct tree *tree2;
> + if (len != 82 || !isspace(line[40]) || get_sha1_hex(line + 41, sha1)) {
> + error("Need precisely two trees, separated by one space");
> + return -1;
> + }error() returns -1, so:
if (len != 82 || !isspace(line[40]) || get_sha1_hex(line + 41, sha1))
return error("Need two trees, separated by one space");> + tree2 = lookup_tree(sha1); > + if (!tree2 || parse_tree(tree2)) > + return -1;
Don't you want to make error() say something here as well?
> + printf("%s %s\n", sha1_to_hex(tree1->object.sha1),
> + sha1_to_hex(tree2->object.sha1));Since this is strictly for Porcelain's use, you may want to document this output format.
Two-tree form from the command line does not have anything like this, and two-commit form from --stdin have either a single object name, the log message under -v or --pretty options. I notice that these are not documented but we may want to document it while at it.
Other than that, the patch looks good. Thanks.