Re: [PATCH 10/12] t/lib-rebase: prepare for testing `git rebase --rebase-merges`

> On 2019-07-26 at 14:01:03, Johannes Schindelin wrote:
> > Actually, the part that uses it is not shown in the patch (one of the
> > many, many reasons why I try to discourage patch review and encourage
> > code review instead). The relevant section currently looks somewhat like
> > this:
>
> I feel like I may have communicated poorly earlier, so let me retry
> asking this in a different way.

> > -- snip --
> > set_fake_editor () {
> > 	write_script fake-editor.sh <<-\EOF
> > 	case "$1" in
> > 	*/COMMIT_EDITMSG)
> > 		test -z "$EXPECT_HEADER_COUNT" ||
> > 			test "$EXPECT_HEADER_COUNT" = "$(sed -n '1s/^# This is a combination of \(.*\) commits\./\1/p' < "$1")" ||
> > 			test "# # GETTEXT POISON #" = "$(sed -n '1p' < "$1")" ||
> > 			exit
> > 		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"
> > 	echo 'rebase -i script before editing:'
> > 	cat "$1".tmp
> > 	action=pick
>
> I believe you changed this line to "action=\&".
>
> > 	for line in $FAKE_LINES; do
> > 		case $line in
> > 		pick|p|squash|s|fixup|f|edit|e|reword|r|drop|d)
> > 			action="$line";;
> > 		exec_*|x_*|break|b)
> > 			echo "$line" | sed 's/_/ /g' >> "$1";;
> > 		"#")
> > 			echo '# comment' >> "$1";;
> > 		">")
> > 			echo >> "$1";;
> > 		bad)
> > 			action="badcmd";;
> > 		fakesha)
> > 			echo "$action XXXXXXX False commit" >> "$1"
>
> And my question was about this line.

>
> > 			action=pick;;
> > 		*)
> > 			sed -n "${line}s/^pick/$action/p" < "$1".tmp >> "$1"
> > 			action=pick;;
> > 		esac
> > 	done
> > 	echo 'rebase -i script after editing:'
> > 	cat "$1"
> > 	EOF
> >
> > 	test_set_editor "$(pwd)/fake-editor.sh"
> > }
> > -- snap --
> >
> > Most importantly, `action` is used here:
> >
> >                         sed -n "${line}s/^pick/$action/p" < "$1".tmp >> "$1"
> >
> > and I changed it to
> >
> > 			sed -n "${line}s/^[a-z][a-z]*/$action/p" < "$1".tmp >> "$1"
> >
> > In other words, rather than expecting the lines that are used by the
> > fake editor to start with `pick`, after this patch, the tests expect the
> > todo lists to start with a command consisting of lower-case ASCII
> > letters (which catches `pick`, of course, but also `label`, `reset` and
> > `merge`).
> >
> > After this patch, the fake editor also does not try to replace whatever
> > command it finds by `pick`, but it keeps it as-is instead.
>
> Right, that's how I read it, and that part I agree with. I think my
> question is this: in what case do we execute the "fakesha" case? Are we
> guaranteed that when we do, action isn't "&"? "&" seems fine for the
> right-hand side of a sed s-statement, but not as the beginning of a
> typical line in a sequencer file.