Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow running only actions specified on the command line #137

Draft
wants to merge 10 commits into
base: main
Choose a base branch
from
85 changes: 83 additions & 2 deletions src/Console/Command/Hook.php
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
use CaptainHook\App\Config;
use CaptainHook\App\Console\IOUtil;
use CaptainHook\App\Hook\Util;
use CaptainHook\App\Runner\Hook as RunnerHook;
use Exception;
use RuntimeException;
use Symfony\Component\Console\Input\InputInterface;
Expand Down Expand Up @@ -55,6 +56,50 @@ protected function configure(): void
InputOption::VALUE_OPTIONAL,
'Relative path from your config file to your bootstrap file'
);

$this->addOption(
'list-actions',
'l',
InputOption::VALUE_NONE,
'List actions for this hook without running the hook'
);

$this->addOption(
'action',
'a',
InputOption::VALUE_IS_ARRAY | InputOption::VALUE_REQUIRED,
'Run only the actions listed'
);

$this->addOption(
'disable-plugins',
null,
InputOption::VALUE_NONE,
'Disable all hook plugins'
);
}

/**
* Initialize the command by checking/modifying inputs before validation
*
* @param \Symfony\Component\Console\Input\InputInterface $input
* @param \Symfony\Component\Console\Output\OutputInterface $output
* @return void
*/
protected function initialize(InputInterface $input, OutputInterface $output): void
{
// If `--list-actions` is present, we will ignore any arguments, since
// this option intends to output a list of actions for the hook without
// running the hook. So, if any arguments are required but not present
// in the input, we will set them to an empty string in the input to
// suppress any validation errors.
if ($input->getOption('list-actions') === true) {
foreach ($this->getDefinition()->getArguments() as $arg) {
if ($arg->isRequired() && $input->getArgument($arg->getName()) === null) {
$input->setArgument($arg->getName(), '');
}
}
}
Comment on lines +97 to +103
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think about creating three separate commands (e.g. hook:pre-commit:list-actions) instead of adding those as options with need for this magic?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the idea of separating the list functionality from the execution functionality
I think one command is enough captainhook list-actions pre-commit :)
Then this can be done in a separate Runner\ListActions
Maybe we can move the getHookConfigsToHandle functionality to the Config class. This way we would not have duplicate code in Runner\Hook and Runner\ListActions

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ramsey have you seen this comment as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This sounds good to me.

}

/**
Expand All @@ -76,13 +121,23 @@ protected function execute(InputInterface $input, OutputInterface $output): int

// use ansi coloring only if not disabled in captainhook.json
$output->setDecorated($config->useAnsiColors());
// use the configured verbosity to manage general output verbosity
$output->setVerbosity(IOUtil::mapConfigVerbosity($config->getVerbosity()));

// If the verbose option is present on the command line, then use it.
// Otherwise, use the verbosity setting from the configuration.
if (!$input->hasOption('verbose') || !$input->getOption('verbose')) {
$output->setVerbosity(IOUtil::mapConfigVerbosity($config->getVerbosity()));
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did this because I found that captainhook didn't respect the verbosity option (i.e., -v, -vv, and -vvv) passed on the command line. It was always setting the output to whatever was configured in captainhook.json.

IMO, if you pass verbosity on the command line, it should override whatever is configured in captainhook.json.


$class = '\\CaptainHook\\App\\Runner\\Hook\\' . Util::getHookCommand($this->hookName);
/** @var \CaptainHook\App\Runner\Hook $hook */
$hook = new $class($io, $config, $repository);

// If list-actions is true, then list the hook actions instead of running them.
if ($input->getOption('list-actions') === true) {
$this->listActions($output, $hook);
return 0;
}

try {
$hook->run();
return 0;
Expand Down Expand Up @@ -128,4 +183,30 @@ private function handleError(OutputInterface $output, Exception $e): int

return 1;
}

/**
* Print out a list of actions for this hook
*
* @param OutputInterface $output
* @param RunnerHook $hook
*/
private function listActions(OutputInterface $output, RunnerHook $hook): void
{
$output->writeln('<comment>Listing ' . $hook->getName() . ' actions:</comment>');

if (!$hook->isEnabled()) {
$output->writeln(' - hook is disabled');
return;
}

$actions = $hook->getActions();
if (count($actions) === 0) {
$output->writeln(' - no actions configured');
return;
}

foreach ($actions as $action) {
$output->writeln(" - <fg=blue>{$action->getAction()}</>");
}
}
}
22 changes: 22 additions & 0 deletions src/Console/IO/Base.php
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,28 @@ public function getArgument(string $name, string $default = ''): string
return $default;
}

/**
* Return the original cli options
*
* @return array
*/
public function getOptions(): array
{
return [];
}

/**
* Return the original cli option or a given default
*
* @param string $name
* @param string|string[]|bool|null $default
* @return string|string[]|bool|null
*/
public function getOption(string $name, $default = null)
{
return $default;
}

/**
* Return the piped in standard input
*
Expand Down
22 changes: 22 additions & 0 deletions src/Console/IO/DefaultIO.php
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,28 @@ public function getArgument(string $name, string $default = ''): string
return (string)($this->getArguments()[$name] ?? $default);
}

/**
* Return the original cli options
*
* @return array
*/
public function getOptions(): array
{
return $this->input->getOptions();
}

/**
* Return the original cli option or a given default
*
* @param string $name
* @param string|string[]|bool|null $default
* @return string|string[]|bool|null
*/
public function getOption(string $name, $default = null)
{
return $this->getOptions()[$name] ?? $default;
}

/**
* Return the piped in standard input
*
Expand Down
1 change: 1 addition & 0 deletions src/Hook/File/Action/Check.php
Original file line number Diff line number Diff line change
Expand Up @@ -92,6 +92,7 @@ public function execute(Config $config, IO $io, Repository $repository, Config\A
* Setup the action, reading and validating all config settings
*
* @param \CaptainHook\App\Config\Options $options
* @codeCoverageIgnore
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method does nothing here. It only has a comment in it, but it was showing up in coverage reports as an uncovered method.

*/
protected function setUp(Config\Options $options): void
{
Expand Down
109 changes: 92 additions & 17 deletions src/Runner/Hook.php
Original file line number Diff line number Diff line change
Expand Up @@ -114,12 +114,19 @@ public function run(): void
{
$hookConfigs = $this->getHookConfigsToHandle();

$this->io->write('<comment>' . $this->hook . ':</comment> ');
/** @var IO\Base $io */
$io = $this->io;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is for the benefit of PHPStan, since getOption() is not on the Console\IO interface.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we need the getOption method. I think it would be better to handle the option stuff on the outside (inside the Command) and set an explicit setting on the runner with $runner->setActions or $runner->disablePlugins ...
This way we could get rid of the option methods.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll give this a try. I didn't like adding the getOption() method. 🙂


$io->write('<comment>' . $this->hook . ':</comment> ');

if ($io->getOption('disable-plugins')) {
$io->write('<fg=magenta>Running with plugins disabled</>');
}

// if the hook and all triggered virtual hooks
// are NOT enabled in the captainhook configuration skip the execution
if (!$this->isAnyConfigEnabled($hookConfigs)) {
$this->io->write(' - hook is disabled');
$io->write(' - hook is disabled');
return;
}

Expand All @@ -128,7 +135,7 @@ public function run(): void
$actions = $this->getActionsToExecute($hookConfigs);
// are any actions configured
if (count($actions) === 0) {
$this->io->write(' - no actions to execute', true);
$io->write(' - no actions to execute', true);
} else {
$this->executeActions($actions);
}
Expand All @@ -153,6 +160,16 @@ public function getHookConfigsToHandle(): array
return $configs;
}

/**
* Returns true if this hook or any applicable virtual hooks are enabled
*
* @return bool
*/
public function isEnabled(): bool
{
return $this->isAnyConfigEnabled($this->getHookConfigsToHandle());
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This allows the hook command class access to know whether the hook is enabled.


/**
* @param \CaptainHook\App\Config\Hook[] $configs
* @return bool
Expand Down Expand Up @@ -194,6 +211,16 @@ public function shouldSkipActions(?bool $shouldSkip = null): bool
return $this->skipActions;
}

/**
* Returns all actions configured for this hook or applicable virtual hooks
*
* @return \CaptainHook\App\Config\Action[]
*/
public function getActions(): array
{
return $this->getActionsToExecute($this->getHookConfigsToHandle());
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This allows the hook command class access to the list of actions configured for the hook, so it may use these to list the actions, when --list-actions is provided.


/**
* Return all the actions to execute
*
Expand Down Expand Up @@ -283,18 +310,9 @@ private function executeFailAfterAllActions(array $actions): void
*/
private function handleAction(Config\Action $action): void
{
if ($this->shouldSkipActions()) {
$this->io->write(
$this->formatActionOutput($action->getAction()) . ': <comment>deactivated</comment>',
true
);
return;
}

$this->io->write(' - <fg=blue>' . $this->formatActionOutput($action->getAction()) . '</> : ', false);

if (!$this->doConditionsApply($action->getConditions())) {
$this->io->write('<comment>skipped</comment>', true);
if ($this->checkSkipAction($action)) {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I did this to reduce the amount of code in the handleAction() method and, hopefully, to simplify the logic, so it's easier to read and understand.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

return;
}

Expand All @@ -303,6 +321,7 @@ private function handleAction(Config\Action $action): void
// The beforeAction() method may indicate that the current and all
// remaining actions should be skipped. If so, return here.
if ($this->shouldSkipActions()) {
$this->io->write('<comment>deactivated</comment>', true);
return;
}

Expand Down Expand Up @@ -360,6 +379,55 @@ public static function getExecMethod(string $type): string
return $valid[$type];
}

/**
* Check if the action should be skipped
*
* @param Config\Action $action
* @return bool
*/
private function checkSkipAction(Config\Action $action): bool
{
if (
$this->shouldSkipActions()
|| $this->cliSkipAction($action)
|| !$this->doConditionsApply($action->getConditions())
) {
$this->io->write('<comment>skipped</comment>', true);

return true;
}

return false;
}

/**
* Check if the CLI `action` options indicates the action should be skipped
*
* @param Config\Action $action
* @return bool
*/
private function cliSkipAction(Config\Action $action): bool
{
/** @var IO\Base $io */
$io = $this->io;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is for the benefit of PHPStan, since getOption() is not on the Console\IO interface.


/** @var string[] $actionsToRun */
$actionsToRun = $io->getOption('action', []);

if (empty($actionsToRun)) {
// No actions specified on CLI; run all actions.
return false;
}

if (in_array($action->getAction(), $actionsToRun)) {
// Action specified on CLI; do not skip.
return false;
}

// Action not specified on CLI; skip.
return true;
}

/**
* Check if conditions apply
*
Expand Down Expand Up @@ -413,7 +481,7 @@ private function getHookPlugins(): array
}

$this->io->write(
['', 'Configuring Hook Plugin: <comment>' . $pluginClass . '</comment>'],
['Configured Hook Plugin: <comment>' . $pluginClass . '</comment>'],
true,
IO::VERBOSE
);
Expand Down Expand Up @@ -449,10 +517,17 @@ private function getHookPlugins(): array
*/
private function executeHookPluginsFor(string $method, ?Config\Action $action = null): void
{
/** @var IO\Base $io */
$io = $this->io;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is for the benefit of PHPStan, since getOption() is not on the Console\IO interface.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We will not need this if we are not using the IO to make decision inside Runner
See previous comment on src/Runner/Hook.php


if ($io->getOption('disable-plugins')) {
return;
}

$plugins = $this->getHookPlugins();

if (count($plugins) === 0) {
$this->io->write(['', 'No plugins to execute for: <comment>' . $method . '</comment>'], true, IO::DEBUG);
$io->write(['', 'No plugins to execute for: <comment>' . $method . '</comment>'], true, IO::DEBUG);

return;
}
Expand All @@ -463,10 +538,10 @@ private function executeHookPluginsFor(string $method, ?Config\Action $action =
$params[] = $action;
}

$this->io->write(['', 'Executing plugins for: <comment>' . $method . '</comment>'], true, IO::DEBUG);
$io->write(['', 'Executing plugins for: <comment>' . $method . '</comment>'], true, IO::DEBUG);

foreach ($plugins as $plugin) {
$this->io->write('<info>- Running ' . get_class($plugin) . '::' . $method . '</info>', true, IO::DEBUG);
$io->write(' <info>- Running ' . get_class($plugin) . '::' . $method . '</info>', true, IO::DEBUG);
$plugin->{$method}(...$params);
}
}
Expand Down
2 changes: 2 additions & 0 deletions tests/files/config/valid-with-all-disabled-hooks.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
{
}