Patchwork [2,of,5] filemerge: indicate that local/other are p1/p2

login
register
mail settings
Submitter timeless@mozdev.org
Date March 17, 2016, 3:46 p.m.
Message ID <18892389001b3c991524.1458229609@waste.org>
Download mbox | patch
Permalink /patch/13920/
State Accepted
Commit 66d085e55ecd84e96be6b210f081882c15df64e3
Headers show

Comments

timeless@mozdev.org - March 17, 2016, 3:46 p.m.
# HG changeset patch
# User timeless <timeless@mozdev.org>
# Date 1458174961 0
#      Thu Mar 17 00:36:01 2016 +0000
# Node ID 18892389001b3c991524cf04bbb004a7a1a15188
# Parent  a96a4945808a3d3896e81d2204ef1ccd3a44a251
filemerge: indicate that local/other are p1/p2
timeless - March 17, 2016, 3:49 p.m.
This specific commit (not the rest of the series) is doc only and
could probably be sent to stable if people like it.

On Thu, Mar 17, 2016 at 11:46 AM, timeless <timeless@mozdev.org> wrote:
> # HG changeset patch
> # User timeless <timeless@mozdev.org>
> # Date 1458174961 0
> #      Thu Mar 17 00:36:01 2016 +0000
> # Node ID 18892389001b3c991524cf04bbb004a7a1a15188
> # Parent  a96a4945808a3d3896e81d2204ef1ccd3a44a251
> filemerge: indicate that local/other are p1/p2
>
> diff --git a/mercurial/filemerge.py b/mercurial/filemerge.py
> --- a/mercurial/filemerge.py
> +++ b/mercurial/filemerge.py
> @@ -230,8 +230,8 @@
>
>  @internaltool('prompt', nomerge)
>  def _iprompt(repo, mynode, orig, fcd, fco, fca, toolconf):
> -    """Asks the user which of the local or the other version to keep as
> -    the merged version."""
> +    """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()
>
> @@ -268,12 +268,12 @@
>
>  @internaltool('local', nomerge)
>  def _ilocal(repo, mynode, orig, fcd, fco, fca, toolconf):
> -    """Uses the local version of files as the merged version."""
> +    """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):
> -    """Uses the other version of files as the merged version."""
> +    """Uses the other (p2) version of files as the merged version."""
>      if fco.isabsent():
>          # local changed, remote deleted -- 'deleted' picked
>          repo.wvfs.unlinkpath(fcd.path())
> @@ -411,7 +411,7 @@
>  def _imergelocal(*args, **kwargs):
>      """
>      Like :merge, but resolve all conflicts non-interactively in favor
> -    of the local changes."""
> +    of the local (p1) changes."""
>      success, status = _imergeauto(localorother='local', *args, **kwargs)
>      return success, status, False
>
> @@ -419,7 +419,7 @@
>  def _imergeother(*args, **kwargs):
>      """
>      Like :merge, but resolve all conflicts non-interactively in favor
> -    of the other changes."""
> +    of the other (p2) changes."""
>      success, status = _imergeauto(localorother='other', *args, **kwargs)
>      return success, status, False
>
> diff --git a/tests/test-help.t b/tests/test-help.t
> --- a/tests/test-help.t
> +++ b/tests/test-help.t
> @@ -1537,7 +1537,7 @@
>          to resolve these conflicts.
>
>        ":local"
> -        Uses the local version of files as the merged version.
> +        Uses the local (p1) version of files as the merged version.
>
>        ":merge"
>          Uses the internal non-interactive simple merge algorithm for merging
> @@ -1547,11 +1547,11 @@
>
>        ":merge-local"
>          Like :merge, but resolve all conflicts non-interactively in favor of the
> -        local changes.
> +        local (p1) changes.
>
>        ":merge-other"
>          Like :merge, but resolve all conflicts non-interactively in favor of the
> -        other changes.
> +        other (p2) changes.
>
>        ":merge3"
>          Uses the internal non-interactive simple merge algorithm for merging
> @@ -1560,11 +1560,11 @@
>          side of the merge and one for the base content.
>
>        ":other"
> -        Uses the other version of files as the merged version.
> +        Uses the other (p2) version of files as the merged version.
>
>        ":prompt"
> -        Asks the user which of the local or the other version to keep as the
> -        merged version.
> +        Asks the user which of the local (p1) or the other (p2) version to keep
> +        as the merged version.
>
>        ":tagmerge"
>          Uses the internal tag merge algorithm (experimental).
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Gregory Szorc - March 17, 2016, 4:02 p.m.
Something to watch out for with p1 and p2 is that Mercurial doesn't preserve parent order: it sorts the nodes and first is p1.

I think this patch is referring to different parents, so it might be fine. Then again, it does expose p1 and p2 to the docs, so...

> On Mar 17, 2016, at 08:46, timeless <timeless@mozdev.org> wrote:
> 
> # HG changeset patch
> # User timeless <timeless@mozdev.org>
> # Date 1458174961 0
> #      Thu Mar 17 00:36:01 2016 +0000
> # Node ID 18892389001b3c991524cf04bbb004a7a1a15188
> # Parent  a96a4945808a3d3896e81d2204ef1ccd3a44a251
> filemerge: indicate that local/other are p1/p2
> 
> diff --git a/mercurial/filemerge.py b/mercurial/filemerge.py
> --- a/mercurial/filemerge.py
> +++ b/mercurial/filemerge.py
> @@ -230,8 +230,8 @@
> 
> @internaltool('prompt', nomerge)
> def _iprompt(repo, mynode, orig, fcd, fco, fca, toolconf):
> -    """Asks the user which of the local or the other version to keep as
> -    the merged version."""
> +    """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()
> 
> @@ -268,12 +268,12 @@
> 
> @internaltool('local', nomerge)
> def _ilocal(repo, mynode, orig, fcd, fco, fca, toolconf):
> -    """Uses the local version of files as the merged version."""
> +    """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):
> -    """Uses the other version of files as the merged version."""
> +    """Uses the other (p2) version of files as the merged version."""
>     if fco.isabsent():
>         # local changed, remote deleted -- 'deleted' picked
>         repo.wvfs.unlinkpath(fcd.path())
> @@ -411,7 +411,7 @@
> def _imergelocal(*args, **kwargs):
>     """
>     Like :merge, but resolve all conflicts non-interactively in favor
> -    of the local changes."""
> +    of the local (p1) changes."""
>     success, status = _imergeauto(localorother='local', *args, **kwargs)
>     return success, status, False
> 
> @@ -419,7 +419,7 @@
> def _imergeother(*args, **kwargs):
>     """
>     Like :merge, but resolve all conflicts non-interactively in favor
> -    of the other changes."""
> +    of the other (p2) changes."""
>     success, status = _imergeauto(localorother='other', *args, **kwargs)
>     return success, status, False
> 
> diff --git a/tests/test-help.t b/tests/test-help.t
> --- a/tests/test-help.t
> +++ b/tests/test-help.t
> @@ -1537,7 +1537,7 @@
>         to resolve these conflicts.
> 
>       ":local"
> -        Uses the local version of files as the merged version.
> +        Uses the local (p1) version of files as the merged version.
> 
>       ":merge"
>         Uses the internal non-interactive simple merge algorithm for merging
> @@ -1547,11 +1547,11 @@
> 
>       ":merge-local"
>         Like :merge, but resolve all conflicts non-interactively in favor of the
> -        local changes.
> +        local (p1) changes.
> 
>       ":merge-other"
>         Like :merge, but resolve all conflicts non-interactively in favor of the
> -        other changes.
> +        other (p2) changes.
> 
>       ":merge3"
>         Uses the internal non-interactive simple merge algorithm for merging
> @@ -1560,11 +1560,11 @@
>         side of the merge and one for the base content.
> 
>       ":other"
> -        Uses the other version of files as the merged version.
> +        Uses the other (p2) version of files as the merged version.
> 
>       ":prompt"
> -        Asks the user which of the local or the other version to keep as the
> -        merged version.
> +        Asks the user which of the local (p1) or the other (p2) version to keep
> +        as the merged version.
> 
>       ":tagmerge"
>         Uses the internal tag merge algorithm (experimental).
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
timeless - March 17, 2016, 4:59 p.m.
all I know from the tests is that tip can be local or other, which
gave me a /bit/ of hope that it was doing the right thing.

wrt the text tself, I think I'm going to use: `p1()` and let the
templater replace the backticks.

On Thu, Mar 17, 2016 at 12:02 PM, Gregory Szorc <gregory.szorc@gmail.com> wrote:
> Something to watch out for with p1 and p2 is that Mercurial doesn't preserve parent order: it sorts the nodes and first is p1.
>
> I think this patch is referring to different parents, so it might be fine. Then again, it does expose p1 and p2 to the docs, so...
>
>> On Mar 17, 2016, at 08:46, timeless <timeless@mozdev.org> wrote:
>>
>> # HG changeset patch
>> # User timeless <timeless@mozdev.org>
>> # Date 1458174961 0
>> #      Thu Mar 17 00:36:01 2016 +0000
>> # Node ID 18892389001b3c991524cf04bbb004a7a1a15188
>> # Parent  a96a4945808a3d3896e81d2204ef1ccd3a44a251
>> filemerge: indicate that local/other are p1/p2
>>
>> diff --git a/mercurial/filemerge.py b/mercurial/filemerge.py
>> --- a/mercurial/filemerge.py
>> +++ b/mercurial/filemerge.py
>> @@ -230,8 +230,8 @@
>>
>> @internaltool('prompt', nomerge)
>> def _iprompt(repo, mynode, orig, fcd, fco, fca, toolconf):
>> -    """Asks the user which of the local or the other version to keep as
>> -    the merged version."""
>> +    """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()
>>
>> @@ -268,12 +268,12 @@
>>
>> @internaltool('local', nomerge)
>> def _ilocal(repo, mynode, orig, fcd, fco, fca, toolconf):
>> -    """Uses the local version of files as the merged version."""
>> +    """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):
>> -    """Uses the other version of files as the merged version."""
>> +    """Uses the other (p2) version of files as the merged version."""
>>     if fco.isabsent():
>>         # local changed, remote deleted -- 'deleted' picked
>>         repo.wvfs.unlinkpath(fcd.path())
>> @@ -411,7 +411,7 @@
>> def _imergelocal(*args, **kwargs):
>>     """
>>     Like :merge, but resolve all conflicts non-interactively in favor
>> -    of the local changes."""
>> +    of the local (p1) changes."""
>>     success, status = _imergeauto(localorother='local', *args, **kwargs)
>>     return success, status, False
>>
>> @@ -419,7 +419,7 @@
>> def _imergeother(*args, **kwargs):
>>     """
>>     Like :merge, but resolve all conflicts non-interactively in favor
>> -    of the other changes."""
>> +    of the other (p2) changes."""
>>     success, status = _imergeauto(localorother='other', *args, **kwargs)
>>     return success, status, False
>>
>> diff --git a/tests/test-help.t b/tests/test-help.t
>> --- a/tests/test-help.t
>> +++ b/tests/test-help.t
>> @@ -1537,7 +1537,7 @@
>>         to resolve these conflicts.
>>
>>       ":local"
>> -        Uses the local version of files as the merged version.
>> +        Uses the local (p1) version of files as the merged version.
>>
>>       ":merge"
>>         Uses the internal non-interactive simple merge algorithm for merging
>> @@ -1547,11 +1547,11 @@
>>
>>       ":merge-local"
>>         Like :merge, but resolve all conflicts non-interactively in favor of the
>> -        local changes.
>> +        local (p1) changes.
>>
>>       ":merge-other"
>>         Like :merge, but resolve all conflicts non-interactively in favor of the
>> -        other changes.
>> +        other (p2) changes.
>>
>>       ":merge3"
>>         Uses the internal non-interactive simple merge algorithm for merging
>> @@ -1560,11 +1560,11 @@
>>         side of the merge and one for the base content.
>>
>>       ":other"
>> -        Uses the other version of files as the merged version.
>> +        Uses the other (p2) version of files as the merged version.
>>
>>       ":prompt"
>> -        Asks the user which of the local or the other version to keep as the
>> -        merged version.
>> +        Asks the user which of the local (p1) or the other (p2) version to keep
>> +        as the merged version.
>>
>>       ":tagmerge"
>>         Uses the internal tag merge algorithm (experimental).
>> _______________________________________________
>> Mercurial-devel mailing list
>> Mercurial-devel@mercurial-scm.org
>> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Simon King - March 17, 2016, 9 p.m.
> On 17 Mar 2016, at 16:02, Gregory Szorc <gregory.szorc@gmail.com> wrote:
> 
> Something to watch out for with p1 and p2 is that Mercurial doesn't preserve parent order: it sorts the nodes and first is p1.
> 
> I think this patch is referring to different parents, so it might be fine. Then again, it does expose p1 and p2 to the docs, so...

What exactly do you mean by this? I was always under the impression that during a merge, p1 was the revision that was checked out, and p2 was the revision specified in “hg merge” (and a quick test suggests that’s the case). Is it not guaranteed?

Simon
Pierre-Yves David - March 19, 2016, 5:34 p.m.
On 03/17/2016 08:46 AM, timeless wrote:
> # HG changeset patch
> # User timeless <timeless@mozdev.org>
> # Date 1458174961 0
> #      Thu Mar 17 00:36:01 2016 +0000
> # Node ID 18892389001b3c991524cf04bbb004a7a1a15188
> # Parent  a96a4945808a3d3896e81d2204ef1ccd3a44a251
> filemerge: indicate that local/other are p1/p2

I've pushed this one to the clowncopter.

patch 3 had comment from simon, I had feedback on patch 5 and patch 4 
make sense for patch 5 only. Dropping the rest of the series off the 
patchwork.

Cheers,
Matt Mackall - March 24, 2016, 10:15 p.m.
On Thu, 2016-03-17 at 21:00 +0000, Simon King wrote:
> > On 17 Mar 2016, at 16:02, Gregory Szorc <gregory.szorc@gmail.com> wrote:
> > 
> > Something to watch out for with p1 and p2 is that Mercurial doesn't preserve
> > parent order: it sorts the nodes and first is p1.
> > 
> > I think this patch is referring to different parents, so it might be fine.
> > Then again, it does expose p1 and p2 to the docs, so...
> 
> What exactly do you mean by this? I was always under the impression that
> during a merge, p1 was the revision that was checked out, and p2 was the
> revision specified in “hg merge” (and a quick test suggests that’s the case).
> Is it not guaranteed?

For the purposes of hash stability, we do sort the parents when calculating the
revision hash, under the theory that the contents of merge(a, b) ought to equal
merge(b,a) and not be distinct. This made more sense in paleo-Mercurial where
everything was intentionally symmetric, but things like branch names have mucked
that up. But we actually preserve the ordering of parents from the dirstate when
recording the index.

-- 
Mathematics is the supreme nostalgia of our time.

Patch

diff --git a/mercurial/filemerge.py b/mercurial/filemerge.py
--- a/mercurial/filemerge.py
+++ b/mercurial/filemerge.py
@@ -230,8 +230,8 @@ 
 
 @internaltool('prompt', nomerge)
 def _iprompt(repo, mynode, orig, fcd, fco, fca, toolconf):
-    """Asks the user which of the local or the other version to keep as
-    the merged version."""
+    """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()
 
@@ -268,12 +268,12 @@ 
 
 @internaltool('local', nomerge)
 def _ilocal(repo, mynode, orig, fcd, fco, fca, toolconf):
-    """Uses the local version of files as the merged version."""
+    """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):
-    """Uses the other version of files as the merged version."""
+    """Uses the other (p2) version of files as the merged version."""
     if fco.isabsent():
         # local changed, remote deleted -- 'deleted' picked
         repo.wvfs.unlinkpath(fcd.path())
@@ -411,7 +411,7 @@ 
 def _imergelocal(*args, **kwargs):
     """
     Like :merge, but resolve all conflicts non-interactively in favor
-    of the local changes."""
+    of the local (p1) changes."""
     success, status = _imergeauto(localorother='local', *args, **kwargs)
     return success, status, False
 
@@ -419,7 +419,7 @@ 
 def _imergeother(*args, **kwargs):
     """
     Like :merge, but resolve all conflicts non-interactively in favor
-    of the other changes."""
+    of the other (p2) changes."""
     success, status = _imergeauto(localorother='other', *args, **kwargs)
     return success, status, False
 
diff --git a/tests/test-help.t b/tests/test-help.t
--- a/tests/test-help.t
+++ b/tests/test-help.t
@@ -1537,7 +1537,7 @@ 
         to resolve these conflicts.
   
       ":local"
-        Uses the local version of files as the merged version.
+        Uses the local (p1) version of files as the merged version.
   
       ":merge"
         Uses the internal non-interactive simple merge algorithm for merging
@@ -1547,11 +1547,11 @@ 
   
       ":merge-local"
         Like :merge, but resolve all conflicts non-interactively in favor of the
-        local changes.
+        local (p1) changes.
   
       ":merge-other"
         Like :merge, but resolve all conflicts non-interactively in favor of the
-        other changes.
+        other (p2) changes.
   
       ":merge3"
         Uses the internal non-interactive simple merge algorithm for merging
@@ -1560,11 +1560,11 @@ 
         side of the merge and one for the base content.
   
       ":other"
-        Uses the other version of files as the merged version.
+        Uses the other (p2) version of files as the merged version.
   
       ":prompt"
-        Asks the user which of the local or the other version to keep as the
-        merged version.
+        Asks the user which of the local (p1) or the other (p2) version to keep
+        as the merged version.
   
       ":tagmerge"
         Uses the internal tag merge algorithm (experimental).