From 4da7bbbb5926b0c4b045965aa9853bfd0f5a6bd8 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Maria=20Jos=C3=A9=20Solano?= <majosolano99@gmail.com>
Date: Thu, 13 Apr 2023 05:34:23 -0700
Subject: [PATCH] Add git hooks for formatting and running `clippy` (#8820)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

# Description

As another life improvement (and to avoid those `run cargo fmt` commits
😉), this PR adds a command to the toolkit for formatting and running
`clippy` when committing and pushing.

Thanks to @amtoine for the idea!

---------

Co-authored-by: Darren Schroeder <343840+fdncred@users.noreply.github.com>
---
 .githooks/pre-commit |  5 +++++
 .githooks/pre-push   |  6 ++++++
 CONTRIBUTING.md      | 28 ++++++++++++++++++++++++
 toolkit.nu           | 51 ++++++++++++++++++++++++++++++++++++--------
 4 files changed, 81 insertions(+), 9 deletions(-)
 create mode 100755 .githooks/pre-commit
 create mode 100755 .githooks/pre-push

diff --git a/.githooks/pre-commit b/.githooks/pre-commit
new file mode 100755
index 0000000000..f83086c703
--- /dev/null
+++ b/.githooks/pre-commit
@@ -0,0 +1,5 @@
+#!/usr/bin/env nu
+
+use ../toolkit.nu fmt
+
+fmt --check --verbose
\ No newline at end of file
diff --git a/.githooks/pre-push b/.githooks/pre-push
new file mode 100755
index 0000000000..0ba83eae11
--- /dev/null
+++ b/.githooks/pre-push
@@ -0,0 +1,6 @@
+#!/usr/bin/env nu
+
+use ../toolkit.nu [fmt, clippy]
+
+fmt --check --verbose
+clippy --verbose
\ No newline at end of file
diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md
index 4e9cb6c485..e2d9cd0097 100644
--- a/CONTRIBUTING.md
+++ b/CONTRIBUTING.md
@@ -61,12 +61,22 @@ The most comprehensive test suite we have is the `nu-test-support` crate. For te
   ```shell
   cargo clippy --workspace -- -D warnings -D clippy::unwrap_used -A clippy::needless_collect
   ```
+  or via the `toolkit.nu` command:
+  ```shell
+  use toolkit.nu clippy
+  clippy
+  ```
 
 - Run all tests:
 
   ```shell
   cargo test --workspace
   ```
+  or via the `toolkit.nu` command:
+  ```shell
+  use toolkit.nu test
+  test
+  ```
 
 - Run all tests for a specific command
 
@@ -79,12 +89,30 @@ The most comprehensive test suite we have is the `nu-test-support` crate. For te
   ```shell
   cargo fmt --all -- --check
   ```
+  or via the `toolkit.nu` command:
+  ```shell
+  use toolkit.nu fmt
+  fmt --check
+  ```
 
 - Format the code in the project
 
   ```shell
   cargo fmt --all
   ```
+  or via the `toolkit.nu` command:
+  ```shell
+  use toolkit.nu fmt
+  fmt
+  ```
+
+- Set up `git` hooks to check formatting and run `clippy` before committing and pushing:
+
+  ```shell
+  use toolkit.nu setup-git-hooks
+  setup-git-hooks
+  ```
+  _Unfortunately, this hook isn't available on Windows._
 
 ### Debugging Tips
 
diff --git a/toolkit.nu b/toolkit.nu
index aa2ebc1860..91f688bbf8 100644
--- a/toolkit.nu
+++ b/toolkit.nu
@@ -9,9 +9,18 @@
 # check standard code formatting and apply the changes
 export def fmt [
     --check: bool  # do not apply the format changes, only check the syntax
+    --verbose: bool # print extra information about the command's progress
 ] {
-    if ($check) {
-        cargo fmt --all -- --check
+    if $verbose {
+        print $"running ('toolkit fmt' | pretty-print-command)"
+    }
+
+    if $check {
+        try {
+            cargo fmt --all -- --check
+        } catch {
+            error make -u { msg: $"\nplease run ('toolkit fmt' | pretty-print-command) to fix formatting!" }
+        }
     } else {
         cargo fmt --all
     }
@@ -20,8 +29,18 @@ export def fmt [
 # check that you're using the standard code style
 #
 # > it is important to make `clippy` happy :relieved:
-export def clippy [] {
-    cargo clippy --workspace -- -D warnings -D clippy::unwrap_used -A clippy::needless_collect
+export def clippy [
+    --verbose: bool # print extra information about the command's progress
+] {
+    if $verbose {
+        print $"running ('toolkit clippy' | pretty-print-command)"
+    }
+
+    try {
+        cargo clippy --workspace -- -D warnings -D clippy::unwrap_used -A clippy::needless_collect
+    } catch {
+        error make -u { msg: $"\nplease fix the above ('clippy' | pretty-print-command) errors before continuing!" }
+    }
 }
 
 # check that all the tests pass
@@ -180,17 +199,14 @@ def report [
 export def "check pr" [
     --fast: bool  # use the "nextext" `cargo` subcommand to speed up the tests (see [`cargo-nextest`](https://nexte.st/) and [`nextest-rs/nextest`](https://github.com/nextest-rs/nextest))
 ] {
-    print $"running ('toolkit fmt' | pretty-print-command)"
     try {
-        fmt --check
+        fmt --check --verbose
     } catch {
-        print $"\nplease run (ansi default_dimmed)(ansi default_italic)toolkit fmt(ansi reset) to fix the formatting"
         return (report --fail-fmt)
     }
 
-    print $"running ('toolkit clippy' | pretty-print-command)"
     try {
-        clippy
+        clippy --verbose
     } catch {
         return (report --fail-clippy)
     }
@@ -211,3 +227,20 @@ export def "check pr" [
 
     report --no-fail
 }
+
+# set up git hooks to run:
+# - `toolkit fmt --check --verbose` on `git commit`
+# - `toolkit fmt --check --verbose` and `toolkit clippy --verbose` on `git push`
+export def set-git-hooks [] {
+    if $nu.os-info.name == windows {
+        return (print "This git hook isn't available on Windows. Sorry!")
+    }
+
+    print "This command will change your local git configuration and hence modify your development workflow. Are you sure you want to continue? [y]"
+    if (input) == "y" {
+        print $"running ('toolkit set-git-hooks' | pretty-print-command)"
+        git config --local core.hooksPath .githooks
+    } else {
+        print $"aborting ('toolkit set-git-hooks' | pretty-print-command)"
+    }
+}