[PATCH] hooks: hook ordering via sort variable in [hooks]

[Matt Mackall]

On Sun, 2012-01-15 at 11:29 -0700, Matt Zuba wrote:
> How about something like this...
> 
> # HG changeset patch
> # User Matt Zuba <matt.zuba at goodwillaz.org>
> # Date 1326651917 25200
> # Node ID 88be2efad116b71479e204c5b2c17c33e081c7c2
> # Parent  e5feebc1f3bb50f0180c76d31e8456a390a86c78
> hooks: prioritize run order of hooks
> 
> As of Mercurial 1.3, hooks are sorted in the order they are read into
> Mercurial.  There are many instances when someone may want the hooks
> sorted in a specific order; this patch allows prioritizing hooks, while
> maintaining the existing enumeration for hooks without a priority.
> 
> diff --git a/mercurial/help/config.txt b/mercurial/help/config.txt
> --- a/mercurial/help/config.txt
> +++ b/mercurial/help/config.txt
> @@ -655,7 +655,10 @@
>   various actions such as starting or finishing a commit. Multiple
>   hooks can be run for the same action by appending a suffix to the
>   action. Overriding a site-wide hook can be done by changing its
> -value or setting it to an empty string.
> +value or setting it to an empty string.  Hooks can be prioritized
> +by adding a prefix of ``priority`` to the hook name on a new line
> +and setting the priority.  The default priority is 0 if
> +not specified.

Your emailer is mangling your patches.

>   Example ``.hg/hgrc``::
> 
> @@ -666,6 +669,8 @@
>     incoming =
>     incoming.email = /my/email/hook
>     incoming.autobuild = /my/build/hook
> +  # force autobuild hook to run before other incoming hooks
> +  priority.incoming.autobuild = 1
> 
>   Most hooks are run with environment variables set that give useful
>   additional information. For each hook below, the environment
> diff --git a/mercurial/hook.py b/mercurial/hook.py
> --- a/mercurial/hook.py
> +++ b/mercurial/hook.py
> @@ -6,7 +6,7 @@
>   # GNU General Public License version 2 or any later version.
> 
>   from i18n import _
> -import os, sys
> +import os, sys, heapq
>   import extensions, util
> 
>   def _pythonhook(ui, repo, name, hname, funcname, args, throw):
> @@ -124,6 +124,14 @@
>           ui.warn(_('warning: %s hook %s\n') % (name, desc))
>       return r
> 
> +def _allhooks(ui):
> +    hooks = []
> +    for hname, cmd in ui.configitems('hooks'):
> +        if hname.split('.')[0] != 'priority':
> +            priority = ui.configint('hooks', 'priority.%s' % hname, 0)
> +            heapq.heappush(hooks, (-priority, len(hooks), hname, cmd))
> +    return [(k, v) for priority, count, k, v in hooks]
> +

Using a heap here is overkill. I would do something like:

    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 [(n, c) for p, o, n, c in sorted(hooks)]


>   _redirect = False
>   def redirect(state):
>       global _redirect
> @@ -147,7 +155,7 @@
>               pass
> 
>       try:
> -        for hname, cmd in ui.configitems('hooks'):
> +        for hname, cmd in _allhooks(ui):
>               if hname.split('.')[0] != name or not cmd:
>                   continue
>               if util.safehasattr(cmd, '__call__'):
> diff --git a/tests/test-hook.t b/tests/test-hook.t
> --- a/tests/test-hook.t
> +++ b/tests/test-hook.t
> @@ -553,3 +553,19 @@
>     calling hook pre-identify: hooktests.verbosehook
>     verbose output from hook
>     cb9a9f314b8b
> +
> +Ensure hooks can be prioritized
> +
> +  $ echo '[hooks]' > .hg/hgrc
> +  $ echo 'pre-identify.a = python:hooktests.verbosehook' >> .hg/hgrc
> +  $ echo 'pre-identify.b = python:hooktests.verbosehook' >> .hg/hgrc
> +  $ echo 'priority.pre-identify.b = 1' >> .hg/hgrc
> +  $ echo 'pre-identify.c = python:hooktests.verbosehook' >> .hg/hgrc
> +  $ hg id --verbose
> +  calling hook pre-identify.b: hooktests.verbosehook
> +  verbose output from hook
> +  calling hook pre-identify.a: hooktests.verbosehook
> +  verbose output from hook
> +  calling hook pre-identify.c: hooktests.verbosehook
> +  verbose output from hook
> +  cb9a9f314b8b
> 
> 
> On 01/14/2012 03:43 PM, Matt Mackall wrote:
> > A better approach would be a way of adding hook priorities. Something
> > like:
> >
> > [hooks]
> > precommit.mine = blah
> > priority.precommit.mine = 1
> >
> > This lets people insert their hook in arbitrary places in the list.
> >
> > Note the trick with implementing this is using the existing enumeration
> > order as a secondary sort key.


-- 
Mathematics is the supreme nostalgia of our time.