From 9d012d7f1eb0472090d41825ae09b09f0f834e0a Mon Sep 17 00:00:00 2001 From: Len Woodward Date: Fri, 6 Sep 2024 16:51:09 -0700 Subject: [PATCH] [ Feat ] Pass optional arguments to hooks (#77) * Adds optional argument param to run command * update snippet to pass optional argument * add commit-msg hook to project for local testing * Update callouts in README * update README * remove commit-msg hook used for local testing * fill in tests, fix skipped test * add test temp path helper for windows garbage * add temporary folder in project that persists * fix tests for windows... maybe? * WINDOWS! FML * make test more strict * trim extra dbl_quotes from windows cmd line * accomodate different cli behavior for passing string arguments on windows * try again * blah * fuck you * reduce expectations and try again --- README.md | 36 +++++++++-- app/Commands/Run.php | 8 ++- app/Hook.php | 3 +- app/Platform.php | 9 +++ tests/Feature/RunTest.php | 124 +++++++++++++++++++++++++++++++++++++- tests/tmp/.gitignore | 2 + 6 files changed, 173 insertions(+), 9 deletions(-) create mode 100644 tests/tmp/.gitignore diff --git a/README.md b/README.md index 36948af..d8fcd7b 100644 --- a/README.md +++ b/README.md @@ -58,10 +58,35 @@ The `install` command will create a `whisky.json` file in your project root: For a complete list of supported git hooks, see the [Git Documentation](https://git-scm.com/docs/githooks#_hooks). -> **Warning** all hooks are **evaluated as-is** in the terminal. Keep this in mind when committing anything involving changes to your `whisky.json`. +> [!CAUTION] +> all hooks are **evaluated as-is** in the terminal. Keep this in mind when committing anything involving changes to your `whisky.json`. Adding or removing any **hooks** (_not_ individual commands) to your `whisky.json` file should be followed by `./vendor/bin/whisky update` to ensure that these changes are reflected in your `.git/hooks` directory. +### Working With Hook Arguments +Some hooks in git are passed arguments. + +The `commit-msg` hook is a perfect example. It's passed the path to git's temporary file containing the commit message, +which can then be used by scripts like npm's [commitlint](https://commitlint.js.org/) to allow or prevent commit +messages that might not conform to your project's standards. + +To use this argument that git passes, you can _optionally_ include `$1` in your array of commands. +(You should wrap it in escaped double quotes to prevent odd behavior due to whitespace) + +```js +// whisky.json +// ... + "commit-msg": [ + "npx --no -- commitlint --edit \"$1\"" + ] +// ... +``` + +> [!IMPORTANT] +> For `commitlint` specifically, you'll need to follow the instructions in their +> [documentation](https://commitlint.js.org/guides/getting-started.html), +> as it will require extra packages and setup to run in your project. + ### Automating Hook Updates While we suggest leaving Whisky as an 'opt-in' tool, by adding a couple of Composer scripts we can _ensure_ consistent git hooks for all project contributors. This will **force** everyone on the project to use Whisky: @@ -93,7 +118,8 @@ In this case, running the following command will have the exact same effect. ./vendor/bin/whisky skip-once ``` -> **Note** by adding `alias whisky=./vendor/bin/whisky` to your `bash.rc` file, you can shorten the length of this command. +> [!TIP] +> by adding `alias whisky=./vendor/bin/whisky` to your `bash.rc` file, you can shorten the length of this command. ### Disabling Hooks Adding a hook's name to the `disabled` array in your `whisky.json` will disable the hook from running. @@ -115,7 +141,8 @@ you to run scripts written in _any_ language. // ... ``` -> **Note** When doing this, make sure any scripts referenced are **executable**: +> [!NOTE] +> When doing this, make sure any scripts referenced are **executable**: ```bash chmod +x ./scripts/* ``` @@ -142,7 +169,8 @@ whisky uninstall -n ## Contributing -> **Note** Don't build the binary when contributing. The binary will be built when a release is tagged. +> [!NOTE] +> Don't build the binary when contributing. The binary will be built when a release is tagged. Please see [CONTRIBUTING](CONTRIBUTING.md) for more details. diff --git a/app/Commands/Run.php b/app/Commands/Run.php index b20358e..dc206a3 100644 --- a/app/Commands/Run.php +++ b/app/Commands/Run.php @@ -12,7 +12,7 @@ class Run extends Command { - protected $signature = 'run {hook}'; + protected $signature = 'run {hook} {argument?}'; protected $description = 'Run the scripts for a given hook'; @@ -41,6 +41,12 @@ public function handle(): int Hook::make($this->argument('hook')) ->getScripts() ->each(function (string $script) use (&$exitCode): void { + $script = str_replace( + search: '$1', + replace: trim($this->argument('argument'), '"'), + subject: $script, + ); + $isTtySupported = SymfonyProcess::isTtySupported(); $result = $isTtySupported diff --git a/app/Hook.php b/app/Hook.php index 3aed82b..0dec3bd 100644 --- a/app/Hook.php +++ b/app/Hook.php @@ -140,8 +140,9 @@ public function getScripts(): Collection public function getSnippets(): Collection { return collect([ - "{$this->bin} run {$this->hook}", + "{$this->bin} run {$this->hook} \"$1\"", // Legacy Snippets. + "{$this->bin} run {$this->hook}", "eval \"$({$this->bin} get-run-cmd {$this->hook})\"", "eval \"$(./vendor/bin/whisky get-run-cmd {$this->hook})\"", ]); diff --git a/app/Platform.php b/app/Platform.php index e0972dc..2117221 100644 --- a/app/Platform.php +++ b/app/Platform.php @@ -15,6 +15,15 @@ public static function cwd(string $path = ''): string return static::normalizePath(getcwd()); } + public static function temp_test_path(string $path = ''): string + { + if ($path) { + return static::cwd("tests/tmp/{$path}"); + } + + return static::cwd('tests/tmp'); + } + public static function normalizePath(string $path): string { if ((new self)->isWindows()) { diff --git a/tests/Feature/RunTest.php b/tests/Feature/RunTest.php index 59844a4..db46d02 100644 --- a/tests/Feature/RunTest.php +++ b/tests/Feature/RunTest.php @@ -3,7 +3,7 @@ use Illuminate\Support\Facades\File; use ProjektGopher\Whisky\Platform; -it('deletes skip-once file if exists as long as whisky.json exists', function () { +it('deletes skip-once file if exists as long as whisky.json exists and does not run the hook', function () { File::shouldReceive('missing') ->once() ->with(Platform::cwd('whisky.json')) @@ -19,7 +19,125 @@ ->with(Platform::cwd('.git/hooks/skip-once')) ->andReturnTrue(); + $tmp_file = Platform::temp_test_path('whisky_test_commit_msg'); + + File::shouldReceive('get') + ->with(Platform::cwd('whisky.json')) + ->andReturn(<<<"JSON" + { + "disabled": [], + "hooks": { + "pre-commit": [ + "echo \"poop\" > {$tmp_file}" + ] + } + } + JSON); + $this->artisan('run pre-commit') - ->doesntExpectOutputToContain('run-hook') ->assertExitCode(0); -})->skip('Needs to be refactored so that the hooks don\'t actually run'); + + expect(file_exists($tmp_file)) + ->toBeFalse(); +}); + +it('accepts an optional argument and the argument is correct', function () { + File::shouldReceive('missing') + ->with(Platform::cwd('whisky.json')) + ->andReturnFalse(); + + File::shouldReceive('exists') + ->with(Platform::cwd('.git/hooks/skip-once')) + ->andReturnFalse(); + + $tmp_file = Platform::temp_test_path('whisky_test_commit_msg'); + + /** + * We need to send the output to the disk + * otherwise the output is sent to the + * test output and we can't check it. + */ + File::shouldReceive('get') + ->with(Platform::cwd('whisky.json')) + ->andReturn(<<<"JSON" + { + "disabled": [], + "hooks": { + "commit-msg": [ + "echo \"$1\" > {$tmp_file}" + ] + } + } + JSON); + + $this->artisan('run commit-msg ".git/COMMIT_EDITMSG"') + ->assertExitCode(0); + + expect(file_get_contents($tmp_file)) + ->toContain('.git/COMMIT_EDITMSG'); + + unlink($tmp_file); +}); + +it('still works if no arguments are passed to run command', function () { + File::shouldReceive('missing') + ->with(Platform::cwd('whisky.json')) + ->andReturnFalse(); + + File::shouldReceive('exists') + ->with(Platform::cwd('.git/hooks/skip-once')) + ->andReturnFalse(); + + $tmp_file = Platform::temp_test_path('whisky_test_pre_commit'); + + File::shouldReceive('get') + ->with(Platform::cwd('whisky.json')) + ->andReturn(<<<"JSON" + { + "disabled": [], + "hooks": { + "pre-commit": [ + "echo \"pre-commit\" > {$tmp_file}" + ] + } + } + JSON); + + $this->artisan('run pre-commit') + ->assertExitCode(0); + + unlink($tmp_file); +}); + +it('handles a missing expected argument gracefully', function () { + File::shouldReceive('missing') + ->with(Platform::cwd('whisky.json')) + ->andReturnFalse(); + + File::shouldReceive('exists') + ->with(Platform::cwd('.git/hooks/skip-once')) + ->andReturnFalse(); + + $tmp_file = Platform::temp_test_path('whisky_test_commit_msg'); + + File::shouldReceive('get') + ->with(Platform::cwd('whisky.json')) + ->andReturn(<<<"JSON" + { + "disabled": [], + "hooks": { + "commit-msg": [ + "echo \"$1\" > {$tmp_file}" + ] + } + } + JSON); + + $this->artisan('run commit-msg') + ->assertExitCode(0); + + expect(file_exists($tmp_file)) + ->toBeTrue(); + + unlink($tmp_file); +}); diff --git a/tests/tmp/.gitignore b/tests/tmp/.gitignore new file mode 100644 index 0000000..d6b7ef3 --- /dev/null +++ b/tests/tmp/.gitignore @@ -0,0 +1,2 @@ +* +!.gitignore