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

[Junio C Hamano <gitster@pobox.com>]

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

> 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".

> 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 ;-)

> +	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