Patchwork [6,of,9] histedit: pass state to processreplacement

login
register
mail settings
Submitter David Soria Parra
Date Oct. 16, 2014, 5:34 p.m.
Message ID <638453c54662a68d4243.1413480882@davidsp-mbp.dhcp.thefacebook.com>
Download mbox | patch
Permalink /patch/6344/
State Accepted
Headers show

Comments

David Soria Parra - Oct. 16, 2014, 5:34 p.m.
# HG changeset patch
# User David Soria Parra <davidsp@fb.com>
# Date 1413479209 25200
#      Thu Oct 16 10:06:49 2014 -0700
# Node ID 638453c54662a68d4243976c36712f0ac3ea272d
# Parent  cbe0c2c08822ca9a36463c385601ecd9a5bb53e2
histedit: pass state to processreplacement
Olle Lundberg - Oct. 16, 2014, 6:15 p.m.
On Thu, Oct 16, 2014 at 7:34 PM, David Soria Parra <davidsp@fb.com> wrote:

> # HG changeset patch
> # User David Soria Parra <davidsp@fb.com>
> # Date 1413479209 25200
> #      Thu Oct 16 10:06:49 2014 -0700
> # Node ID 638453c54662a68d4243976c36712f0ac3ea272d
> # Parent  cbe0c2c08822ca9a36463c385601ecd9a5bb53e2
> histedit: pass state to processreplacement
>
> diff --git a/hgext/histedit.py b/hgext/histedit.py
> --- a/hgext/histedit.py
> +++ b/hgext/histedit.py
> @@ -572,8 +572,7 @@
>          state = bootstrapcontinue(ui, state, opts)
>      elif goal == 'abort':
>          state = readstate(repo)
> -        mapping, tmpnodes, leafs, _ntm = processreplacement(repo,
> -                state.replacements)
> +        mapping, tmpnodes, leafs, _ntm = processreplacement(repo, state)
>
The repo parameter becomes unnecessary here since you keep a reference to
the repo in the state object being sent in?
For consistencies sake it would be nice if you could assign it like in
patch cbe0c2c08822ca9a36463c385601ecd9a5bb53e2

         ui.debug('restore wc to old parent %s\n' %
> node.short(state.topmost))
>          # check whether we should update away
>          parentnodes = [c.node() for c in repo[None].parents()]
> @@ -649,8 +648,7 @@
>
>      hg.update(repo, state.parentctx.node())
>
> -    mapping, tmpnodes, created, ntm = processreplacement(repo,
> -            state.replacements)
> +    mapping, tmpnodes, created, ntm = processreplacement(repo, state)
>
same.

>      if mapping:
>          for prec, succs in mapping.iteritems():
>              if not succs:
> @@ -841,12 +839,13 @@
>                           hint=_('do you want to use the drop action?'))
>      return parsed
>
> -def processreplacement(repo, replacements):
> +def processreplacement(repo, state):
>
def processreplacement(state):

>      """process the list of replacements to return
>
>      1) the final mapping between original and created nodes
>      2) the list of temporary node created by histedit
>      3) the list of new commit created by histedit"""
> +    replacements = state.replacements
>
repo = state.repo

>      allsuccs = set()
>      replaced = set()
>      fullmapping = {}
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
>
David Soria Parra - Oct. 16, 2014, 6:24 p.m.
Olle <olle.lundberg@gmail.com> writes:

> On Thu, Oct 16, 2014 at 7:34 PM, David Soria Parra <davidsp@fb.com>
> wrote:
>
>     # HG changeset patch
>     # User David Soria Parra <davidsp@fb.com>
>     # Date 1413479209 25200
>     # Thu Oct 16 10:06:49 2014 -0700
>     # Node ID 638453c54662a68d4243976c36712f0ac3ea272d
>     # Parent cbe0c2c08822ca9a36463c385601ecd9a5bb53e2
>     histedit: pass state to processreplacement
>     
>     diff --git a/hgext/histedit.py b/hgext/histedit.py
>     --- a/hgext/histedit.py
>     +++ b/hgext/histedit.py
>     @@ -572,8 +572,7 @@
>     state = bootstrapcontinue(ui, state, opts)
>     elif goal == 'abort':
>     state = readstate(repo)
>     - mapping, tmpnodes, leafs, _ntm = processreplacement(repo,
>     - state.replacements)
>     + mapping, tmpnodes, leafs, _ntm = processreplacement(repo, state)
> The repo parameter becomes unnecessary here since you keep a reference
> to the repo in the state object being sent in?
> For consistencies sake it would be nice if you could assign it like in
> patch cbe0c2c08822ca9a36463c385601ecd9a5bb53e2

good catch. will wait for augie and pierre-yves to review the rest of
the series and send a v2 with that change

Patch

diff --git a/hgext/histedit.py b/hgext/histedit.py
--- a/hgext/histedit.py
+++ b/hgext/histedit.py
@@ -572,8 +572,7 @@ 
         state = bootstrapcontinue(ui, state, opts)
     elif goal == 'abort':
         state = readstate(repo)
-        mapping, tmpnodes, leafs, _ntm = processreplacement(repo,
-                state.replacements)
+        mapping, tmpnodes, leafs, _ntm = processreplacement(repo, state)
         ui.debug('restore wc to old parent %s\n' % node.short(state.topmost))
         # check whether we should update away
         parentnodes = [c.node() for c in repo[None].parents()]
@@ -649,8 +648,7 @@ 
 
     hg.update(repo, state.parentctx.node())
 
-    mapping, tmpnodes, created, ntm = processreplacement(repo,
-            state.replacements)
+    mapping, tmpnodes, created, ntm = processreplacement(repo, state)
     if mapping:
         for prec, succs in mapping.iteritems():
             if not succs:
@@ -841,12 +839,13 @@ 
                          hint=_('do you want to use the drop action?'))
     return parsed
 
-def processreplacement(repo, replacements):
+def processreplacement(repo, state):
     """process the list of replacements to return
 
     1) the final mapping between original and created nodes
     2) the list of temporary node created by histedit
     3) the list of new commit created by histedit"""
+    replacements = state.replacements
     allsuccs = set()
     replaced = set()
     fullmapping = {}