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

[Junio C Hamano <gitster@pobox.com>]

Phillip Wood <phillip.wood123@gmail.com> 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.