[feature] install only enabled hooks

[CircleCode]

trafficstars

when running in docker mode, every hook has to spawn a new container, even if the corresponding captain hook is disabled. This is not a big issue, but in scenarios involving multiple hooks, the time allocated to starting a container per hook to print "hook disabled" is not negligible.

The user can explicitely choose hooks to install with the --hook option, but this only accepts a single hook name

An additional option like --only-activated could be useful in such scenarii.

The drawback I can see is that any change to captainhook.json is not automatically propagated to hooks, and the doc has to be clear about this (We can even suggest an action running captainhook install every time the config file has changed).

If this sounds a reasonable feature to you, I am willing to contribute with a PR.

[sebastianfeldmann]

Sounds interesting, especially the part with the --only-activated option.

I'm not sure about the action. It would only work if you commit changes to the captainhook.json so I would argue that we do not have to provide such an action or any special hints.

[CircleCode]

Seems ok for me for adding the --only-activated option.

I have no ETA for this, but I think I will work on it in the coming weeks

[xwillq]

Hey! I found different approach for fixing this problem. I use this as my run-exec:

[ $(jq ".[\"$(basename $0)\"].enabled" captainhook.json) != 'true' ] || <docker command>

It checks if hook is enabled before running the command, so container won't start if it's not. Similar check could be added to generated hooks by default, although it should use some other approach that doesn't depend on external programs.

[alexandrmazur96]

Currently, hooks installation is really painful. We use only pre-commit & pre-push.

Our hooks work over docker. So now you are not only required to have run containers just to make a git checkout, but each other git operation is slowed because for checking is hook enabled or not script goes to docker then to the captainhook binary, and that's only to receive:

[alexandrmazur96]

Okay, I found a workaround for my issue. It could be done via:

vendor/bin/captainhook install -f -s pre-commit and vendor/bin/captainhook install -f -s pre-push

[CircleCode]

Hi, after almost a year, I am about to introduce again captainhook in our company. Amongst other features, this is one I would like to contribute to.

I started to design the feature, and it appears to me the only challenging part will be to reinvent a way to detect if a hook is enabled or not:

I thought about using \CaptainHook\App\Runner\Hook::isAnyConfigEnabled, but

1. it is private
2. it depends on a call to \CaptainHook\App\Runner\Hook::getHookConfigsToHandle which is also private
3. it would need to build an instance of every \CaptainHook\App\Hooks::nativeHooks (not a big deal as we are not in a quest for micro optimizations during installation)

@sebastianfeldmann would you go with

• making both methods public?
• writing another public method like \CaptainHook\App\Runner\Hook::isHookEnabled to wrap these two methods
• reading the conf to get if a hook is enabled without the need to instanciate it (note that it should also duplicate the handling of virtual hooks for example)

[sebastianfeldmann]

I would suggest to add some isHookEnabled functionality to the Config class (Config::isHookEnabled).

Then the method Runner\Installer::getHooksToInstall can return the correct hooks.

Currently it returns all hooks or only a specific one.

The idea would be if --only-activated is set to remove the ones that aren't enabled

public function getHooksToInstall(): array
{

    if (!empty($this->hookToHandle) {
        return [$this->hookToHandle => false];
    }

    $hooks = array_map(fn($hook) => true, Hooks::nativeHooks());

    if ($this->onlyInstallActivatedHooks) {
        foreach (array_keys($hooks) as $hook) {
            if (!$this->config->isHookEnabled($hook)) {
                unset($hooks[$hook]);
            }
        }
    }
    return $hooks;
}

The tricky thing would be how to handle virtual hooks. Because if the virtual hook post-change is enabled we have to enable all native hooks which trigger the virtual one like post-checkout, post-rewrite and post-merge.

[CircleCode]

I would suggest to add some isHookEnabled functionality to the Config class (Config::isHookEnabled).

Then the method Runner\Installer::getHooksToInstall can return the correct hooks.

Currently it returns all hooks or only a specific one.

Will definitely go this way (and probably also use this method in \CaptainHook\App\Runner\Hook::isAnyConfigEnabled to avoid code duplication)

The tricky thing would be how to handle virtual hooks. Because if the virtual hook post-change is enabled we have to enable all native hooks which trigger the virtual one like post-checkout, post-rewrite and post-merge.

Got it, this is exactly what I was implying when I said

note that it should also duplicate the handling of virtual hooks for example

Thanks for the insights :-)

[CircleCode]

@sebastianfeldmann Am I right if I say that I still have to update the doc in the gh-pages branch? And are these html files generated, or hand-written?

[sebastianfeldmann]

They are hand written :) I'm currently on it because I had to update some other stuff as well that I forgot to bring up to date.

So I will take care of it. Thanks for putting in the work, it is greatly appreciated :)