From 6af9fe5e10d327e5fb6624d033eb62ce6f7fc9d4 Mon Sep 17 00:00:00 2001
From: Antoine Stevan <44101798+amtoine@users.noreply.github.com>
Date: Sun, 18 Jun 2023 16:16:05 +0200
Subject: [PATCH] toolkit: use `--features` instead of `--dataframe` and
 refactor a bit (#9425)

related to
- https://github.com/nushell/nushell/pull/9357

# Description
with the new `extra` feature introduce in the `toolkit.nu` module in
https://github.com/nushell/nushell/pull/9357, the `--dataframe` option
does not make sense anymore...

this PR changes that and replaces it with `--features: list<string>`.
this has the benefit of simplifying the commands because we can just
pass `--features ($features | str join ",")` to the `cargo` commands
regardless of whether the `$features` list is empty of not :ok_hand:

i've also refactored a bit the module:
- break the long `error make -u` lines
- break the long `cargo clippy` command

# User-Facing Changes
devs can now choose which feature to test with the `toolkit` commands.

# Tests + Formatting
- :green_circle: `toolkit fmt`
- :green_circle: `toolkit clippy`
- :black_circle: `toolkit test`
- :black_circle: `toolkit test stdlib`

# After Submitting
```
$nothing
```
---
 toolkit.nu | 70 +++++++++++++++++++++++++++++++++---------------------
 1 file changed, 43 insertions(+), 27 deletions(-)

diff --git a/toolkit.nu b/toolkit.nu
index fc60c0b08..b4577662f 100644
--- a/toolkit.nu
+++ b/toolkit.nu
@@ -19,7 +19,9 @@ export def fmt [
         try {
             cargo fmt --all -- --check
         } catch {
-            error make -u { msg: $"\nplease run ('toolkit fmt' | pretty-print-command) to fix formatting!" }
+            error make --unspanned {
+                msg: $"\nplease run ('toolkit fmt' | pretty-print-command) to fix formatting!"
+            }
         }
     } else {
         cargo fmt --all
@@ -31,36 +33,56 @@ export def fmt [
 # > it is important to make `clippy` happy :relieved:
 export def clippy [
     --verbose: bool # print extra information about the command's progress
-    --dataframe: bool # use the dataframe feature
+    --features: list<string> # the list of features to run *Clippy* on
+    --workspace: bool # run the *Clippy* command on the whole workspace (overrides `--features`)
 ] {
     if $verbose {
         print $"running ('toolkit clippy' | pretty-print-command)"
     }
 
     try {
-        if $dataframe {
-            cargo clippy --workspace --features=dataframe,extra -- -D warnings -D clippy::unwrap_used -A clippy::needless_collect -A clippy::result_large_err
-        } else {
-            cargo clippy --workspace -- -D warnings -D clippy::unwrap_used -A clippy::needless_collect -A clippy::result_large_err
-        }
+        if $workspace {(
+            cargo clippy
+                --workspace
+            --
+                -D warnings
+                -D clippy::unwrap_used
+                -A clippy::needless_collect
+                -A clippy::result_large_err
+        )} else {(
+            cargo clippy
+                --features ($features | str join ",")
+            --
+                -D warnings
+                -D clippy::unwrap_used
+                -A clippy::needless_collect
+                -A clippy::result_large_err
+        )}
     } catch {
-        error make -u { msg: $"\nplease fix the above ('clippy' | pretty-print-command) errors before continuing!" }
+        error make --unspanned {
+            msg: $"\nplease fix the above ('clippy' | pretty-print-command) errors before continuing!"
+        }
     }
 }
 
 # check that all the tests pass
 export def test [
     --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))
-    --dataframe: bool # use the dataframe feature
+    --features: list<string> # the list of features to run the tests on
+    --workspace: bool # run the *Clippy* command on the whole workspace (overrides `--features`)
 ] {
-    if ($fast and $dataframe) {
-        cargo nextest run --all --features=dataframe,extra
-    } else if ($fast) {
-        cargo nextest run --all
-    } else if ($dataframe) {
-        cargo test --workspace --features=dataframe,extra
+    if $fast {
+        if $workspace {
+            cargo nextest run --all
+        } else {
+            cargo nextest run --features ($features | str join ",")
+        }
     } else {
-        cargo test --workspace
+        if $workspace {
+            cargo test --workspace
+        } else {
+            cargo test --features ($features | str join ",")
+        }
     }
 }
 
@@ -208,7 +230,7 @@ def report [
 # now the whole `toolkit check pr` passes! :tada:
 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))
-    --dataframe: bool # use the dataframe feature
+    --features: list<string> # the list of features to check the current PR on
 ] {
     let-env NU_TEST_LOCALE_OVERRIDE = 'en_US.utf8';
     try {
@@ -218,23 +240,17 @@ export def "check pr" [
     }
 
     try {
-        if $dataframe {
-            clippy --dataframe --verbose
-        } else {
-            clippy --verbose
-        }
+        clippy --features $features --verbose
     } catch {
         return (report --fail-clippy)
     }
 
     print $"running ('toolkit test' | pretty-print-command)"
     try {
-        if $fast and $dataframe {
-            test --fast --dataframe
-        } else if $fast {
-            test --fast
+        if $fast {
+            test --features $features --fast
         } else {
-            test
+            test --features $features
         }
     } catch {
         return (report --fail-test)