Patchwork [1,of,5] mergestate: make filename argument optional in _mergestate_base.extras()

login
register
mail settings
Submitter Pulkit Goyal
Date Oct. 13, 2020, 7:33 a.m.
Message ID <ca34efec098e04ef3934.1602574407@workspace>
Download mbox | patch
Permalink /patch/47437/
State Accepted
Headers show

Comments

Pulkit Goyal - Oct. 13, 2020, 7:33 a.m.
# HG changeset patch
# User Pulkit Goyal <7895pulkit@gmail.com>
# Date 1602313984 -19800
#      Sat Oct 10 12:43:04 2020 +0530
# Node ID ca34efec098e04ef39341ef181c038f4a4522148
# Parent  64a9423450efb39d7f1bc5b6b2cca1efafb1870e
# EXP-Topic merge-newnode-final
mergestate: make filename argument optional in _mergestate_base.extras()

Earlier `extras()` can only be used for getting extra for a file. However at
couple of places in code, we wanted to iterate over all the extras stored with
the mergestate and they were accessing the private `_stateextras`.

Now, if filename is not passed, we return all the extras.

Differential Revision: https://phab.mercurial-scm.org/D9190
Yuya Nishihara - Oct. 13, 2020, 10:50 a.m.
On Tue, 13 Oct 2020 13:03:27 +0530, Pulkit Goyal wrote:
> # HG changeset patch
> # User Pulkit Goyal <7895pulkit@gmail.com>
> # Date 1602313984 -19800
> #      Sat Oct 10 12:43:04 2020 +0530
> # Node ID ca34efec098e04ef39341ef181c038f4a4522148
> # Parent  64a9423450efb39d7f1bc5b6b2cca1efafb1870e
> # EXP-Topic merge-newnode-final
> mergestate: make filename argument optional in _mergestate_base.extras()

> diff --git a/mercurial/mergestate.py b/mercurial/mergestate.py
> --- a/mercurial/mergestate.py
> +++ b/mercurial/mergestate.py
> @@ -305,7 +305,12 @@ class _mergestate_base(object):
>              ):
>                  yield f
>  
> -    def extras(self, filename):
> +    def extras(self, filename=None):
> +        """ return extras stored with the mergestate
> +
> +        if filename is passed, extras for that file is only returned """
> +        if filename is None:
> +            return self._stateextras
>          return self._stateextras[filename]

How about adding a separate function? I don't like this kind of overloads
because different return types typically mean the function will serve
differently.

Patch

diff --git a/mercurial/commit.py b/mercurial/commit.py
--- a/mercurial/commit.py
+++ b/mercurial/commit.py
@@ -147,7 +147,7 @@  def _prepare_files(tr, ctx, error=False,
         # some cases.
         ms = mergestate.mergestate.read(repo)
         if ms.active():
-            for fname in sorted(ms._stateextras.keys()):
+            for fname in sorted(ms.extras().keys()):
                 might_removed = ms.extras(fname).get(b'merge-removal-candidate')
                 if might_removed == b'yes':
                     if fname in ctx:
diff --git a/mercurial/debugcommands.py b/mercurial/debugcommands.py
--- a/mercurial/debugcommands.py
+++ b/mercurial/debugcommands.py
@@ -2107,7 +2107,7 @@  def debugmergestate(ui, repo, *args, **o
     fm_files.end()
 
     fm_extras = fm.nested(b'extras')
-    for f, d in sorted(pycompat.iteritems(ms._stateextras)):
+    for f, d in sorted(pycompat.iteritems(ms.extras())):
         if f in ms:
             # If file is in mergestate, we have already processed it's extras
             continue
diff --git a/mercurial/mergestate.py b/mercurial/mergestate.py
--- a/mercurial/mergestate.py
+++ b/mercurial/mergestate.py
@@ -305,7 +305,12 @@  class _mergestate_base(object):
             ):
                 yield f
 
-    def extras(self, filename):
+    def extras(self, filename=None):
+        """ return extras stored with the mergestate
+
+        if filename is passed, extras for that file is only returned """
+        if filename is None:
+            return self._stateextras
         return self._stateextras[filename]
 
     def _resolve(self, preresolve, dfile, wctx):