Hi brian,
On Fri, 26 Jul 2019, brian m. carlson wrote:
> On 2019-07-25 at 10:11:22, Johannes Schindelin via GitGitGadget wrote:
> > From: Johannes Schindelin <johannes.schindelin@xxxxxx>
> >
> > The format of the todo list is quite a bit different in the
> > `--rebase-merges` mode; Let's prepare the fake editor to handle those
> > todo lists properly, too.
> >
> > Signed-off-by: Johannes Schindelin <johannes.schindelin@xxxxxx>
> > ---
> > t/lib-rebase.sh | 8 ++++----
> > 1 file changed, 4 insertions(+), 4 deletions(-)
> >
> > diff --git a/t/lib-rebase.sh b/t/lib-rebase.sh
> > index 7ea30e5006..662a958575 100644
> > --- a/t/lib-rebase.sh
> > +++ b/t/lib-rebase.sh
> > @@ -44,10 +44,10 @@ set_fake_editor () {
> > rm -f "$1"
> > echo 'rebase -i script before editing:'
> > cat "$1".tmp
> > - action=pick
> > + action=\&
>
> So we set action to "&" so we can use it as the result in the sed
> expression below…
>
> > for line in $FAKE_LINES; do
> > case $line in
> > - pick|p|squash|s|fixup|f|edit|e|reword|r|drop|d)
> > + pick|p|squash|s|fixup|f|edit|e|reword|r|drop|d|label|l|reset|r|merge|m)
> > action="$line";;
> > exec_*|x_*|break|b)
> > echo "$line" | sed 's/_/ /g' >> "$1";;
> > @@ -61,8 +61,8 @@ set_fake_editor () {
> > echo "$action XXXXXXX False commit" >> "$1"
>
> but then here it doesn't look like "&" is a thing we'd want to use. Is
> there something I'm missing about this particular case?
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:
-- 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
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"
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.
Ciao,
Dscho
