Allow running only actions specified on the command line #137

ramsey · 2021-05-31T00:59:29Z

This PR adds 3 new hook command options:

--list-actions
--action
--disable-plugins

--list-actions

This option allows you to list all the configured actions for a given hook:

$ ./bin/captainhook hook:pre-commit --list-actions
Listing pre-commit actions:
 - \CaptainHook\App\Hook\PHP\Action\Linting
 - tools/phpcs --standard=psr12 src
 - \CaptainHook\App\Hook\PHP\Action\TestCoverage
 - \CaptainHook\App\Hook\Composer\Action\CheckLockFile

--action

This option allows you to pass one or more actions to execute for a given hook. All other actions configured for the hook are skipped.

$ ./bin/captainhook hook:pre-commit --action="tools/phpcs --standard=psr12 src" --action="\CaptainHook\App\Hook\Composer\Action\CheckLockFile"
pre-commit: 
 - \CaptainHook\App\Hook\PHP\Action\Linting                          : skipped
 - tools/phpcs --standard=psr12 src                                  : done
 - \CaptainHook\App\Hook\PHP\Action\TestCoverage                     : skipped
 - \CaptainHook\App\Hook\Composer\Action\CheckLockFile               : done

--disable-plugins

This option is useful for combining with the --action option to disable any plugins while executing only the specified actions.

$ ./bin/captainhook hook:pre-commit --disable-plugins --action="tools/phpcs --standard=psr12 src" --action="\CaptainHook\App\Hook\Composer\Action\CheckLockFile"
pre-commit: 
Running with plugins disabled
 - \CaptainHook\App\Hook\PHP\Action\Linting                          : skipped
 - tools/phpcs --standard=psr12 src                                  : done
 - \CaptainHook\App\Hook\PHP\Action\TestCoverage                     : skipped
 - \CaptainHook\App\Hook\Composer\Action\CheckLockFile               : done

Rationale

As discussed in #122, I'm working on a DisallowActionChanges plugin that will extend the PreserveWorkingTree plugin to check whether any actions make changes and, if so, fail. As part of the failure message, I want to list the actions that made changes and instruct the user with a message like this:

Use the following to run this action and inspect the changes it makes:

  captainhook hook:pre-commit --disable-plugins --action="tools/phpcs --standard=psr12 src"

The --list-actions option is a helper option that I thought to include so a user can quickly list all actions configured for a hook. The names of the actions it lists are the same names that you would pass to the --action option.

IO::getOptions() and IO::getOption()

I've added these two methods to allow retrieving options from IO, just like you can retrieve arguments. However, I did not add these to the IO interface, since that would likely require a major version bump. For now, they're only on Console\IO\DefaultIO and Console\IO\Base. They should also be added to the Console\IO interface in version 6.0.0.

ramsey · 2021-05-31T01:25:50Z

src/Console/Command/Hook.php

+        // Otherwise, use the verbosity setting from the configuration.
+        if (!$input->hasOption('verbose') || !$input->getOption('verbose')) {
+            $output->setVerbosity(IOUtil::mapConfigVerbosity($config->getVerbosity()));
+        }


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.

ramsey · 2021-05-31T01:27:34Z

src/Hook/File/Action/Check.php

@@ -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


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

ramsey · 2021-05-31T01:28:23Z

src/Runner/Hook.php

+    public function isEnabled(): bool
+    {
+        return $this->isAnyConfigEnabled($this->getHookConfigsToHandle());
+    }


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

ramsey · 2021-05-31T01:29:03Z

src/Runner/Hook.php

+    public function getActions(): array
+    {
+        return $this->getActionsToExecute($this->getHookConfigsToHandle());
+    }


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.

ramsey · 2021-05-31T01:30:10Z

src/Runner/Hook.php

        $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)) {


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.

ramsey · 2021-05-31T01:31:19Z

src/Runner/Hook.php

@@ -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;


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

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.

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

ramsey · 2021-05-31T01:31:35Z

src/Runner/Hook.php

+    private function cliSkipAction(Config\Action $action): bool
+    {
+        /** @var IO\Base $io */
+        $io = $this->io;


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

ramsey · 2021-05-31T01:31:42Z

src/Runner/Hook.php

@@ -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;


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

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

simivar · 2021-05-31T08:11:45Z

src/Console/Command/Hook.php

+        if ($input->getOption('list-actions') === true) {
+            foreach ($this->getDefinition()->getArguments() as $arg) {
+                if ($arg->isRequired() && $input->getArgument($arg->getName()) === null) {
+                    $input->setArgument($arg->getName(), '');
+                }
+            }
+        }


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?

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

@ramsey have you seen this comment as well?

This sounds good to me.

sebastianfeldmann

Looks pretty nice, the only thing to discuss would be the ->getOption stuff.

sebastianfeldmann · 2021-05-31T08:17:57Z

src/Runner/Hook.php

@@ -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;


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.

sebastianfeldmann · 2021-05-31T08:30:51Z

src/Runner/Hook.php

        $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)) {


ramsey added 8 commits May 29, 2021 18:10

Add public isEnabled() and getActions() methods

24d6c0b

Add --list-actions option to hook commands

6ea2030

Add getOptions() and getOption() to the IO classes

e65c88c

Allow disabling plugins from the command line

163f380

Allow setting verbosity from CLI to override configuration

e29570e

Run only actions passed on the command line

e413409

Ensure action output in tests is consistent with expected output

3b64e0d

Hint to PHPStan to use IO\Base for getOption()

6acf735

ramsey commented May 31, 2021

View reviewed changes

sebastianfeldmann self-assigned this May 31, 2021

simivar reviewed May 31, 2021

View reviewed changes

sebastianfeldmann reviewed May 31, 2021

View reviewed changes

ramsey added 2 commits December 21, 2021 15:34

commit wip

c23b6ed

Merge branch 'main' into feature/run-specific-actions

226072a

ramsey marked this pull request as draft December 21, 2021 21:50

sebastianfeldmann force-pushed the main branch from 149f4f6 to 5f589b4 Compare October 2, 2023 08:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow running only actions specified on the command line #137

Allow running only actions specified on the command line #137

ramsey commented May 31, 2021 •

edited

ramsey May 31, 2021

ramsey May 31, 2021

ramsey May 31, 2021

ramsey May 31, 2021

ramsey May 31, 2021

sebastianfeldmann May 31, 2021

ramsey May 31, 2021

sebastianfeldmann May 31, 2021 •

edited

ramsey May 31, 2021

ramsey May 31, 2021

ramsey May 31, 2021

sebastianfeldmann May 31, 2021

simivar May 31, 2021

sebastianfeldmann May 31, 2021 •

edited

sebastianfeldmann May 31, 2021

ramsey May 31, 2021

sebastianfeldmann left a comment

sebastianfeldmann May 31, 2021 •

edited

sebastianfeldmann May 31, 2021

Allow running only actions specified on the command line #137

Are you sure you want to change the base?

Allow running only actions specified on the command line #137

Conversation

ramsey commented May 31, 2021 • edited

--list-actions

--action

--disable-plugins

Rationale

IO::getOptions() and IO::getOption()

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sebastianfeldmann May 31, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sebastianfeldmann May 31, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sebastianfeldmann left a comment

Choose a reason for hiding this comment

sebastianfeldmann May 31, 2021 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ramsey commented May 31, 2021 •

edited

sebastianfeldmann May 31, 2021 •

edited

sebastianfeldmann May 31, 2021 •

edited

sebastianfeldmann May 31, 2021 •

edited