Thread (1 message) 1 message, 1 author, 2023-09-18

Re: [PATCH v5] merge-tree: add -X strategy option

From: Junio C Hamano <hidden>
Date: 2023-09-18 16:10:02

Phillip Wood [off-list ref] writes:
...
This is the right place to call parse_merge_opt() but I think we
should first check that the user has requested a real merge rather
than a trivial merge.

	if (xopts.nr && o.mode == MODE_TRIVIAL)
		die(_("--trivial-merge is incompatible with all other options"));

Otherwise if the user passes in invalid strategy option to a trivial
merge they'll get an error about an invalid strategy option rather
than being told --strategy-option is not supported with
--trivial-merge.
I presume that another issue with not having that compatibility
checking with "--trivial" would be when the user passes a valid
strategy option and it would be silently ignored.
Ideally there would be a preparatory patch that moves the switch
statement that is below the "if(o.use_stdin)" block up to this point
so we'd always have set o.mode before checking if it is a trivial
merge. (I think you'd to change the code slightly when it is moved to
add a check for o.use_stdin)
That sounds very good.

Thanks for a good "stepping back a bit" review.  Highly appreciated.
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help