Re: [PATCH v4 0/2] Fix scissors bug during merge conflict

From: Denton Liu <liu.denton@gmail.com>
Date: Nov 22, 2018, 01:10 UTC
Message-ID: <20181122011055.GA1584@archbookpro.localdomain>
In-Reply-To: <xmqqefbe7otv.fsf@gitster-ct.c.googlers.com>

On Wed, Nov 21, 2018 at 06:38:20PM +0900, Junio C Hamano wrote:

Show 7 quoted lines

> Denton Liu <liu.denton@gmail.com> writes:
> 
> > Changes since V3:
> > 	* Add patch to cleanup 'merge --squash c3 with c7' test in t7600
> > 	* Use test_i18ncmp instead of test_cmp to pass GETTEXT_POISON tests
> 
> Queued. Thanks, both.

I just realised that there is a slight problem with the proposed change. When we do a merge and there are no merge conflicts, at the end of the merge, we get dropped into an editor with this text:

	Merge branch 'master' into new

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

Note that in git-merge, the cleanup only removes commented lines and this cannot be configured to be scissors or whatever else. I think that the fact that it's not configurable isn't a problem; most hardcore commit message editing happens in git-commit anyway.

However, since we taught git-merge the --cleanup option, this might be misleading for the end-user since they would expect the MERGE_MSG to be cleaned up as specified.

I see two resolutions for this. We can either rename --cleanup more precisely so users won't be confused (perhaps something like --merge-conflict-scissors but a lot more snappy) or we can actually make git-merge respect the cleanup option and post-process the message according to the specified cleanup rule.

I would personally think the first option is better than the second but I'd like to hear your thoughts.

Thanks!