[PATCH 7 of 7] histedit: use scmutil.cleanupnodes (BC)

Martin von Zweigbergk martinvonz at google.com
Sun Jul 9 15:54:50 EDT 2017


On Sun, Jul 9, 2017 at 12:21 PM, Jun Wu <quark at fb.com> wrote:
> Excerpts from Martin von Zweigbergk's message of 2017-07-09 12:04:11 -0700:
>> On Sat, Jul 8, 2017 at 4:51 PM, Jun Wu <quark at fb.com> wrote:
>> > diff --git a/hgext/histedit.py b/hgext/histedit.py
>> > --- a/hgext/histedit.py
>> > +++ b/hgext/histedit.py
>> > @@ -1182,5 +1182,9 @@ def _finishhistedit(ui, repo, state):
>> >          mapping[n] = ()
>> >
>> > -    safecleanupnode(ui, repo, mapping)
>> > +    # remove entries about unknown nodes
>> > +    nodemap = repo.unfiltered().changelog.nodemap
>> > +    mapping = {k: v for k, v in mapping.items()
>> > +               if k in nodemap and all(n in nodemap for n in v)}
>>
>> I suppose these few lines could potentially move into cleanupnodes?
>> IIUC, it's to prevent crashing if you had stripped nodes during
>> histedit. The same can be done during rebase (while stopped to resolve
>> conflicts) and
>
> The current rebase and amend do not have the "remove unknown node" logic
> (might be a bug for rebase, but amend does not seem to need this). Since
> it's only useful in a subset of all cases, and commands like metaedit,
> split, amend, etc do not need it, I think it's cleaner to require explicit
> node removal from the callsite, so when the callsite made a mistake, we can
> catch it instead of ignoring it silently.

I agree that amend and metaedit should not need it. I could be made an
option, since at least rebase and histedit seem to need it. Anyway, we
can always deal with that when someone files a bug report.

>
>> > +    scmutil.cleanupnodes(repo, mapping, 'histedit')
>> >
>> >      state.clear()


More information about the Mercurial-devel mailing list