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