[PATCH 1 of 4] histedit: make verification configurable
Augie Fackler
raf at durin42.com
Tue Nov 17 18:56:16 UTC 2015
On Mon, Nov 16, 2015 at 11:10:42PM +0900, Yuya Nishihara wrote:
> On Thu, 12 Nov 2015 17:45:56 -0800, Mateusz Kwapich wrote:
> > # HG changeset patch
> > # User Mateusz Kwapich <mitrandir at fb.com>
> > # Date 1447375340 28800
> > # Thu Nov 12 16:42:20 2015 -0800
> > # Node ID 659c8c265d8516a5685ac226abc91aaa79028202
> > # Parent f9984f76fd90e439221425d751e29bae17bec995
> > histedit: make verification configurable
> >
> > Before we can add a 'base' action to histedit need to change verification
> > so that action can specify which steps of verification should run for it.
> >
> > Also it's everything we need for the exec and stop actions implementation.
> >
> > I thought about baking verification into each histedit action (so each
> > of them is responsible for verifying its constraints) but it felt wrong
> > because:
> > - every action would need to know its context (eg. the list of all other
> > actions)
> > - a lot of duplicated work will be added - each action will iterate through
> > all others
> > - the steps of the verification would need to be extracted and named anyway
> > in order to be reused
> >
> > diff --git a/hgext/histedit.py b/hgext/histedit.py
> > --- a/hgext/histedit.py
> > +++ b/hgext/histedit.py
> > @@ -334,6 +334,24 @@
> > raise error.Abort(_('unknown changeset %s listed') % rulehash[:12])
> > return cls(state, node)
> >
> > + def constraints(self):
> > + """Return a set of constrains that this action should be verified for
> > +
> > + Available constraints:
> > + noduplicates - aborts if there are multiple rules for one node
> > + noother - abort if the node doesn't belong to edited stack
> > + """
> > +
> > + return set(['noduplicates', 'noother'])
> > +
> > + def nodetoverify(self):
> > + """Returns a node associated with the action that will be used for
> > + verification purposes.
> > +
> > + If the action doesn't correspond to node it should return None
> > + """
> > + return self.node
> > +
> > def run(self):
> > """Runs the action. The default behavior is simply apply the action's
> > rulectx onto the current parentctx."""
> > @@ -807,7 +825,7 @@
> > f.close()
> > rules = [l for l in (r.strip() for r in rules.splitlines())
> > if l and not l.startswith('#')]
> > - rules = verifyrules(rules, repo, [repo[c] for [_a, c] in state.rules])
> > + rules = verifyrules(rules, state, [repo[c] for [_a, c] in state.rules])
> > state.rules = rules
> > state.write()
> > return
> > @@ -888,7 +906,7 @@
> > f.close()
> > rules = [l for l in (r.strip() for r in rules.splitlines())
> > if l and not l.startswith('#')]
> > - rules = verifyrules(rules, repo, ctxs)
> > + rules = verifyrules(rules, state, ctxs)
> >
> > parentctxnode = repo[root].parents()[0].node()
> >
> > @@ -1038,7 +1056,7 @@
> >
> > return rules
> >
> > -def verifyrules(rules, repo, ctxs):
> > +def verifyrules(rules, state, ctxs):
> > """Verify that there exists exactly one edit rule per given changeset.
> >
> > Will abort if there are to many or too few rules, a malformed rule,
> > @@ -1050,22 +1068,25 @@
> > for r in rules:
> > if ' ' not in r:
> > raise error.Abort(_('malformed line "%s"') % r)
> > - action, rest = r.split(' ', 1)
> > - ha = rest.strip().split(' ', 1)[0]
> > - try:
> > - ha = repo[ha].hex()
> > - except error.RepoError:
> > - raise error.Abort(_('unknown changeset %s listed') % ha[:12])
> > - if ha not in expected:
> > - raise error.Abort(
> > - _('may not use changesets other than the ones listed'))
> > - if ha in seen:
> > - raise error.Abort(_('duplicated command for changeset %s') %
> > - ha[:12])
> > - seen.add(ha)
> > - if action not in actiontable or action.startswith('_'):
> > - raise error.Abort(_('unknown action "%s"') % action)
> > - parsed.append([action, ha])
> > + verb, rest = r.split(' ', 1)
> > +
> > + if verb not in actiontable or verb.startswith('_'):
> > + raise error.Abort(_('unknown action "%s"') % verb)
> > + action = actiontable[verb].fromrule(state, rest)
> > + constraints = action.constraints()
> > + nodetoverify = action.nodetoverify()
> > + if nodetoverify is not None:
> > + ha = node.hex(nodetoverify)
> > + if 'noother' in constraints and ha not in expected:
> > + raise error.Abort(
> > + _('may not use "%s" with changesets '
> > + 'other than the ones listed' % verb))
> > + if 'noduplicates' in constraints and ha in seen:
> > + raise error.Abort(_('duplicated command for changeset %s') %
> > + ha[:12])
>
> Can we detect unhandled constraints in some way? I think that is a common
> mistake we do when we (ab)use a string as an enum-like constant.
Yes, we should hard-fail if there's a constraint that we don't know about.
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at selenic.com
> https://selenic.com/mailman/listinfo/mercurial-devel
More information about the Mercurial-devel
mailing list