Patchwork update: introduce --tool for controlling the merge tool

login
register
mail settings
Submitter Mads Kiilerich
Date May 18, 2014, 11:53 p.m.
Message ID <1d24b3e5efdbb7f972f5.1400457219@localhost.localdomain>
Download mbox | patch
Permalink /patch/4795/
State Accepted
Commit 61151f429a5f93bafe184890e19a0399a690a2b8
Headers show

Comments

Mads Kiilerich - May 18, 2014, 11:53 p.m.
# HG changeset patch
# User Mads Kiilerich <madski@unity3d.com>
# Date 1400457214 -7200
#      Mon May 19 01:53:34 2014 +0200
# Node ID 1d24b3e5efdbb7f972f5dfe862b334cf4a64e5e4
# Parent  883e268cb860c0ea2eb0faa94114e11c3a4a3893
update: introduce --tool for controlling the merge tool

Update is a kind of merge and may also need a merge tool and should have the
options described in the merge-tools help.
Siddharth Agarwal - May 19, 2014, 4:59 p.m.
On 05/18/2014 04:53 PM, Mads Kiilerich wrote:
> # HG changeset patch
> # User Mads Kiilerich <madski@unity3d.com>
> # Date 1400457214 -7200
> #      Mon May 19 01:53:34 2014 +0200
> # Node ID 1d24b3e5efdbb7f972f5dfe862b334cf4a64e5e4
> # Parent  883e268cb860c0ea2eb0faa94114e11c3a4a3893
> update: introduce --tool for controlling the merge tool

LGTM

>
> Update is a kind of merge and may also need a merge tool and should have the
> options described in the merge-tools help.
>
> diff --git a/mercurial/commands.py b/mercurial/commands.py
> --- a/mercurial/commands.py
> +++ b/mercurial/commands.py
> @@ -5807,9 +5807,11 @@ def unbundle(ui, repo, fname1, *fnames,
>       ('c', 'check', None,
>        _('update across branches if no uncommitted changes')),
>       ('d', 'date', '', _('tipmost revision matching date'), _('DATE')),
> -    ('r', 'rev', '', _('revision'), _('REV'))],
> +    ('r', 'rev', '', _('revision'), _('REV'))
> +     ] + mergetoolopts,
>       _('[-c] [-C] [-d DATE] [[-r] REV]'))
> -def update(ui, repo, node=None, rev=None, clean=False, date=None, check=False):
> +def update(ui, repo, node=None, rev=None, clean=False, date=None, check=False,
> +           tool=None):
>       """update working directory (or switch revisions)
>   
>       Update the repository's working directory to the specified
> @@ -5890,6 +5892,8 @@ def update(ui, repo, node=None, rev=None
>               rev = repo[repo[None].branch()].rev()
>           mergemod._checkunknown(repo, repo[None], repo[rev])
>   
> +    repo.ui.setconfig('ui', 'forcemerge', tool, 'update')
> +
>       if clean:
>           ret = hg.clean(repo, rev)
>       else:
> diff --git a/tests/test-completion.t b/tests/test-completion.t
> --- a/tests/test-completion.t
> +++ b/tests/test-completion.t
> @@ -212,7 +212,7 @@ Show all commands + options
>     serve: accesslog, daemon, daemon-pipefds, errorlog, port, address, prefix, name, web-conf, webdir-conf, pid-file, stdio, cmdserver, templates, style, ipv6, certificate
>     status: all, modified, added, removed, deleted, clean, unknown, ignored, no-status, copies, print0, rev, change, include, exclude, subrepos
>     summary: remote
> -  update: clean, check, date, rev
> +  update: clean, check, date, rev, tool
>     addremove: similarity, include, exclude, dry-run
>     archive: no-decode, prefix, rev, type, subrepos, include, exclude
>     backout: merge, parent, rev, tool, include, exclude, message, logfile, date, user
> diff --git a/tests/test-merge-tools.t b/tests/test-merge-tools.t
> --- a/tests/test-merge-tools.t
> +++ b/tests/test-merge-tools.t
> @@ -587,6 +587,54 @@ HGMERGE specifies internal:other but is
>   
>     $ unset HGMERGE # make sure HGMERGE doesn't interfere with remaining tests
>   
> +update is a merge ...
> +
> +  $ beforemerge
> +  [merge-tools]
> +  false.whatever=
> +  true.priority=1
> +  true.executable=cat
> +  # hg update -C 1
> +  $ hg debugsetparent 0
> +  $ hg update -r 2
> +  merging f
> +  revision 1
> +  space
> +  revision 0
> +  space
> +  revision 2
> +  space
> +  0 files updated, 1 files merged, 0 files removed, 0 files unresolved
> +  $ aftermerge
> +  # cat f
> +  revision 1
> +  space
> +  # hg stat
> +  M f
> +
> +update should also have --tool
> +
> +  $ beforemerge
> +  [merge-tools]
> +  false.whatever=
> +  true.priority=1
> +  true.executable=cat
> +  # hg update -C 1
> +  $ hg debugsetparent 0
> +  $ hg update -r 2 --tool false
> +  merging f
> +  merging f failed!
> +  0 files updated, 0 files merged, 0 files removed, 1 files unresolved
> +  use 'hg resolve' to retry unresolved file merges
> +  [1]
> +  $ aftermerge
> +  # cat f
> +  revision 1
> +  space
> +  # hg stat
> +  M f
> +  ? f.orig
> +
>   Default is silent simplemerge:
>   
>     $ beforemerge
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
Augie Fackler - May 26, 2014, 4:31 p.m.
On Mon, May 19, 2014 at 01:53:39AM +0200, Mads Kiilerich wrote:
> # HG changeset patch
> # User Mads Kiilerich <madski@unity3d.com>
> # Date 1400457214 -7200
> #      Mon May 19 01:53:34 2014 +0200
> # Node ID 1d24b3e5efdbb7f972f5dfe862b334cf4a64e5e4
> # Parent  883e268cb860c0ea2eb0faa94114e11c3a4a3893
> update: introduce --tool for controlling the merge tool

bikeshed: could this be --merge-tool instead of --tool? The reason I
ask is that it's a non-obvious flag name when it's embedded on 'hg
update'.

>
> Update is a kind of merge and may also need a merge tool and should have the
> options described in the merge-tools help.
>
> diff --git a/mercurial/commands.py b/mercurial/commands.py
> --- a/mercurial/commands.py
> +++ b/mercurial/commands.py
> @@ -5807,9 +5807,11 @@ def unbundle(ui, repo, fname1, *fnames,
>      ('c', 'check', None,
>       _('update across branches if no uncommitted changes')),
>      ('d', 'date', '', _('tipmost revision matching date'), _('DATE')),
> -    ('r', 'rev', '', _('revision'), _('REV'))],
> +    ('r', 'rev', '', _('revision'), _('REV'))
> +     ] + mergetoolopts,
>      _('[-c] [-C] [-d DATE] [[-r] REV]'))
> -def update(ui, repo, node=None, rev=None, clean=False, date=None, check=False):
> +def update(ui, repo, node=None, rev=None, clean=False, date=None, check=False,
> +           tool=None):
>      """update working directory (or switch revisions)
>
>      Update the repository's working directory to the specified
> @@ -5890,6 +5892,8 @@ def update(ui, repo, node=None, rev=None
>              rev = repo[repo[None].branch()].rev()
>          mergemod._checkunknown(repo, repo[None], repo[rev])
>
> +    repo.ui.setconfig('ui', 'forcemerge', tool, 'update')
> +
>      if clean:
>          ret = hg.clean(repo, rev)
>      else:
> diff --git a/tests/test-completion.t b/tests/test-completion.t
> --- a/tests/test-completion.t
> +++ b/tests/test-completion.t
> @@ -212,7 +212,7 @@ Show all commands + options
>    serve: accesslog, daemon, daemon-pipefds, errorlog, port, address, prefix, name, web-conf, webdir-conf, pid-file, stdio, cmdserver, templates, style, ipv6, certificate
>    status: all, modified, added, removed, deleted, clean, unknown, ignored, no-status, copies, print0, rev, change, include, exclude, subrepos
>    summary: remote
> -  update: clean, check, date, rev
> +  update: clean, check, date, rev, tool
>    addremove: similarity, include, exclude, dry-run
>    archive: no-decode, prefix, rev, type, subrepos, include, exclude
>    backout: merge, parent, rev, tool, include, exclude, message, logfile, date, user
> diff --git a/tests/test-merge-tools.t b/tests/test-merge-tools.t
> --- a/tests/test-merge-tools.t
> +++ b/tests/test-merge-tools.t
> @@ -587,6 +587,54 @@ HGMERGE specifies internal:other but is
>
>    $ unset HGMERGE # make sure HGMERGE doesn't interfere with remaining tests
>
> +update is a merge ...
> +
> +  $ beforemerge
> +  [merge-tools]
> +  false.whatever=
> +  true.priority=1
> +  true.executable=cat
> +  # hg update -C 1
> +  $ hg debugsetparent 0
> +  $ hg update -r 2
> +  merging f
> +  revision 1
> +  space
> +  revision 0
> +  space
> +  revision 2
> +  space
> +  0 files updated, 1 files merged, 0 files removed, 0 files unresolved
> +  $ aftermerge
> +  # cat f
> +  revision 1
> +  space
> +  # hg stat
> +  M f
> +
> +update should also have --tool
> +
> +  $ beforemerge
> +  [merge-tools]
> +  false.whatever=
> +  true.priority=1
> +  true.executable=cat
> +  # hg update -C 1
> +  $ hg debugsetparent 0
> +  $ hg update -r 2 --tool false
> +  merging f
> +  merging f failed!
> +  0 files updated, 0 files merged, 0 files removed, 1 files unresolved
> +  use 'hg resolve' to retry unresolved file merges
> +  [1]
> +  $ aftermerge
> +  # cat f
> +  revision 1
> +  space
> +  # hg stat
> +  M f
> +  ? f.orig
> +
>  Default is silent simplemerge:
>
>    $ beforemerge
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
Mads Kiilerich - May 26, 2014, 4:50 p.m.
On 05/26/2014 06:31 PM, Augie Fackler wrote:
> On Mon, May 19, 2014 at 01:53:39AM +0200, Mads Kiilerich wrote:
>> # HG changeset patch
>> # User Mads Kiilerich <madski@unity3d.com>
>> # Date 1400457214 -7200
>> #      Mon May 19 01:53:34 2014 +0200
>> # Node ID 1d24b3e5efdbb7f972f5dfe862b334cf4a64e5e4
>> # Parent  883e268cb860c0ea2eb0faa94114e11c3a4a3893
>> update: introduce --tool for controlling the merge tool
> bikeshed: could this be --merge-tool instead of --tool? The reason I
> ask is that it's a non-obvious flag name when it's embedded on 'hg
> update'.

I see your point. The help will however explain it as "specify merge tool".

But more important: The option is called --tool in all other places. It 
would be inconsistent if it was called something else here. It would 
require a special entry in commands.py and a special explanation in 
merge-tools.txt. I think it is better to stick with --tool.

/Mads
Pierre-Yves David - May 26, 2014, 5:43 p.m.
On 05/18/2014 04:53 PM, Mads Kiilerich wrote:
> # HG changeset patch
> # User Mads Kiilerich <madski@unity3d.com>
> # Date 1400457214 -7200
> #      Mon May 19 01:53:34 2014 +0200
> # Node ID 1d24b3e5efdbb7f972f5dfe862b334cf4a64e5e4
> # Parent  883e268cb860c0ea2eb0faa94114e11c3a4a3893
> update: introduce --tool for controlling the merge tool

I'm surprise it does not already have it.

+1 for this patch (and +1 for the --tool flag used everywhere else)

Actually I'm so +1 that I pushed it to the clowncopter.

(queued)

Patch

diff --git a/mercurial/commands.py b/mercurial/commands.py
--- a/mercurial/commands.py
+++ b/mercurial/commands.py
@@ -5807,9 +5807,11 @@  def unbundle(ui, repo, fname1, *fnames, 
     ('c', 'check', None,
      _('update across branches if no uncommitted changes')),
     ('d', 'date', '', _('tipmost revision matching date'), _('DATE')),
-    ('r', 'rev', '', _('revision'), _('REV'))],
+    ('r', 'rev', '', _('revision'), _('REV'))
+     ] + mergetoolopts,
     _('[-c] [-C] [-d DATE] [[-r] REV]'))
-def update(ui, repo, node=None, rev=None, clean=False, date=None, check=False):
+def update(ui, repo, node=None, rev=None, clean=False, date=None, check=False,
+           tool=None):
     """update working directory (or switch revisions)
 
     Update the repository's working directory to the specified
@@ -5890,6 +5892,8 @@  def update(ui, repo, node=None, rev=None
             rev = repo[repo[None].branch()].rev()
         mergemod._checkunknown(repo, repo[None], repo[rev])
 
+    repo.ui.setconfig('ui', 'forcemerge', tool, 'update')
+
     if clean:
         ret = hg.clean(repo, rev)
     else:
diff --git a/tests/test-completion.t b/tests/test-completion.t
--- a/tests/test-completion.t
+++ b/tests/test-completion.t
@@ -212,7 +212,7 @@  Show all commands + options
   serve: accesslog, daemon, daemon-pipefds, errorlog, port, address, prefix, name, web-conf, webdir-conf, pid-file, stdio, cmdserver, templates, style, ipv6, certificate
   status: all, modified, added, removed, deleted, clean, unknown, ignored, no-status, copies, print0, rev, change, include, exclude, subrepos
   summary: remote
-  update: clean, check, date, rev
+  update: clean, check, date, rev, tool
   addremove: similarity, include, exclude, dry-run
   archive: no-decode, prefix, rev, type, subrepos, include, exclude
   backout: merge, parent, rev, tool, include, exclude, message, logfile, date, user
diff --git a/tests/test-merge-tools.t b/tests/test-merge-tools.t
--- a/tests/test-merge-tools.t
+++ b/tests/test-merge-tools.t
@@ -587,6 +587,54 @@  HGMERGE specifies internal:other but is 
 
   $ unset HGMERGE # make sure HGMERGE doesn't interfere with remaining tests
 
+update is a merge ...
+
+  $ beforemerge
+  [merge-tools]
+  false.whatever=
+  true.priority=1
+  true.executable=cat
+  # hg update -C 1
+  $ hg debugsetparent 0
+  $ hg update -r 2
+  merging f
+  revision 1
+  space
+  revision 0
+  space
+  revision 2
+  space
+  0 files updated, 1 files merged, 0 files removed, 0 files unresolved
+  $ aftermerge
+  # cat f
+  revision 1
+  space
+  # hg stat
+  M f
+
+update should also have --tool
+
+  $ beforemerge
+  [merge-tools]
+  false.whatever=
+  true.priority=1
+  true.executable=cat
+  # hg update -C 1
+  $ hg debugsetparent 0
+  $ hg update -r 2 --tool false
+  merging f
+  merging f failed!
+  0 files updated, 0 files merged, 0 files removed, 1 files unresolved
+  use 'hg resolve' to retry unresolved file merges
+  [1]
+  $ aftermerge
+  # cat f
+  revision 1
+  space
+  # hg stat
+  M f
+  ? f.orig
+
 Default is silent simplemerge:
 
   $ beforemerge