-
-
Notifications
You must be signed in to change notification settings - Fork 86
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
base: main
Are you sure you want to change the base?
Changes from 8 commits
24d6c0b
6ea2030
e65c88c
163f380
e29570e
e413409
3b64e0d
6acf735
c23b6ed
226072a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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; | ||
|
@@ -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(), ''); | ||
} | ||
} | ||
} | ||
} | ||
|
||
/** | ||
|
@@ -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())); | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I did this because I found that IMO, if you pass verbosity on the command line, it should override whatever is configured in |
||
|
||
$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; | ||
|
@@ -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()}</>"); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||
{ | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is for the benefit of PHPStan, since There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll give this a try. I didn't like adding the |
||
|
||
$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; | ||
} | ||
|
||
|
@@ -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); | ||
} | ||
|
@@ -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()); | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||
|
@@ -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()); | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 |
||
|
||
/** | ||
* Return all the actions to execute | ||
* | ||
|
@@ -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)) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I did this to reduce the amount of code in the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 |
||
return; | ||
} | ||
|
||
|
@@ -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; | ||
} | ||
|
||
|
@@ -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; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is for the benefit of PHPStan, since |
||
|
||
/** @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 | ||
* | ||
|
@@ -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 | ||
); | ||
|
@@ -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; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is for the benefit of PHPStan, since There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We will not need this if we are not using the |
||
|
||
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; | ||
} | ||
|
@@ -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); | ||
} | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,2 @@ | ||
{ | ||
} |
There was a problem hiding this comment.
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?There was a problem hiding this comment.
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 theConfig
class. This way we would not have duplicate code inRunner\Hook
andRunner\ListActions
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.