Re: [PATCH v3 0/1] Fix scissors bug during merge conflict

From: Junio C Hamano <gitster@pobox.com>
Date: Nov 18, 2018, 06:54 UTC
Message-ID: <xmqqwopag9kb.fsf@gitster-ct.c.googlers.com>
In-Reply-To: <cover.1542496915.git.liu.denton@gmail.com>

Denton Liu <liu.denton@gmail.com> writes:

Show 20 quoted lines

>> I wonder if this is what you really meant to have instead:
>> 
>> >  		else if (cleanup_mode == COMMIT_MSG_CLEANUP_SCISSORS &&
>> > -			 whence == FROM_COMMIT)
>> > - 			wt_status_add_cut_line(s->fp);
>> > +			 whence == FROM_COMMIT) {
>> > +			 if (!merge_contains_scissors)
>> > +				wt_status_add_cut_line(s->fp);
>> > +		}
>> >  		else /* COMMIT_MSG_CLEANUP_SPACE, that is. */
>> >  			status_printf(s, GIT_COLOR_NORMAL,
>> 
>> That is, the only behaviour change in "merge contains scissors"
>> mode is to omit the cut line and nothing else.
>
> Thanks for catching this. I noticed that the originally behaviour is
> buggy too: in the case where we're merging a commit and scissors are
> printed from the `if (whence != FROM_COMMIT)` block, the original
> behaviour would drop us into the else (COMMIT_MSG_CLEANUP_SPACE)
> statement, which is undesired.

The original calls add-cut-line in the "whence != FROM_COMMIT" when cleanup_mode is CLEANUP_SCISSORS (and then in that block it also adds the message about committing a merge or cherry-pick). After that, the original does the three-arm if/else if/else cascade and we end up showing the "lines starting with # will be kept" message.

Yeah, you read the original correctly and I agree that in that block the right thing to do is not to say anything in CLEANUP_SCISSORS mode.

Thanks for carefully reading the code again.