[PATCH 4 of 7 mergedriver] filemerge._picktool: only pick from nomerge tools for change/delete conflicts

[Martin von Zweigbergk]

On Thu, Nov 19, 2015 at 1:36 PM Siddharth Agarwal <sid0 at fb.com> wrote:

> # HG changeset patch
> # User Siddharth Agarwal <sid0 at fb.com>
> # Date 1447652415 28800
> #      Sun Nov 15 21:40:15 2015 -0800
> # Node ID 6100ebf277120db2ea3efb7fb7e7deb0c2585a60
> # Parent  696cd54b8958729b6b1763b19a159c3c97b7b72b
> # Available At http://42.netv6.net/sid0-wip/hg/
> #              hg pull http://42.netv6.net/sid0-wip/hg/ -r 6100ebf27712
> filemerge._picktool: only pick from nomerge tools for change/delete
> conflicts
>
> For --tool or HGMERGE, we could have either:
> (a) proceeded with the particular tool, then failed the merge.
> (b) chosen to prompt regardless.
>
> We're explicitly choosing (b) here, because it's effectively what we've
> been
> doing so far and helps maintain an easier-to-use interface.
>
> However, in future patches we're going to change the default selection from
> 'pick changed version' to 'leave unresolved'. That fixes most of the
> brokenness
> involved with choice (b).
>
> diff --git a/mercurial/filemerge.py b/mercurial/filemerge.py
> --- a/mercurial/filemerge.py
> +++ b/mercurial/filemerge.py
> @@ -120,8 +120,11 @@ def findexternaltool(ui, tool):
>      exe = _toolstr(ui, tool, "executable", tool)
>      return util.findexe(util.expandpath(exe))
>
> -def _picktool(repo, ui, path, binary, symlink):
> -    def check(tool, pat, symlink, binary):
> +def _picktool(repo, ui, path, binary, symlink, changedelete):
> +    def supportscd(tool):
> +        return tool in internals and internals[tool].mergetype == nomerge
> +
> +    def check(tool, pat, symlink, binary, changedelete):
>          tmsg = tool
>          if pat:
>              tmsg += " specified for " + pat
> @@ -134,6 +137,10 @@ def _picktool(repo, ui, path, binary, sy
>              ui.warn(_("tool %s can't handle symlinks\n") % tmsg)
>          elif binary and not _toolbool(ui, tool, "binary"):
>              ui.warn(_("tool %s can't handle binary\n") % tmsg)
> +        elif changedelete and not supportscd(tool):
> +            # the nomerge tools are the only tools that support
> change/delete
> +            # conflicts
> +            pass
>          elif not util.gui() and _toolbool(ui, tool, "gui"):
>              ui.warn(_("tool %s requires a GUI\n") % tmsg)
>          else:
> @@ -145,21 +152,27 @@ def _picktool(repo, ui, path, binary, sy
>      force = ui.config('ui', 'forcemerge')
>      if force:
>          toolpath = _findtool(ui, force)
> -        if toolpath:
> -            return (force, util.shellquote(toolpath))
> +        if not (changedelete and not supportscd(toolpath)):
> +            if toolpath:
> +                return (force, util.shellquote(toolpath))
> +            else:
> +                # mimic HGMERGE if given tool not found
> +                return (force, force)
>          else:
> -            # mimic HGMERGE if given tool not found
> -            return (force, force)
> +            return ":prompt", None
>
>      # HGMERGE takes next precedence
>      hgmerge = os.environ.get("HGMERGE")
>      if hgmerge:
> -        return (hgmerge, hgmerge)
> +        if not (changedelete and not supportscd(hgmerge)):
>

Not this patch's fault, but would it make sense to use the same name for
the tool path in all three cases instead of "tool", "toolpath" and
"hgmerge"?


> +            return (hgmerge, hgmerge)
> +        else:
> +            return ":prompt", None
>
>      # then patterns
>      for pat, tool in ui.configitems("merge-patterns"):
>          mf = match.match(repo.root, '', [pat])
> -        if mf(path) and check(tool, pat, symlink, False):
> +        if mf(path) and check(tool, pat, symlink, False, changedelete):
>              toolpath = _findtool(ui, tool)
>              return (tool, util.shellquote(toolpath))
>
> @@ -176,17 +189,19 @@ def _picktool(repo, ui, path, binary, sy
>      tools = sorted([(-p, t) for t, p in tools.items() if t not in
> disabled])
>      uimerge = ui.config("ui", "merge")
>      if uimerge:
> -        if uimerge not in names:
> +        # external tools defined in uimerge won't be able to handle
> +        # change/delete conflicts
> +        if uimerge not in names and not changedelete:
>              return (uimerge, uimerge)
>          tools.insert(0, (None, uimerge)) # highest priority
>      tools.append((None, "hgmerge")) # the old default, if found
>      for p, t in tools:
> -        if check(t, None, symlink, binary):
> +        if check(t, None, symlink, binary, changedelete):
>              toolpath = _findtool(ui, t)
>              return (t, util.shellquote(toolpath))
>
>      # internal merge or prompt as last resort
> -    if symlink or binary:
> +    if symlink or binary or changedelete:
>          return ":prompt", None
>      return ":merge", None
>
> @@ -535,7 +550,8 @@ def _filemerge(premerge, repo, mynode, o
>      fd = fcd.path()
>      binary = fcd.isbinary() or fco.isbinary() or fca.isbinary()
>      symlink = 'l' in fcd.flags() + fco.flags()
> -    tool, toolpath = _picktool(repo, ui, fd, binary, symlink)
> +    changedelete = fcd.isabsent() or fco.isabsent()
> +    tool, toolpath = _picktool(repo, ui, fd, binary, symlink,
> changedelete)
>      if tool in internals and tool.startswith('internal:'):
>          # normalize to new-style names (':merge' etc)
>          tool = tool[len('internal'):]
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at selenic.com
> https://selenic.com/mailman/listinfo/mercurial-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://selenic.com/pipermail/mercurial-devel/attachments/20151119/cbcb59ba/attachment.html>