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

On 11/25/2010 5:19 AM, Vincent Ladeuil wrote:
> Making --detect or --list mandatory sounds weird though. --list
> should be default but doesn't it duplicate 'bzr config
> bzr.mergetools' ?
> Or rather 'bzr config *mergetool*' to also get default_mergetool.

Making --list the default operation sounds like a good idea. Yes, this
is duplicating functionality from `bzr config`. Would you rather have a
single command called 'detect-merge-tools'? I think it's nice to have a
pretty-printed list of merge tools.

> And then... why not use bzr.mergetools.default and forbid 'default'
> as a valid merge tool name in your other proposal ?

I suppose I could. :)

> It would be nice to mention that --detect *also* set the mergetools
> in the the config (which one ?).
>
> Which one is the default in that case ? None ?

None, it doesn't set a default since the command-line doesn't need a
default, you always have to specify a name when you want to use a merge
tool with merge or remerge. The default only applies to qbzr and there
you have qconfig to set the default.

> If that's the case, this means the user must specify which merge tool
> he want to use no ?

Correct.

> Since you're providing access to pre-defined merge tools as soon as
> you have detected them, why not go a step further and make this
> '--detect' step optional and provides the default command-line for,
> say kdiff3, if the user try to use --resolve-using kdiff3 ?

Yes, I could try that. With an appropriate message to the effect of
"auto-detecting kdiff3 on PATH"? Or is that unnecessary?