From: Mark Jamsek <mark@jamsek.com>
Subject: Re: fix histedit -m with filemode-only changes
To: Omar Polo <op@omarpolo.com>
Cc: Stefan Sperling <stsp@stsp.name>, gameoftrees@openbsd.org
Date: Sun, 29 Jan 2023 20:15:33 +1100

On 23-01-29 09:51AM, 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@

Yes, agreed. EDITOR fails for me, too, but VISUAL works

ok for me too with this change

> > [...]
> > +	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
> 
> > +		got histedit -m > $testroot/stdout)
> > +
> > +	local new_commit1=`git_show_head $testroot/repo`
> > +	local new_author_time1=`git_show_author_time $testroot/repo`
> > +
> > +	local short_old_commit1=`trim_obj_id 28 $old_commit1`
> > +	local short_new_commit1=`trim_obj_id 28 $new_commit1`
> > +
> > +	echo "G  alpha" > $testroot/stdout.expected
> > +	echo "$short_old_commit1 -> $short_new_commit1: set executable bit on alpha" \
> > +		>> $testroot/stdout.expected
> > +	echo "Switching work tree to refs/heads/master" \
> > +		>> $testroot/stdout.expected
> > +
> > +	cmp -s $testroot/stdout.expected $testroot/stdout
> > +	ret=$?
> > +	if [ $ret -ne 0 ]; then
> > +		diff -u $testroot/stdout.expected $testroot/stdout
> > +		test_done "$testroot" "$ret"
> > +		return 1
> > +	fi
> > +
> > +	echo "alpha" > $testroot/content.expected
> > +	cat $testroot/wt/alpha > $testroot/content
> 
> just for curiosity, why not cp(1) or just comparing content with
> alpha?
> 
> > +	cmp -s $testroot/content.expected $testroot/content
> > +	ret=$?
> > +	if [ $ret -ne 0 ]; then
> > +		diff -u $testroot/content.expected $testroot/content
> > +		test_done "$testroot" "$ret"
> > +		return 1
> > +	fi
> 

-- 
Mark Jamsek <fnc.bsdbox.org>
GPG: F2FF 13DE 6A06 C471 CA80  E6E2 2930 DC66 86EE CF68