Re: [PATCH 2/3 v2] Teach git diff-tree --stdin to diff trees

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Karl Hasselström <kha@xxxxxxxxxxx> 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.

> 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.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux