commit f1e5aff40c5f42ab6fd83a2fb41ca0d10b3b21c9
from: Omar Polo <op@omarpolo.com>
date: Tue Jul 12 17:56:19 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 - 67d6e2c2d443318cc8e4ce198f9d9cb60c769093
commit + f1e5aff40c5f42ab6fd83a2fb41ca0d10b3b21c9
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 - 77ee3290c2b299ba86499c5b196115d3f54e96b6
blob + b7c305dd8fba01e7b8b4d08eae548d3f7c53246c
--- got/got.c
+++ got/got.c
@@ -10699,7 +10699,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;
@@ -10743,7 +10743,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;
 			}
@@ -10760,6 +10761,7 @@ histedit_parse_list(struct got_histedit_list *histedit
 			}
 			free(line);
 			line = NULL;
+			lastcmd = cmd->code;
 			continue;
 		} else {
 			end = p;
@@ -10786,6 +10788,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 - b15d00ef680f545b9c19d914b7c9d87cf23b1c8f
blob + e59bd1c0fdd538478d022c0c26e6dff857aed19a
--- regress/cmdline/histedit.sh
+++ regress/cmdline/histedit.sh
@@ -740,7 +740,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)
@@ -1972,7 +1971,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
@@ -1993,3 +2111,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