[PATCH V3] extdiff: add --per-file and --confirm options

Yuya Nishihara yuya at tcha.org
Sat Jan 26 02:53:47 EST 2019


On Sat, 19 Jan 2019 05:12:36 -0800, Ludovic Chabant wrote:
> # HG changeset patch
> # User Ludovic Chabant <ludovic at chabant.com>
> # Date 1547180577 28800
> #      Thu Jan 10 20:22:57 2019 -0800
> # Node ID 5686d56d5a2cebaa504383b270e359156352090f
> # Parent  ef0e2f7224358c32b0f62b13e83e89ba2399c8cf
> extdiff: add --per-file and --confirm options

> +def _checktool(ui, tool):
> +    if not procutil.gui():
> +        needsgui = (ui.config('diff-tools', tool+'.gui') or
> +                    ui.config('merge-tools', tool+'.gui'))
> +        if needsgui:
> +            ui.warn(_("tool %s requires a GUI\n") % tool)
> +            return False
> +    return True

Can you split this to a separate patch? It'll need a bit more churn, and
we wouldn't want to make it a blocker of the --per-file/--confirm feature.

What I pointed out last time was that we shouldn't spawn non-GUI tools
asynchronously. You can see the problem by the following command on Unix.

  $ hg --config extdiff.vimdiff= vimdiff -c. --per-file

>      def __call__(self, ui, repo, *pats, **opts):
> +        if not _checktool(ui, self._cmd):
> +            return 2

Nit: just raise error.Abort().

And perhaps, it's too late to compare ui.config value based on self._cmd
here. If self._cmd is the key of [diff-tools] for example, we shouldn't look
for [merge-tools].


More information about the Mercurial-devel mailing list