Re: [PATCH 3/6] var: accept multiple variables on the command line

From: Jonathan Nieder <jrnieder@gmail.com>
Date: Nov 14, 2012, 17:01 UTC
Message-ID: <20121114170148.GB6858@elie.Belkin>
In-Reply-To: <20121113165247.GC12626@sigill.intra.peff.net>

Jeff King wrote:

> This patch lets callers specify multiple variables, and
> prints one per line.

Yay!

[...]

Show 15 quoted lines

> --- a/Documentation/git-var.txt
> +++ b/Documentation/git-var.txt
> @@ -9,11 +9,16 @@ git-var - Show a git logical variable
>  SYNOPSIS
>  --------
>  [verse]
> -'git var' ( -l | <variable> )
> +'git var' ( -l | <variable>... )
>  
>  DESCRIPTION
>  -----------
> -Prints a git logical variable.
> +Prints one or more git logical variables, separated by newlines.
> +
> +Note that some variables may contain newlines themselves

Maybe a -z option to NUL-terminate values would be useful some day.

Show 12 quoted lines

> --- a/builtin/var.c
> +++ b/builtin/var.c
> @@ -73,8 +73,7 @@ static int show_config(const char *var, const char *value, void *cb)
>  
>  int cmd_var(int argc, const char **argv, const char *prefix)
>  {
> -	const char *val = NULL;
> -	if (argc != 2)
> +	if (argc < 2)
>  		usage(var_usage);
>  
>  	if (strcmp(argv[1], "-l") == 0) {

What should happen if I pass "-l" followed by other arguments?

[...]

Show 11 quoted lines

> --- /dev/null
> +++ b/t/t0007-git-var.sh
> @@ -0,0 +1,29 @@
> +#!/bin/sh
> +
> +test_description='basic sanity checks for git var'
> +. ./test-lib.sh
> +
> +test_expect_success 'get GIT_AUTHOR_IDENT' '
> +	test_tick &&
> +	echo "A U Thor <author@example.com> 1112911993 -0700" >expect &&

Do we need to hardcode the timestamp?  Something like

	test_cmp_filtered () {
		expect=$1 actual=$2 &&
		sed -e 's/[0-9][0-9]* [-+][0-9][0-9][0-9][0-9]/TIMESTAMP" \
			<"$actual" >"$actual.filtered" &&
		test_cmp "$expect" "$actual.filtered"
	}
	...

		echo "A U Thor <author@example.com> $timestamp" >expect &&
		git var GIT_AUTHOR_IDENT >actual &&
		test_cmp_filtered expect actual

should make reordering tests a lot easier, though it has the downside of not being able to catch a weird bug that would make the timestamp out of sync with reality.

Hope that helps, Jonathan