Submitter | Pierre-Yves David |
---|---|
Date | April 15, 2016, 7:50 a.m. |
Message ID | <063993ec1c63c22ff616.1460706620@nodosa.octopoid.net> |
Download | mbox | patch |
Permalink | /patch/14636/ |
State | Superseded |
Headers | show |
Comments
On 15/04/2016 08:50, Pierre-Yves David wrote: > # HG changeset patch > # User Pierre-Yves David <pierre-yves.david@ens-lyon.org> > # Date 1460626126 25200 > # Thu Apr 14 02:28:46 2016 -0700 > # Node ID 063993ec1c63c22ff616609001b50292668bd92e > # Parent 8d398155bfda3a98e664af74096f0e405cc7dd09 > # EXP-Topic hooks > hook: small refactor in the way we compute the hooks list > > We are about to take untrusted hooks into account (to report them as failure) > so we need to rearrange the code a bit to allow config overwriting each other > in a later patch. > > diff -r 8d398155bfda -r 063993ec1c63 mercurial/hook.py > --- a/mercurial/hook.py Mon Feb 29 22:58:15 2016 +0900 > +++ b/mercurial/hook.py Thu Apr 14 02:28:46 2016 -0700 > @@ -162,12 +162,13 @@ > return r > > def _allhooks(ui): > - hooks = [] > + hooks = {} > for name, cmd in ui.configitems('hooks'): > if not name.startswith('priority'): > priority = ui.configint('hooks', 'priority.%s' % name, 0) > - hooks.append((-priority, len(hooks), name, cmd)) > - return [(k, v) for p, o, k, v in sorted(hooks)] > + hooks[name] = (-priority, len(hooks), cmd) > + order = sorted(hooks, key=lambda x: hooks[x][:2] + (x,)) > + return [(k, hooks[k][2]) for k in order] I'd find this easier to review as: hooks[name] = (-priority, len(hooks), name, cmd) return [(k, v) for p, o, k, v in sorted(hooks.values())] with similar changes to the other two patches. That way, it's obvious that the behaviour isn't changing until patch 3, and there's less counting of elements to do. > > _redirect = False > def redirect(state): > _______________________________________________ > Mercurial-devel mailing list > Mercurial-devel@mercurial-scm.org > https://urldefense.proofpoint.com/v2/url?u=https-3A__www.mercurial-2Dscm.org_mailman_listinfo_mercurial-2Ddevel&d=CwIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=mEgSWILcY4c4W3zjApBQLA&m=1nPp7BXuxnlF6QBfJE7KJ-7-fQ215xgb6pApq6RxM_E&s=cwskb1ilApEaJL4pTniUVp4NIv7KaEiu2u5BpGweq3A&e= >
> We are about to take untrusted hooks into account (to report them as
failure)
As failures
On 04/15/2016 04:26 AM, Simon Farnsworth wrote: > On 15/04/2016 08:50, Pierre-Yves David wrote: >> # HG changeset patch >> # User Pierre-Yves David <pierre-yves.david@ens-lyon.org> >> # Date 1460626126 25200 >> # Thu Apr 14 02:28:46 2016 -0700 >> # Node ID 063993ec1c63c22ff616609001b50292668bd92e >> # Parent 8d398155bfda3a98e664af74096f0e405cc7dd09 >> # EXP-Topic hooks >> hook: small refactor in the way we compute the hooks list >> >> We are about to take untrusted hooks into account (to report them as >> failure) >> so we need to rearrange the code a bit to allow config overwriting >> each other >> in a later patch. >> >> diff -r 8d398155bfda -r 063993ec1c63 mercurial/hook.py >> --- a/mercurial/hook.py Mon Feb 29 22:58:15 2016 +0900 >> +++ b/mercurial/hook.py Thu Apr 14 02:28:46 2016 -0700 >> @@ -162,12 +162,13 @@ >> return r >> >> def _allhooks(ui): >> - hooks = [] >> + hooks = {} >> for name, cmd in ui.configitems('hooks'): >> if not name.startswith('priority'): >> priority = ui.configint('hooks', 'priority.%s' % name, 0) >> - hooks.append((-priority, len(hooks), name, cmd)) >> - return [(k, v) for p, o, k, v in sorted(hooks)] >> + hooks[name] = (-priority, len(hooks), cmd) >> + order = sorted(hooks, key=lambda x: hooks[x][:2] + (x,)) >> + return [(k, hooks[k][2]) for k in order] > > I'd find this easier to review as: > > hooks[name] = (-priority, len(hooks), name, cmd) > return [(k, v) for p, o, k, v in sorted(hooks.values())] > > with similar changes to the other two patches. That way, it's obvious > that the behaviour isn't changing until patch 3, and there's less > counting of elements to do. Yeah, I agree. I'll send a V2.
Patch
diff -r 8d398155bfda -r 063993ec1c63 mercurial/hook.py --- a/mercurial/hook.py Mon Feb 29 22:58:15 2016 +0900 +++ b/mercurial/hook.py Thu Apr 14 02:28:46 2016 -0700 @@ -162,12 +162,13 @@ return r def _allhooks(ui): - hooks = [] + hooks = {} for name, cmd in ui.configitems('hooks'): if not name.startswith('priority'): priority = ui.configint('hooks', 'priority.%s' % name, 0) - hooks.append((-priority, len(hooks), name, cmd)) - return [(k, v) for p, o, k, v in sorted(hooks)] + hooks[name] = (-priority, len(hooks), cmd) + order = sorted(hooks, key=lambda x: hooks[x][:2] + (x,)) + return [(k, hooks[k][2]) for k in order] _redirect = False def redirect(state):