[PATCH 2 of 2 V2] extdiff: add --mode option

Yuya Nishihara yuya at tcha.org
Sat Jan 12 01:47:58 EST 2019


On Thu, 10 Jan 2019 21:37:21 -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 dfda7867497e5609a03727508dc0da2cab3218a3
> # Parent  ef0e2f7224358c32b0f62b13e83e89ba2399c8cf
> extdiff: add --mode option

> +            proc = _systemdetached(curcmdline, cwd=tmproot)
> +            waitprocs.append(proc)
> +
> +    if waitprocs:
> +        with ui.timeblockedsection('extdiff'):
> +            for proc in waitprocs:
> +                proc.wait()

Maybe need to check the .gui flag of the tool. Otherwise the console
would be messed up if the tool wasn't a GUI program.

> +    :d (or nothing):    Runs the external program once and passes the
> +                        names of two directories to compare. Each
> +                        directory contains snapshots of the files before
> +                        and after the given revisions. This is the
> +                        default mode.
> +    :f:                 Runs the external program once for each file,
> +                        passing the path to the snapshots.
> +    :p:                 Modifies the 'f' mode by prompting the user
> +                        before running the external program for each
> +                        file.

(from the V1 thread)

On Thu, 10 Jan 2019 16:03:00 -0800, Ludovic Chabant wrote:
> > I don't think "--mode <char>" is good ui. I don't have nice idea, but maybe
> > it can be split into two flags (e.g. --per-file and --confirm/--interactive)?
> > 
> > And the config option can specify if the tool support directory diff, for
> > example. If the tool doesn't support directory diff, and if it's gui, spawn
> > processes per file by default.
> 
> I was actually debating with myself between the 2 ways of specifying the diff mode, and eventually I settled on the --mode option. Here's the rationale (although I can be easily convinced to use the alternative):
> 
> - A --mode option is easier to later extend to more ways to diff things, like for example a mode where, when you pass a revision range, it will diff each revision one by one (invoking the external program N times if there are N revisions in the range). Different flags that may or may not be compatible between each other are more confusing IMHO.

Well, I think it will produce 8 modes,
dir-or-file x revpair-or-revrage x prompt-or-not.

And we can even get rid of the dir-or-file mode from command flags by
registering two tools, one for dir-diff and the other for per-file-diff.
I don't know if that is a better idea, but we'll never use the per-file
mode if the tool had a decent support for directory diff. So it's a tool-
specific property at some level.

> - The --confirm option doesn't do anything if you don't also pass --per-file...

A prompt could be shown for the directory diff. I don't think it's useful
right now, but if we had a per-revision-diff mode, it would make some sense
to show prompt to skip some revisions.

> I don't know if there are other aspects of the hg cli that have this? Now I realize that I also fell short of that downside since "--mode p" doesn't do anything either, but the point is that if we want something to represent "per file" and "per file with prompt", 2 modes do that better than 2 flags (and I could change my patch to have "p" be a mode of its own instead of modifying the "f" mode).


More information about the Mercurial-devel mailing list