Re: [PATCH 1/6] t3404 & t3411: undo copy&paste

From: Junio C Hamano <gitster@pobox.com>
Date: Jan 27, 2009, 21:01 UTC
Message-ID: <7v7i4g31lj.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <alpine.DEB.1.00.0901271845380.3586@pacific.mpi-cbg.de>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

Show 5 quoted lines

> Rather than copying and pasting, which is prone to lead to fixes
> missing in one version, move the fake-editor generator to t/t3404/.
>
> While at it, fix a typo that causes head-scratching: use
> ${SHELL_PATH-/bin/sh} instead of $SHELL_PATH.

I've learned to be cautious whenever I see "while at it".

Show 10 quoted lines

> diff --git a/t/lib-rebase.sh b/t/lib-rebase.sh
> new file mode 100644
> index 0000000..8c8caab
> --- /dev/null
> +++ b/t/lib-rebase.sh
> @@ -0,0 +1,36 @@
> +#!/bin/sh
> +
> +set_fake_editor () {
> +	echo "#!${SHELL_PATH-/bin_sh}" >fake-editor.sh

It is unclear why you would want to do this. It was unclear what "typo" you were referring to in your commit log message, either.

The tests are supposed to run under the shell the user specified, so if there is a case you found that $SHELL_PATH is unset, that is a bug we would want to fix, and ${SHELL_PATH-/bin/sh} is sweeping the problem under the rug to make it harder to fix, isn't it?

I would understand if it were

	${SHELL_PATH?"SHELL_PATH Not Set --- bug in tests?"}

though.

Besides, it's /bin/sh, not /bin_sh ;-)

Show 24 quoted lines

> +	cat >> fake-editor.sh <<\EOF
> +case "$1" in
> +*/COMMIT_EDITMSG)
> +	test -z "$FAKE_COMMIT_MESSAGE" || echo "$FAKE_COMMIT_MESSAGE" > "$1"
> +	test -z "$FAKE_COMMIT_AMEND" || echo "$FAKE_COMMIT_AMEND" >> "$1"
> +	exit
> +	;;
> +esac
> +test -z "$EXPECT_COUNT" ||
> +	test "$EXPECT_COUNT" = $(sed -e '/^#/d' -e '/^$/d' < "$1" | wc -l) ||
> +	exit
> +test -z "$FAKE_LINES" && exit
> +grep -v '^#' < "$1" > "$1".tmp
> +rm -f "$1"
> +cat "$1".tmp
> +action=pick
> +for line in $FAKE_LINES; do
> +	case $line in
> +	squash|edit)
> +		action="$line";;
> +	*)
> +		echo sed -n "${line}s/^pick/$action/p"
> +		sed -n "${line}p" < "$1".tmp
> +		sed -n "${line}s/^pick/$action/p" < "$1".tmp >> "$1"

I looked at the output from this and wondered what these "sed -n" shown in the "-v" output were about last night. I do think it is a good idea to show what edit was done to the insn stream, but I suspect it may be easier to read the output if you did this instead:

> +		sed -n "${line}p" < "$1".tmp
> +		sed -n "${line}s/^pick/$action/p" < "$1".tmp >> "$1"
> +		sed -n "${line}s/^pick/$action/p" < "$1".tmp