Re: [PATCH v7 5/8] merge: cleanup messages like commit

Phillip Wood [off-list ref] writes:

quoted

-	if (0 < option_edit)
-		strbuf_commented_addf(&msg, _(merge_editor_comment), comment_line_char);
+	if (0 < option_edit) {
+		strbuf_addch(&msg, '\n');
+		if (cleanup_mode == COMMIT_MSG_CLEANUP_SCISSORS)
+			wt_status_append_cut_line(&msg);
+		else
+			strbuf_commented_addf(&msg, _(comment_line_explanation), comment_line_char);
+
+		strbuf_commented_addf(&msg, "\n");
+		strbuf_commented_addf(&msg, _(merge_editor_comment));

I think this has rearranged the message presented to the user so it
now reads

Lines starting with '#' will be ignored.
Please enter a commit message to explain why this merge is necessary,
especially if it merges an updated upstream into a topic branch.

An empty message aborts the commit.

To me it read better before, it would be a little more work but I
think it would be worth preserving the message (especially as this is
the message people will see unless they specify --cleanup=scissors)

That may be subjective, but unless the new message is vastly and
uncontroversially better (which I do not think it is, with your
objection), I agree that we should avoid churning the message.

quoted

@@ -168,6 +169,9 @@ static struct option pull_options[] = {
  	OPT_PASSTHRU(0, "edit", &opt_edit, NULL,
  		N_("edit message before committing"),
  		PARSE_OPT_NOARG),
+	OPT_PASSTHRU(0, "cleanup", &opt_cleanup, NULL,
+		N_("how to strip spaces and #comments from message"),
+		PARSE_OPT_NOARG),

cleanup needs to take an argument so PARSE_OPT_NOARG does not look
right. Also I think it would be bettor from the user's point of view
if the value of the argument was checked by pull before it does any
work rather otherwise if they pass in invalid value pull mostly runs
and then merge errors out at the end.

Both good points.