Phillip Wood <phillip.wood123@xxxxxxxxx> writes:
>> - 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.
>> @@ -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.
