Re: [PATCH 2/3 v2] Teach git diff-tree --stdin to diff trees
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:45:07
Karl Hasselström [off-list ref] writes:
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.
quoted hunk ↗ jump to hunk
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.