From: Jonathan Nieder <jrnieder@gmail.com>
Date: Sun, 14 Nov 2010 17:32:41 GMT
Subject: Re: [PATCH 2/4] t7004-tag.sh: re-arrange git tag comment for clarity
Message-ID: <20101114173241.GB26459@burratino>
In-Reply-To: <1289745857-16704-3-git-send-email-avarab@gmail.com>

Ævar Arnfjörð Bjarmason wrote:

>                           I used ! instead of test_must_fail so that
> the GIT_EDITOR variable was only used in this command invocation,

	(
		GIT_EDITOR=cat &&
		export GIT_EDITOR &&
		test_must_fail ...
	)

?

>                                                                   and
> because the surrounding tests use this style.

Oh, okay.  I had the test_must_fail version sitting in my local tree
as part of the unsent part of the &&-chaining series
http://thread.gmane.org/gmane.comp.version-control.git/157903/focus=160419

I am happy to see this fixed sooner.

[...]
> +test_expect_success \
> +	'message in editor has initial comment: first line' '
>  	# check the first line --- should be empty
> -	first=$(sed -e 1q <actual) &&
> -	test -z "$first" &&
> +	echo >first.expect &&
> +	sed -e 1q <actual >first.actual &&
> +	test_cmp first.expect first.actual
> +'
> +
> +test_expect_success \
> +	'message in editor has initial comment: remainder' '
>  	# remove commented lines from the remainder -- should be empty
> -	rest=$(sed -e 1d -e '/^#/d' <actual) &&
> -	test -z "$rest"
> +	>rest.expect
> +	sed -e 1d -e '/^#/d' <actual >rest.actual &&
> +	test_cmp rest.expect rest.actual
>  '

Nice.  Note that you've changed the semantics of the test here
(probably for the better): the $() backquoting operator strips
trailing newlines, while your sed expression does not.

I assume you've run the tests and made sure they still pass.

For what it's worth,
Reviewed-by: Jonathan Nieder <jrnieder@gmail.com>