[ 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
ProjektGopher · Sep 6, 2024 · 9d012d7 · 9d012d7
1 parent 98fe07a
commit 9d012d7
Show file tree

Hide file tree

Showing 6 changed files with 173 additions and 9 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -0,0 +1,2 @@
+*
+!.gitignore