commit 8ea72c47a2077c52fad27872183808829d76c4d8
from: Omar Polo <op@omarpolo.com>
via: Thomas Adam <thomas@xteddy.org>
date: Tue Jul 12 22:07:11 2022 UTC

histedit: make sure mesg is only used after pick or edit

It doesn't really make sense to use mesg after a fold or drop, or after
another mesg.  it currently "works" as intended, but the behaviour is
confusing and not useful, better abort the operation as it's probably
not what the user intended.

Suggested by and ok stsp@

commit - ffcfe569470b9a24db65cf13a33fb059d17752a2
commit + 8ea72c47a2077c52fad27872183808829d76c4d8
blob - beb2b8ca9f072ebfef556186c6c61d244bd11d69
blob + 8f34c674327a0a9472cf39a0e7aa5110ac9cdf85
--- got/got.1
+++ got/got.1
@@ -2113,6 +2113,7 @@ argument provides a new single-line log message to use
 If the
 .Ar log-message
 argument is omitted, open an editor where a new log message can be written.
+Can only be used immediately after a pick or edit command.
 .El
 .Pp
 Every commit in the history being edited must be mentioned in the script.
blob - b574893efe6b88b89bfc49b4d55118a0f4506e26
blob + e5e63b466b8040e49029c5424d9f0f62f49832b3
--- got/got.c
+++ got/got.c
@@ -10698,7 +10698,7 @@ histedit_parse_list(struct got_histedit_list *histedit
 	char *line = NULL, *p, *end;
 	size_t i, size;
 	ssize_t len;
-	int lineno = 0;
+	int lineno = 0, lastcmd = -1;
 	const struct got_histedit_cmd *cmd;
 	struct got_object_id *commit_id = NULL;
 	struct got_histedit_list_entry *hle = NULL;
@@ -10742,7 +10742,8 @@ histedit_parse_list(struct got_histedit_list *histedit
 		while (isspace((unsigned char)p[0]))
 			p++;
 		if (cmd->code == GOT_HISTEDIT_MESG) {
-			if (hle == NULL || hle->logmsg != NULL) {
+			if (lastcmd != GOT_HISTEDIT_PICK &&
+			    lastcmd != GOT_HISTEDIT_EDIT) {
 				err = got_error(GOT_ERR_HISTEDIT_CMD);
 				break;
 			}
@@ -10759,6 +10760,7 @@ histedit_parse_list(struct got_histedit_list *histedit
 			}
 			free(line);
 			line = NULL;
+			lastcmd = cmd->code;
 			continue;
 		} else {
 			end = p;
@@ -10785,6 +10787,7 @@ histedit_parse_list(struct got_histedit_list *histedit
 		free(line);
 		line = NULL;
 		TAILQ_INSERT_TAIL(histedit_cmds, hle, entry);
+		lastcmd = cmd->code;
 	}
 
 	free(line);
blob - 2bd46dbf68f2e2ca0b6dcfc0751ef02b5975ab65
blob + 6e2a13a97a28b755c382b6b238a171507060d184
--- regress/cmdline/histedit.sh
+++ regress/cmdline/histedit.sh
@@ -744,7 +744,6 @@ test_histedit_fold_last_commit() {
 
 	echo "pick $old_commit1" > $testroot/histedit-script
 	echo "fold $old_commit2" >> $testroot/histedit-script
-	echo "mesg committing folded changes" >> $testroot/histedit-script
 
 	(cd $testroot/wt && got histedit -F $testroot/histedit-script \
 		> $testroot/stdout 2> $testroot/stderr)
@@ -1980,7 +1979,126 @@ EOF
 
 	test_done "$testroot" $ret
 }
+
+test_histedit_mesg_invalid() {
+	local testroot=`test_init mesg_invalid`
+
+	local orig_commit=`git_show_head $testroot/repo`
+
+	echo "modified alpha on master" > $testroot/repo/alpha
+	(cd $testroot/repo && git rm -q beta)
+	echo "new file on master" > $testroot/repo/epsilon/new
+	(cd $testroot/repo && git add epsilon/new)
+	git_commit $testroot/repo -m 'committing changes'
+	local old_commit1=`git_show_head $testroot/repo`
+
+	echo "modified zeta on master" > $testroot/repo/epsilon/zeta
+	git_commit $testroot/repo -m 'committing to zeto on master'
+	local old_commit2=`git_show_head $testroot/repo`
+
+	got checkout -c $orig_commit $testroot/repo $testroot/wt > /dev/null
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		test_done "$testroot" $ret
+	fi
+
+	# try with a leading mesg
+
+	echo "mesg something something" > $testroot/histedit-script
+	echo "pick $old_commit1" >> $testroot/histedit-script
+	echo "pick $old_commit2" >> $testroot/histedit-script
+
+	(cd $testroot/wt && got histedit -F $testroot/histedit-script \
+		> $testroot/stdout 2> $testroot/stderr)
+	ret=$?
+	if [ $ret -eq 0 ]; then
+		echo "histedit succeeded unexpectedly" >&2
+		test_done "$testroot" 1
+		return 1
+	fi
 
+	echo "got: bad histedit command" > $testroot/stderr.expected
+	cmp -s $testroot/stderr.expected $testroot/stderr
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		diff -u $testroot/stderr.expected $testroot/stderr
+		test_done "$testroot" $ret
+		return 1
+	fi
+
+	# try again with mesg -> mesg
+
+	echo "pick $old_commit1" > $testroot/histedit-script
+	echo "mesg something something" >> $testroot/histedit-script
+	echo "mesg something something else" >> $testroot/histedit-script
+	echo "pick $old_commit2" >> $testroot/histedit-script
+
+	(cd $testroot/wt && got histedit -F $testroot/histedit-script \
+		> $testroot/stdout 2> $testroot/stderr)
+	ret=$?
+	if [ $ret -eq 0 ]; then
+		echo "histedit succeeded unexpectedly" >&2
+		test_done "$testroot" 1
+		return 1
+	fi
+
+	echo "got: bad histedit command" > $testroot/stderr.expected
+	cmp -s $testroot/stderr.expected $testroot/stderr
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		diff -u $testroot/stderr.expected $testroot/stderr
+		test_done "$testroot" $ret
+		return 1
+	fi
+
+	# try again with drop -> mesg
+
+	echo "drop $old_commit1" > $testroot/histedit-script
+	echo "mesg something something" >> $testroot/histedit-script
+	echo "pick $old_commit2" >> $testroot/histedit-script
+
+	(cd $testroot/wt && got histedit -F $testroot/histedit-script \
+		> $testroot/stdout 2> $testroot/stderr)
+	ret=$?
+	if [ $ret -eq 0 ]; then
+		echo "histedit succeeded unexpectedly" >&2
+		test_done "$testroot" 1
+		return 1
+	fi
+
+	echo "got: bad histedit command" > $testroot/stderr.expected
+	cmp -s $testroot/stderr.expected $testroot/stderr
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		diff -u $testroot/stderr.expected $testroot/stderr
+		test_done "$testroot" $ret
+		return 1
+	fi
+
+	# try again with fold -> mesg
+
+	echo "fold $old_commit1" > $testroot/histedit-script
+	echo "mesg something something" >> $testroot/histedit-script
+	echo "pick $old_commit2" >> $testroot/histedit-script
+
+	(cd $testroot/wt && got histedit -F $testroot/histedit-script \
+		> $testroot/stdout 2> $testroot/stderr)
+	ret=$?
+	if [ $ret -eq 0 ]; then
+		echo "histedit succeeded unexpectedly" >&2
+		test_done "$testroot" 1
+		return 1
+	fi
+
+	echo "got: bad histedit command" > $testroot/stderr.expected
+	cmp -s $testroot/stderr.expected $testroot/stderr
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		diff -u $testroot/stderr.expected $testroot/stderr
+	fi
+	test_done "$testroot" $ret
+}
+
 test_parseargs "$@"
 run_test test_histedit_no_op
 run_test test_histedit_swap
@@ -2001,3 +2119,4 @@ run_test test_histedit_fold_only
 run_test test_histedit_fold_only_empty_logmsg
 run_test test_histedit_edit_only
 run_test test_histedit_prepend_line
+run_test test_histedit_mesg_invalid