Code review comment for lp:~doxxx/bzr/mergetools

> I've made more changes according to vila's comments on
> https://code.launchpad.net/~doxxx/bzr/mergetools-commands/+merge/40924.
>
> I'm not 100% sure I've got the treatment of known merge tools right. One
> problem is that with my modifications to qconfig, it now shows all the user-
> defined and pre-defined merge tools, and if the user clicks Okay, it's going
> to save all of them as user-defined,

Exactly. It occurred to me that no config option defines a set_XXX variant, and this makes even more sense if you take into account that the *user* should decide in which config file/section he wants to define them.

So as I said, set_merge_tools() shouldn't be part of the Config API as it negates this feature (you can't provide a list of items to a single config object expecting it to split them across several config files, so we shouldn't even try to do that but instead provide APIs that deal with a single config option at a time). I'm also now convinced that you shouldn't provide set_default_merge_tool either, the only added value here is to check that the merge tool is valid. But again, since the config files can be updated manually, this control can be defeated and should be done by Config callers when they access the value.

All of this should be easier if known_merge_tools was a true registry methinks.

> which negates the usefulness of having
> the pre-defined merge tools as vila described in the other mp. qconfig needs
> to be aware of the difference between a user-defined and a pre-defined merge
> tool. But at the same time, I don't want to make it hard for clients of the
> mergetools module and its functions on Config to use user-defined or pre-
> defined mergetools.

Right, I share the feeling, and I'll add that since we can't guarantee stability here we'd better:
- provide a simpler mean based on get_user_option/set_user_option,
- provide more advanced features (is_available) in the known_merge_tools registry,
- implement a minimal set of features in qconfig (but this is outside the scope of this proposal and bzrlib ought be plugin-neutral anyway :), so let qconfig handle the list of possible merge tools and let's keep bzrlib implementation minimal. We'll provide better support when the infrastructure will be in place.