Re: [PATCH v4] format-patch --signature-file <file>

From: Jeff King <peff@peff.net>
Date: May 19, 2014, 10:48 UTC
Message-ID: <20140519104810.GA17492@sigill.intra.peff.net>
In-Reply-To: <1400444522-15523-2-git-send-email-jmmahler@gmail.com>

On Sun, May 18, 2014 at 01:22:02PM -0700, Jeremiah Mahler wrote:

Show 6 quoted lines

> @@ -742,6 +743,8 @@ static int git_format_config(const char *var, const char *value, void *cb)
>  	}
>  	if (!strcmp(var, "format.signature"))
>  		return git_config_string(&signature, var, value);
> +	if (!strcmp(var, "format.signaturefile"))
> +		return git_config_string(&signature_file, var, value);

Should this be git_config_pathname? That would handle tilde-expansion.

Show 6 quoted lines

> +test_expect_success 'format-patch --signature-file=file' '
> +	git format-patch --stdout --signature-file=expect -1 >output &&
> +	check_patch output &&
> +	sed -n "/^-- $/,\$p" <output | head --lines=-2 | tail --lines=+2 >output2 &&
> +	test_cmp expect output2
> +'

I don't think --lines is portable (it's not even POSIX), nor is a negative value for head (but a "+" form for tail is). You can replace the "head" call with a "$d" sed command, and tail should be fine with "-n +2". Like:

  sed -n '$d; /^-- $/,$p' | tail -n +2

You can also do it in one sed like:

  sed -n '$d; /^-- $/,${//!p}'

but that is starting to look a bit like line-noise. ;)

Show 8 quoted lines

> +test_expect_success 'format-patch with format.signaturefile config' '
> +	git config format.signaturefile expect &&
> +	git format-patch --stdout -1 >output &&
> +	check_patch output &&
> +	sed -n "/^-- $/,\$p" <output | head --lines=-2 | tail --lines=+2 >output2 &&
> +	test_cmp expect output2 &&
> +	git config --unset-all format.signaturefile
> +'

You might want to use "test_config format.signaturefile expect" here, which handles unsetting after the test. Besides being one line shorter, it also cleans up when the middle part of the test fails.

You could also use "git -c", but I think setting the actual config in a file is a more realistic test.

-Peff