martinvonz added a comment.
I'll drop this patch from the queue because I think there's a lot that can be cleaned up. I tried to re-unify `_fixdirstate()` and `_uncommitdirstate()` myself. This is my first step: diff --git a/hgext/uncommit.py b/hgext/uncommit.py --- a/hgext/uncommit.py +++ b/hgext/uncommit.py @@ -138,7 +138,7 @@ def _fixdirstate(repo, oldctx, newctx, m ds.copy(src, dst) -def _uncommitdirstate(repo, oldctx, match, interactive): +def _uncommitdirstate(repo, oldctx, newctx, match, interactive): """Fix the dirstate after switching the working directory from oldctx to a copy of oldctx not containing changed files matched by match. @@ -217,21 +217,13 @@ def _uncommitdirstate(repo, oldctx, matc ds.remove(f) # Merge old parent and old working dir copies - oldcopies = {} - if interactive: - # Interactive had different meaning of the variables so restoring the - # original meaning to use them - m, a, r = repo.status(oldctx.p1(), oldctx, match=match)[:3] - for f in (m + a): - src = oldctx[f].renamed() - if src: - oldcopies[f] = src[0] + oldcopies = copiesmod.pathcopies(newctx, oldctx, match) oldcopies.update(copies) copies = dict((dst, oldcopies.get(src, src)) for dst, src in oldcopies.iteritems()) # Adjust the dirstate copies for dst, src in copies.iteritems(): - if (src not in ctx or dst in ctx or ds[dst] != 'a'): + if (src not in newctx or dst in newctx or ds[dst] != 'a'): src = None ds.copy(src, dst) @@ -285,11 +277,11 @@ def uncommit(ui, repo, *pats, **opts): # Fully removed the old commit mapping[old.node()] = () - scmutil.cleanupnodes(repo, mapping, 'uncommit', fixphase=True) - with repo.dirstate.parentchange(): repo.dirstate.setparents(newid, node.nullid) - _uncommitdirstate(repo, old, match, interactive) + _uncommitdirstate(repo, old, repo[newid], match, interactive) + + scmutil.cleanupnodes(repo, mapping, 'uncommit', fixphase=True) def _interactiveuncommit(ui, repo, old, match): """ The function which contains all the logic for interactively uncommiting Is there a reason that's broken? Tests seem to pass (test-uncommit.t passes). If not, could you try to continue in the direction of unifying the two method again? INLINE COMMENTS > uncommit.py:141 > + > +def _uncommitdirstate(repo, oldctx, match, interactive): > + """Fix the dirstate after switching the working directory from Much of this duplicates `_fixdirstate()`. We must be able to share some of this code. > uncommit.py:273 > + if interactive: > + match = scmutil.match(old, pats, opts) > + newid = _interactiveuncommit(ui, repo, old, match) Unnecessary (done on line 269) > uncommit.py:288 > > + scmutil.cleanupnodes(repo, mapping, 'uncommit', fixphase=True) > + Why did this moved here? I just moved it down in https://phab.mercurial-scm.org/D5660. I really should have written a better commit message explaining what the problem was. But it's also unclear why it should be moved back. REPOSITORY rHG Mercurial REVISION DETAIL https://phab.mercurial-scm.org/D5792 To: taapas1128, #hg-reviewers, durin42 Cc: martinvonz, durin42, pulkit, lothiraldan, mercurial-devel _______________________________________________ Mercurial-devel mailing list Mercurial-devel@mercurial-scm.org https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel