On Sun, Jan 29, 2023 at 09:51:52AM +0100, Omar Polo wrote:
> On 2023/01/28 21:57:39 +0100, Stefan Sperling <stsp@stsp.name> wrote:
> > This fixes an issue I stumbled across receently.
> > 
> > The bug is in merge_file_cb() so it probably affects more than
> > just histedit -m. But histedit -m provides a nice test case.
> > 
> > ok?
> 
> with one nit below, ok op@
> 
> > [...]
> > +	cat > $testroot/editor.sh <<EOF
> > +#!/bin/sh
> > +sed -i 's/ x bit / executable bit /' "\$1"
> > +EOF
> > +
> > +	chmod +x $testroot/editor.sh
> > +
> > +	(cd $testroot/wt && env EDITOR="$testroot/editor.sh" \
>                                 ^^^^^^
> 
> should be VISUAL, not EDITOR.
> 
> got/got.c:get_editor prefers VISUAL over EDITOR, so this fails for me
> since I have VISUAL=mg.
> 
> : test_histedit_mesg_filemode_change panic: standard input and output must be a terminal

Thanks, I have fixed this in a follow-up commit.
> > +	echo "alpha" > $testroot/content.expected
> > +	cat $testroot/wt/alpha > $testroot/content
> 
> just for curiosity, why not cp(1) or just comparing content with
> alpha?

Sure, I've made this tweak, too.

The reason for using content.expected is a result of plenty of
copy-pasting between tests. It never really bothered me. In case
of a test failure it could be useful to have content.expected and
content next to each other in the temp dir to compare. But on the
other hand one needs to study the test case code anyway to figure
out the problem, so perhaps it's not all that useful.