Patchwork [1,of,2] merge: use labels in prompts to the user

login
register
mail settings
Submitter Simon Farnsworth
Date April 14, 2016, 5:59 p.m.
Message ID <5d7edaf88da036b100f5.1460656783@simonfar-macbookpro.local>
Download mbox | patch
Permalink /patch/14629/
State Changes Requested
Headers show

Comments

Simon Farnsworth - April 14, 2016, 5:59 p.m.
# HG changeset patch
# User Simon Farnsworth <simonfar@fb.com>
# Date 1460584373 -3600
#      Wed Apr 13 22:52:53 2016 +0100
# Node ID 5d7edaf88da036b100f53cbf2bad9930433ec4c1
# Parent  02be5fc18c0c70c087a9d1ab5ffe5afce926f227
merge: use labels in prompts to the user

Now that we persist the labels, we can consistently use the labels in
prompts for the user without risk of confusion. This changes a huge amount
of command output:

This means that merge prompts like:
  no tool found to merge a
  keep (l)ocal, take (o)ther, or leave (u)nresolved? u
and
  remote changed a which local deleted
  use (c)hanged version, leave (d)eleted, or leave (u)nresolved? c
become:
  no tool found to merge a
  keep (l)ocal [working copy], take (o)ther [destination], or leave (u)nresolved? u
and
  remote [source] changed a which local [dest] deleted
  use (c)hanged version, leave (d)eleted, or leave (u)nresolved? c
where "working copy" and "destination" were supplied by the command that
requested the merge as labels for conflict markers, and thus should be
human-friendly.
timeless - April 14, 2016, 7:21 p.m.
Simon Farnsworth wrote:
> +    prompts = partnames(labels)
> +    prompts['fd'] = fd
>      try:
>          if fco.isabsent():
>              index = ui.promptchoice(
> -                _("local changed %s which remote deleted\n"
> +                _("%(local)s changed %(fd)s which %(remote)s deleted\n"
>                    "use (c)hanged version, (d)elete, or leave (u)nresolved?"
> -                  "$$ &Changed $$ &Delete $$ &Unresolved") % fd, 2)
> +                  "$$ &Changed $$ &Delete $$ &Unresolved") % prompts, 2)

"fd" isn't really a prompt (it's the file). I understand what you're
writing and don't have a better suggestion, but in reading it, it is a
bit confusing.

(I'm going to send individual thoughts, otherwise I'll never send anything)
timeless - April 14, 2016, 7:26 p.m.
Simon Farnsworth wrote:
> diff --git a/tests/failfilemerge.py b/tests/failfilemerge.py
> --- a/tests/failfilemerge.py
> +++ b/tests/failfilemerge.py
> @@ -9,7 +9,7 @@
>  )
>
>  def failfilemerge(filemergefn,
> -        premerge, repo, mynode, orig, fcd, fco, fca, labels=None):
> +                  premerge, repo, mynode, orig, fcd, fco, fca, labels=None):
>      raise error.Abort("^C")
>      return filemergefn(premerge, repo, mynode, orig, fcd, fco, fca, labels)
>

This is unrelated cleanup that should probably be in a different
commit (just to make things easier for anyone reading the big commit).
Yuya Nishihara - April 16, 2016, 9:15 a.m.
On Thu, 14 Apr 2016 18:59:43 +0100, Simon Farnsworth wrote:
> # HG changeset patch
> # User Simon Farnsworth <simonfar@fb.com>
> # Date 1460584373 -3600
> #      Wed Apr 13 22:52:53 2016 +0100
> # Node ID 5d7edaf88da036b100f53cbf2bad9930433ec4c1
> # Parent  02be5fc18c0c70c087a9d1ab5ffe5afce926f227
> merge: use labels in prompts to the user
> 
> Now that we persist the labels, we can consistently use the labels in
> prompts for the user without risk of confusion. This changes a huge amount
> of command output:
> 
> This means that merge prompts like:
>   no tool found to merge a
>   keep (l)ocal, take (o)ther, or leave (u)nresolved? u
> and
>   remote changed a which local deleted
>   use (c)hanged version, leave (d)eleted, or leave (u)nresolved? c
> become:
>   no tool found to merge a
>   keep (l)ocal [working copy], take (o)ther [destination], or leave (u)nresolved? u
> and
>   remote [source] changed a which local [dest] deleted
>   use (c)hanged version, leave (d)eleted, or leave (u)nresolved? c
> where "working copy" and "destination" were supplied by the command that
> requested the merge as labels for conflict markers, and thus should be
> human-friendly.

>              index = ui.promptchoice(
> -                _("no tool found to merge %s\n"
> -                  "keep (l)ocal, take (o)ther, or leave (u)nresolved?"
> -                  "$$ &Local $$ &Other $$ &Unresolved") % fd, 2)
> +                _("no tool found to merge %(fd)s\n"
> +                  "keep %(localact)s, take %(otheract)s, or leave (u)nresolved?"
> +                  "$$ &Local $$ &Other $$ &Unresolved") % prompts, 2)
>              choice = ['local', 'other', 'unresolved'][index]

We have to keep the message "(l)ocal" and the choice "&Local" in a single
translation text. Otherwise, the message could mismatch with the choice.

https://selenic.com/repo/hg/rev/c58b6ab4c26f

Other than that, the patch looks good to me.

Patch

diff --git a/mercurial/filemerge.py b/mercurial/filemerge.py
--- a/mercurial/filemerge.py
+++ b/mercurial/filemerge.py
@@ -229,50 +229,56 @@ 
                 util.writefile(file, newdata)
 
 @internaltool('prompt', nomerge)
-def _iprompt(repo, mynode, orig, fcd, fco, fca, toolconf):
+def _iprompt(repo, mynode, orig, fcd, fco, fca, toolconf, labels=None):
     """Asks the user which of the local `p1()` or the other `p2()` version to
     keep as the merged version."""
     ui = repo.ui
     fd = fcd.path()
 
+    prompts = partnames(labels)
+    prompts['fd'] = fd
     try:
         if fco.isabsent():
             index = ui.promptchoice(
-                _("local changed %s which remote deleted\n"
+                _("%(local)s changed %(fd)s which %(remote)s deleted\n"
                   "use (c)hanged version, (d)elete, or leave (u)nresolved?"
-                  "$$ &Changed $$ &Delete $$ &Unresolved") % fd, 2)
+                  "$$ &Changed $$ &Delete $$ &Unresolved") % prompts, 2)
             choice = ['local', 'other', 'unresolved'][index]
         elif fcd.isabsent():
             index = ui.promptchoice(
-                _("remote changed %s which local deleted\n"
+                _("%(remote)s changed %(fd)s which %(local)s deleted\n"
                   "use (c)hanged version, leave (d)eleted, or "
                   "leave (u)nresolved?"
-                  "$$ &Changed $$ &Deleted $$ &Unresolved") % fd, 2)
+                  "$$ &Changed $$ &Deleted $$ &Unresolved") % prompts, 2)
             choice = ['other', 'local', 'unresolved'][index]
         else:
             index = ui.promptchoice(
-                _("no tool found to merge %s\n"
-                  "keep (l)ocal, take (o)ther, or leave (u)nresolved?"
-                  "$$ &Local $$ &Other $$ &Unresolved") % fd, 2)
+                _("no tool found to merge %(fd)s\n"
+                  "keep %(localact)s, take %(otheract)s, or leave (u)nresolved?"
+                  "$$ &Local $$ &Other $$ &Unresolved") % prompts, 2)
             choice = ['local', 'other', 'unresolved'][index]
 
         if choice == 'other':
-            return _iother(repo, mynode, orig, fcd, fco, fca, toolconf)
+            return _iother(repo, mynode, orig, fcd, fco, fca, toolconf,
+                           labels)
         elif choice == 'local':
-            return _ilocal(repo, mynode, orig, fcd, fco, fca, toolconf)
+            return _ilocal(repo, mynode, orig, fcd, fco, fca, toolconf,
+                           labels)
         elif choice == 'unresolved':
-            return _ifail(repo, mynode, orig, fcd, fco, fca, toolconf)
+            return _ifail(repo, mynode, orig, fcd, fco, fca, toolconf,
+                          labels)
     except error.ResponseExpected:
         ui.write("\n")
-        return _ifail(repo, mynode, orig, fcd, fco, fca, toolconf)
+        return _ifail(repo, mynode, orig, fcd, fco, fca, toolconf,
+                      labels)
 
 @internaltool('local', nomerge)
-def _ilocal(repo, mynode, orig, fcd, fco, fca, toolconf):
+def _ilocal(repo, mynode, orig, fcd, fco, fca, toolconf, labels=None):
     """Uses the local `p1()` version of files as the merged version."""
     return 0, fcd.isabsent()
 
 @internaltool('other', nomerge)
-def _iother(repo, mynode, orig, fcd, fco, fca, toolconf):
+def _iother(repo, mynode, orig, fcd, fco, fca, toolconf, labels=None):
     """Uses the other `p2()` version of files as the merged version."""
     if fco.isabsent():
         # local changed, remote deleted -- 'deleted' picked
@@ -284,7 +290,7 @@ 
     return 0, deleted
 
 @internaltool('fail', nomerge)
-def _ifail(repo, mynode, orig, fcd, fco, fca, toolconf):
+def _ifail(repo, mynode, orig, fcd, fco, fca, toolconf, labels=None):
     """
     Rather than attempting to merge files that were modified on both
     branches, it marks them as unresolved. The resolve command must be
@@ -536,6 +542,27 @@ 
         newlabels.append(_formatconflictmarker(repo, ca, tmpl, labels[2], pad))
     return newlabels
 
+def partnames(labels):
+    """Return a dictionary of merge part names for use in prompts to the user
+
+    These names include labels if supplied"""
+    if labels is None:
+        return {
+            "local": _("local"),
+            "other": _("other"),
+            "remote": _("remote"),
+            "localact": _("(l)ocal"),
+            "otheract": _("(o)ther"),
+        }
+
+    return {
+        "local": _("local [%s]") % labels[0],
+        "other": _("other [%s]") % labels[1],
+        "remote": _("remote [%s]") % labels[1],
+        "localact": _("(l)ocal [%s]") % labels[0],
+        "otheract": _("(o)ther [%s]") % labels[1],
+    }
+
 def _filemerge(premerge, repo, mynode, orig, fcd, fco, fca, labels=None):
     """perform a 3-way merge in the working directory
 
@@ -587,7 +614,7 @@ 
     toolconf = tool, toolpath, binary, symlink
 
     if mergetype == nomerge:
-        r, deleted = func(repo, mynode, orig, fcd, fco, fca, toolconf)
+        r, deleted = func(repo, mynode, orig, fcd, fco, fca, toolconf, labels)
         return True, r, deleted
 
     if premerge:
diff --git a/mercurial/merge.py b/mercurial/merge.py
--- a/mercurial/merge.py
+++ b/mercurial/merge.py
@@ -1537,11 +1537,13 @@ 
         if '.hgsubstate' in actionbyfile:
             f = '.hgsubstate'
             m, args, msg = actionbyfile[f]
+            prompts = filemerge.partnames(labels)
+            prompts['f'] = f
             if m == 'cd':
                 if repo.ui.promptchoice(
-                    _("local changed %s which remote deleted\n"
+                    _("%(local)s changed %(f)s which %(remote)s deleted\n"
                       "use (c)hanged version or (d)elete?"
-                      "$$ &Changed $$ &Delete") % f, 0):
+                      "$$ &Changed $$ &Delete") % prompts, 0):
                     actionbyfile[f] = ('r', None, "prompt delete")
                 elif f in p1:
                     actionbyfile[f] = ('am', None, "prompt keep")
@@ -1551,9 +1553,9 @@ 
                 f1, f2, fa, move, anc = args
                 flags = p2[f2].flags()
                 if repo.ui.promptchoice(
-                    _("remote changed %s which local deleted\n"
+                    _("%(remote)s changed %(f)s which %(local)s deleted\n"
                       "use (c)hanged version or leave (d)eleted?"
-                      "$$ &Changed $$ &Deleted") % f, 0) == 0:
+                      "$$ &Changed $$ &Deleted") % prompts, 0) == 0:
                     actionbyfile[f] = ('g', (flags, False), "prompt recreating")
                 else:
                     del actionbyfile[f]
diff --git a/tests/failfilemerge.py b/tests/failfilemerge.py
--- a/tests/failfilemerge.py
+++ b/tests/failfilemerge.py
@@ -9,7 +9,7 @@ 
 )
 
 def failfilemerge(filemergefn,
-        premerge, repo, mynode, orig, fcd, fco, fca, labels=None):
+                  premerge, repo, mynode, orig, fcd, fco, fca, labels=None):
     raise error.Abort("^C")
     return filemergefn(premerge, repo, mynode, orig, fcd, fco, fca, labels)
 
diff --git a/tests/test-copy-move-merge.t b/tests/test-copy-move-merge.t
--- a/tests/test-copy-move-merge.t
+++ b/tests/test-copy-move-merge.t
@@ -85,7 +85,7 @@ 
   > c
   > EOF
   rebasing 2:add3f11052fa "other" (tip)
-  remote changed a which local deleted
+  remote [source] changed a which local [dest] deleted
   use (c)hanged version, leave (d)eleted, or leave (u)nresolved? c
 
   $ cat b
diff --git a/tests/test-largefiles-update.t b/tests/test-largefiles-update.t
--- a/tests/test-largefiles-update.t
+++ b/tests/test-largefiles-update.t
@@ -611,7 +611,7 @@ 
   > EOF
   rebasing 1:72518492caa6 "#1"
   rebasing 4:07d6153b5c04 "#4"
-  local changed .hglf/large1 which remote deleted
+  local [dest] changed .hglf/large1 which remote [source] deleted
   use (c)hanged version, (d)elete, or leave (u)nresolved? c
 
   $ hg diff -c "tip~1" --nodates .hglf/large1 | grep '^[+-][0-9a-z]'
diff --git a/tests/test-merge-changedelete.t b/tests/test-merge-changedelete.t
--- a/tests/test-merge-changedelete.t
+++ b/tests/test-merge-changedelete.t
@@ -717,9 +717,9 @@ 
   $ echo changed >> file1
   $ hg rm file2
   $ hg update 1 -y
-  local changed file1 which remote deleted
+  local [working copy] changed file1 which remote [destination] deleted
   use (c)hanged version, (d)elete, or leave (u)nresolved? u
-  remote changed file2 which local deleted
+  remote [destination] changed file2 which local [working copy] deleted
   use (c)hanged version, leave (d)eleted, or leave (u)nresolved? u
   1 files updated, 0 files merged, 0 files removed, 2 files unresolved
   use 'hg resolve' to retry unresolved file merges
@@ -893,9 +893,9 @@ 
   $ echo changed >> file1
   $ hg rm file2
   $ hg update 1 --config ui.interactive=True --tool :prompt
-  local changed file1 which remote deleted
+  local [working copy] changed file1 which remote [destination] deleted
   use (c)hanged version, (d)elete, or leave (u)nresolved? 
-  remote changed file2 which local deleted
+  remote [destination] changed file2 which local [working copy] deleted
   use (c)hanged version, leave (d)eleted, or leave (u)nresolved? 
   1 files updated, 0 files merged, 0 files removed, 2 files unresolved
   use 'hg resolve' to retry unresolved file merges
@@ -943,9 +943,9 @@ 
   $ echo changed >> file1
   $ hg rm file2
   $ hg update 1 --tool :merge3
-  local changed file1 which remote deleted
+  local [working copy] changed file1 which remote [destination] deleted
   use (c)hanged version, (d)elete, or leave (u)nresolved? u
-  remote changed file2 which local deleted
+  remote [destination] changed file2 which local [working copy] deleted
   use (c)hanged version, leave (d)eleted, or leave (u)nresolved? u
   1 files updated, 0 files merged, 0 files removed, 2 files unresolved
   use 'hg resolve' to retry unresolved file merges
@@ -999,9 +999,9 @@ 
   (status identical)
   
   === :other -> :prompt ===
-  local changed file1 which remote deleted
+  local [working copy] changed file1 which remote [destination] deleted
   use (c)hanged version, (d)elete, or leave (u)nresolved? 
-  remote changed file2 which local deleted
+  remote [destination] changed file2 which local [working copy] deleted
   use (c)hanged version, leave (d)eleted, or leave (u)nresolved? 
   --- diff of status ---
   (status identical)
@@ -1026,9 +1026,9 @@ 
   (status identical)
   
   === :local -> :prompt ===
-  local changed file1 which remote deleted
+  local [working copy] changed file1 which remote [destination] deleted
   use (c)hanged version, (d)elete, or leave (u)nresolved? 
-  remote changed file2 which local deleted
+  remote [destination] changed file2 which local [working copy] deleted
   use (c)hanged version, leave (d)eleted, or leave (u)nresolved? 
   --- diff of status ---
   (status identical)
@@ -1043,9 +1043,9 @@ 
   (status identical)
   
   === :fail -> :prompt ===
-  local changed file1 which remote deleted
+  local [working copy] changed file1 which remote [destination] deleted
   use (c)hanged version, (d)elete, or leave (u)nresolved? 
-  remote changed file2 which local deleted
+  remote [destination] changed file2 which local [working copy] deleted
   use (c)hanged version, leave (d)eleted, or leave (u)nresolved? 
   --- diff of status ---
   (status identical)
diff --git a/tests/test-merge-types.t b/tests/test-merge-types.t
--- a/tests/test-merge-types.t
+++ b/tests/test-merge-types.t
@@ -173,7 +173,7 @@ 
   (couldn't find merge tool hgmerge|tool hgmerge can't handle symlinks) (re)
   picked tool ':prompt' for a (binary False symlink True changedelete False)
   no tool found to merge a
-  keep (l)ocal, take (o)ther, or leave (u)nresolved? u
+  keep (l)ocal [working copy], take (o)ther [destination], or leave (u)nresolved? u
   0 files updated, 0 files merged, 0 files removed, 1 files unresolved
   use 'hg resolve' to retry unresolved file merges
   1 other heads for branch "default"
diff --git a/tests/test-rebase-newancestor.t b/tests/test-rebase-newancestor.t
--- a/tests/test-rebase-newancestor.t
+++ b/tests/test-rebase-newancestor.t
@@ -135,7 +135,7 @@ 
   note: rebase of 1:1d1a643d390e created no changes to commit
   rebasing 2:ec2c14fb2984 "dev: f-dev stuff"
   rebasing 4:4b019212aaf6 "dev: merge default"
-  remote changed f-default which local deleted
+  remote [source] changed f-default which local [dest] deleted
   use (c)hanged version, leave (d)eleted, or leave (u)nresolved? c
   rebasing 6:9455ee510502 "dev: merge default"
   saved backup bundle to $TESTTMP/ancestor-merge/.hg/strip-backup/1d1a643d390e-43e9e04b-backup.hg (glob)
@@ -164,7 +164,7 @@ 
   > EOF
   rebasing 2:ec2c14fb2984 "dev: f-dev stuff"
   rebasing 4:4b019212aaf6 "dev: merge default"
-  remote changed f-default which local deleted
+  remote [source] changed f-default which local [dest] deleted
   use (c)hanged version, leave (d)eleted, or leave (u)nresolved? c
   rebasing 6:9455ee510502 "dev: merge default"
   saved backup bundle to $TESTTMP/ancestor-merge-2/.hg/strip-backup/ec2c14fb2984-62d0b222-backup.hg (glob)
diff --git a/tests/test-subrepo-missing.t b/tests/test-subrepo-missing.t
--- a/tests/test-subrepo-missing.t
+++ b/tests/test-subrepo-missing.t
@@ -62,7 +62,7 @@ 
   2 files updated, 0 files merged, 0 files removed, 0 files unresolved
   $ rm .hgsubstate
   $ hg up 0
-  remote changed .hgsubstate which local deleted
+  remote [destination] changed .hgsubstate which local [working copy] deleted
   use (c)hanged version or leave (d)eleted? c
   1 files updated, 0 files merged, 0 files removed, 0 files unresolved
   $ hg st