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

Yuya Nishihara yuya at tcha.org
Thu Jan 31 07:19:03 EST 2019


On Wed, 30 Jan 2019 08:57:07 -0800, Ludovic Chabant wrote:
> > My point was that it could be addressed by checking the gui flag (or adding
> > new dedicated flag to [diff/merge-tools].) It's a tool property whether
> > multiple processes can be spawned or not.
> 
> After testing a bit, it turns out it's actually a bit more complicated than I anticipated. Consider this:
> 
> [extdiff]
> blah =
> 
> [diff-tools]
> blah.gui = True
> 
> [merge-tools]
> blah.diffargs = -a -b -c
> 
> The extdiff command will find the options to use on the command line from the [merge-tools] sections. Does this mean we ignore the "gui = True" from [diff-tools] even though that's clearly intended for *diff commands to use?

I think diff-tools.blah.gui should be ignored in this case.

> It could also be the opposite, where the "diffargs" are found in [diff-tools] and "gui" is set in [merge-tools].
> 
> And there's the case where no "diffargs" are provided anywhere, in which case the "gui" setting might be taken from wherever we find, leading to the problem where, if you have:
> 
> [extdiff]
> blah =
> 
> [merge-tools]
> blah.gui = True
> 
> ...then it will consider "blah" to be a GUI tool... until you add a [diff-tools] section with "blah.diffargs" and suddenly "blah" isn't considered a GUI anymore because the presence of a "diffargs" setting there made us stop looking at the [merge-tools] section.
> 
> I think the best we can do might be to look for "gui" in [merge-tools] _only_ if "diffargs" is also there, and otherwise _only_ look for "gui" in [diff-tools]...  ideas?

My idea is basically as follows:

  if cmd and opts found in extdiff:
      # it must be a user-defined tool
      look for extdiff.gui.<cmd>
  elif cmd.diffargs found in diff-tools:
      # perhaps it's a stock tool template defined in [diff-tools] (less common)
      look for diff-tools.<cmd>.gui
  elif cmd.diffargs found in merge-tools:
      # perhaps it's a stock tool template defined in [merge-tools] (common)
      look for merge-tools.<cmd>.gui

> Unless I'm missing something, I feels like we're basically reaching the limits of the, ahem, rather organic spec of the extdiff command (there's not even a help topic on "config.diff-tools"). After we solve this, I would be totally ok working on some patches to make a core "hg difftool" command or something, with a more strict UX.

Nice. I think the core version can also be "hg extdiff" that supports -t/--tool
option. And the extdiff extension will wrap the core extdiff command to support
-p/--program and command aliases for backward compatibility.


More information about the Mercurial-devel mailing list