Patchwork [2,of,8] mergeresult: introduce dedicated tuple for no-op actions

login
register
mail settings
Submitter Pulkit Goyal
Date Sept. 14, 2020, 11:15 a.m.
Message ID <cfabb1ccdf7a2cb67bb2.1600082118@workspace>
Download mbox | patch
Permalink /patch/47156/
State Accepted
Headers show

Comments

Pulkit Goyal - Sept. 14, 2020, 11:15 a.m.
# HG changeset patch
# User Pulkit Goyal <7895pulkit@gmail.com>
# Date 1600071699 -19800
#      Mon Sep 14 13:51:39 2020 +0530
# Node ID cfabb1ccdf7a2cb67bb2819f798bad53c6aac977
# Parent  eab095687e7b655011a26001dc83c802ab474ceb
# EXP-Topic merge-newnode
mergeresult: introduce dedicated tuple for no-op actions

This will help us in adding more no-op actions in next patch while keeping the
code cleaner.
Yuya Nishihara - Sept. 16, 2020, 10:49 a.m.
On Mon, 14 Sep 2020 16:45:18 +0530, Pulkit Goyal wrote:
> # HG changeset patch
> # User Pulkit Goyal <7895pulkit@gmail.com>
> # Date 1600071699 -19800
> #      Mon Sep 14 13:51:39 2020 +0530
> # Node ID cfabb1ccdf7a2cb67bb2819f798bad53c6aac977
> # Parent  eab095687e7b655011a26001dc83c802ab474ceb
> # EXP-Topic merge-newnode
> mergeresult: introduce dedicated tuple for no-op actions

> @@ -564,6 +563,8 @@ class mergeresult(object):
>      It has information about what actions need to be performed on dirstate
>      mapping of divergent renames and other such cases. '''
>  
> +    NO_OP_ACTIONS = (mergestatemod.ACTION_KEEP,)

I prefer defining this kind of constants in module scope (i.e.
mergestatemod.NO_OP_ACTIONS) to make sure it isn't intended to be
overridden by subclasses.
Pierre-Yves David - Sept. 16, 2020, 12:25 p.m.
+1

On 9/16/20 12:49 PM, Yuya Nishihara wrote:
> On Mon, 14 Sep 2020 16:45:18 +0530, Pulkit Goyal wrote:
>> # HG changeset patch
>> # User Pulkit Goyal <7895pulkit@gmail.com>
>> # Date 1600071699 -19800
>> #      Mon Sep 14 13:51:39 2020 +0530
>> # Node ID cfabb1ccdf7a2cb67bb2819f798bad53c6aac977
>> # Parent  eab095687e7b655011a26001dc83c802ab474ceb
>> # EXP-Topic merge-newnode
>> mergeresult: introduce dedicated tuple for no-op actions
> 
>> @@ -564,6 +563,8 @@ class mergeresult(object):
>>       It has information about what actions need to be performed on dirstate
>>       mapping of divergent renames and other such cases. '''
>>   
>> +    NO_OP_ACTIONS = (mergestatemod.ACTION_KEEP,)
> 
> I prefer defining this kind of constants in module scope (i.e.
> mergestatemod.NO_OP_ACTIONS) to make sure it isn't intended to be
> overridden by subclasses.
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>

Patch

diff --git a/mercurial/merge.py b/mercurial/merge.py
--- a/mercurial/merge.py
+++ b/mercurial/merge.py
@@ -523,7 +523,6 @@  def _filternarrowactions(narrowmatch, br
     narrowed.
     """
     # TODO: handle with nonconflicttypes
-    nooptypes = {mergestatemod.ACTION_KEEP}
     nonconflicttypes = {
         mergestatemod.ACTION_ADD,
         mergestatemod.ACTION_ADD_MODIFIED,
@@ -541,7 +540,7 @@  def _filternarrowactions(narrowmatch, br
             pass
         elif not branchmerge:
             mresult.removefile(f)  # just updating, ignore changes outside clone
-        elif action[0] in nooptypes:
+        elif action[0] in mergeresult.NO_OP_ACTIONS:
             mresult.removefile(f)  # merge does not affect file
         elif action[0] in nonconflicttypes:
             raise error.Abort(
@@ -564,6 +563,8 @@  class mergeresult(object):
     It has information about what actions need to be performed on dirstate
     mapping of divergent renames and other such cases. '''
 
+    NO_OP_ACTIONS = (mergestatemod.ACTION_KEEP,)
+
     def __init__(self):
         """
         filemapping: dict of filename as keys and action related info as values
@@ -711,12 +712,12 @@  class mergeresult(object):
                 a
                 not in (
                     mergestatemod.ACTION_GET,
-                    mergestatemod.ACTION_KEEP,
                     mergestatemod.ACTION_EXEC,
                     mergestatemod.ACTION_REMOVE,
                     mergestatemod.ACTION_PATH_CONFLICT_RESOLVE,
                 )
                 and self._actionmapping[a]
+                and a not in self.NO_OP_ACTIONS
             ):
                 return True
 
@@ -1376,7 +1377,7 @@  def applyupdates(
         # mergestate so that it can be reused on commit
         ms.addcommitinfo(f, op)
 
-    numupdates = mresult.len() - mresult.len((mergestatemod.ACTION_KEEP,))
+    numupdates = mresult.len() - mresult.len(mergeresult.NO_OP_ACTIONS)
     progress = repo.ui.makeprogress(
         _(b'updating'), unit=_(b'files'), total=numupdates
     )
diff --git a/mercurial/sparse.py b/mercurial/sparse.py
--- a/mercurial/sparse.py
+++ b/mercurial/sparse.py
@@ -399,7 +399,7 @@  def filterupdatesactions(repo, wctx, mct
             temporaryfiles.append(file)
             prunedactions[file] = action
         elif branchmerge:
-            if type != mergestatemod.ACTION_KEEP:
+            if type not in mergemod.mergeresult.NO_OP_ACTIONS:
                 temporaryfiles.append(file)
                 prunedactions[file] = action
         elif type == mergestatemod.ACTION_FORGET: