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

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

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