change the output of which to be more explicit (#9646)

amtoine · web-flow · commit a1f989caf981 · 2023-07-20T19:10:53.000-05:00
related to - #9637 (comment) # Description this PR changes the output of `which` from `table<arg: string, path: string, built-in: bool> (stream)` to `table<command: string, path: string, type: string> (stream)`. - `command`: same as `arg` but more explicit name - `path`: same as before, `null` when built-in - `type`: instead of `buil-in: bool` says if it's a `built-in` a `custom` command, an `alias` or an `external` # User-Facing Changes the output of `which` has changed ## some examples ```nushell > which open ╭───┬─────────┬──────┬──────────╮ │ # │ command │ path │ type │ ├───┼─────────┼──────┼──────────┤ │ 0 │ open │ │ built-in │ ╰───┴─────────┴──────┴──────────╯ ``` ```nushell > alias foo = print "foo" > which foo ╭───┬─────────┬──────┬───────╮ │ # │ command │ path │ type │ ├───┼─────────┼──────┼───────┤ │ 0 │ foo │ │ alias │ ╰───┴─────────┴──────┴───────╯ ``` ```nushell > def bar [] {} > which bar ╭───┬─────────┬──────┬────────╮ │ # │ command │ path │ type │ ├───┼─────────┼──────┼────────┤ │ 0 │ bar │ │ custom │ ╰───┴─────────┴──────┴────────╯ ``` ```nushell > which git ╭───┬─────────┬──────────────┬──────────╮ │ # │ command │ path │ type │ ├───┼─────────┼──────────────┼──────────┤ │ 0 │ git │ /usr/bin/git │ external │ ╰───┴─────────┴──────────────┴──────────╯ ``` ```nushell > which open git foo bar ╭───┬─────────┬──────────────┬──────────╮ │ # │ command │ path │ type │ ├───┼─────────┼──────────────┼──────────┤ │ 0 │ open │ │ built-in │ │ 1 │ git │ /usr/bin/git │ external │ │ 2 │ foo │ │ alias │ │ 3 │ bar │ │ custom │ ╰───┴─────────┴──────────────┴──────────╯ ``` # Tests + Formatting - 🟢 `toolkit fmt` - 🟢 `toolkit clippy` - ⚫ `toolkit test` - ⚫ `toolkit test stdlib` # After Submitting mention that in the release note
diff --git a/crates/nu-command/src/system/which_.rs b/crates/nu-command/src/system/which_.rs
@@ -57,35 +57,40 @@ impl Command for Which {
 }
 
 // Shortcut for creating an entry to the output table
-fn entry(arg: impl Into<String>, path: impl Into<String>, builtin: bool, span: Span) -> Value {
+fn entry(
+    arg: impl Into<String>,
+    path: impl Into<String>,
+    cmd_type: impl Into<String>,
+    span: Span,
+) -> Value {
     let mut cols = vec![];
     let mut vals = vec![];
 
-    cols.push("arg".to_string());
+    cols.push("command".to_string());
     vals.push(Value::string(arg.into(), span));
 
     cols.push("path".to_string());
     vals.push(Value::string(path.into(), span));
 
-    cols.push("built-in".to_string());
-    vals.push(Value::Bool { val: builtin, span });
+    cols.push("type".to_string());
+    vals.push(Value::string(cmd_type.into(), span));
 
     Value::Record { cols, vals, span }
 }
 
 fn get_entry_in_commands(engine_state: &EngineState, name: &str, span: Span) -> Option<Value> {
     if let Some(decl_id) = engine_state.find_decl(name.as_bytes(), &[]) {
-        let (msg, is_builtin) = if engine_state.get_decl(decl_id).is_custom_command() {
-            ("Nushell custom command", false)
+        let cmd_type = if engine_state.get_decl(decl_id).is_custom_command() {
+            "custom"
         } else if engine_state.get_decl(decl_id).is_alias() {
-            ("Nushell alias", false)
+            "alias"
         } else {
-            ("Nushell built-in command", true)
+            "built-in"
         };
 
         trace!("Found command: {}", name);
 
-        Some(entry(name, msg, is_builtin, span))
+        Some(entry(name, "", cmd_type, span))
     } else {
         None
     }
@@ -118,7 +123,7 @@ fn get_first_entry_in_path(
     paths: impl AsRef<OsStr>,
 ) -> Option<Value> {
     which::which_in(item, Some(paths), cwd)
-        .map(|path| entry(item, path.to_string_lossy().to_string(), false, span))
+        .map(|path| entry(item, path.to_string_lossy().to_string(), "external", span))
         .ok()
 }
 
@@ -141,7 +146,7 @@ fn get_all_entries_in_path(
 ) -> Vec<Value> {
     which::which_in_all(&item, Some(paths), cwd)
         .map(|iter| {
-            iter.map(|path| entry(item, path.to_string_lossy().to_string(), false, span))
+            iter.map(|path| entry(item, path.to_string_lossy().to_string(), "external", span))
                 .collect()
         })
         .unwrap_or_default()
diff --git a/crates/nu-command/tests/commands/which.rs b/crates/nu-command/tests/commands/which.rs
@@ -2,9 +2,9 @@ use nu_test_support::nu;
 
 #[test]
 fn which_ls() {
-    let actual = nu!("which ls | get path.0 | str trim");
+    let actual = nu!("which ls | get type.0");
 
-    assert_eq!(actual.out, "Nushell built-in command");
+    assert_eq!(actual.out, "built-in");
 }
 
 #[ignore = "TODO: Can't have alias recursion"]
@@ -19,17 +19,14 @@ fn which_alias_ls() {
 fn which_custom_alias() {
     let actual = nu!(r#"alias foo = print "foo!"; which foo | to nuon"#);
 
-    assert_eq!(
-        actual.out,
-        "[[arg, path, built-in]; [foo, \"Nushell alias\", false]]"
-    );
+    assert_eq!(actual.out, r#"[[command, path, type]; [foo, "", alias]]"#);
 }
 
 #[test]
 fn which_def_ls() {
-    let actual = nu!("def ls [] {echo def}; which ls | get path.0 | str trim");
+    let actual = nu!("def ls [] {echo def}; which ls | get type.0");
 
-    assert_eq!(actual.out, "Nushell custom command");
+    assert_eq!(actual.out, "custom");
 }
 
 #[ignore = "TODO: Can't have alias with the same name as command"]
@@ -58,24 +55,24 @@ fn multiple_reports_for_alias_def_custom() {
 #[ignore]
 #[test]
 fn correctly_report_of_shadowed_alias() {
-    let actual = nu!(r#"alias xaz = echo alias1
+    let actual = nu!("alias xaz = echo alias1
         def helper [] {
             alias xaz = echo alias2
             which -a xaz
         }
-        helper | get path | str contains alias2"#);
+        helper | get path | str contains alias2");
 
     assert_eq!(actual.out, "true");
 }
 
 #[test]
 fn one_report_of_multiple_defs() {
-    let actual = nu!(r#"def xaz [] { echo def1 }
+    let actual = nu!("def xaz [] { echo def1 }
         def helper [] {
             def xaz [] { echo def2 }
             which -a xaz
         }
-        helper | length"#);
+        helper | length");
 
     let length: i32 = actual.out.parse().unwrap();
     assert_eq!(length, 1);
@@ -91,19 +88,19 @@ fn def_only_seen_once() {
 
 #[test]
 fn do_not_show_hidden_aliases() {
-    let actual = nu!(r#"alias foo = echo foo
+    let actual = nu!("alias foo = echo foo
         hide foo
-        which foo | length"#);
+        which foo | length");
 
     let length: i32 = actual.out.parse().unwrap();
     assert_eq!(length, 0);
 }
 
 #[test]
 fn do_not_show_hidden_commands() {
-    let actual = nu!(r#"def foo [] { echo foo }
+    let actual = nu!("def foo [] { echo foo }
         hide foo
-        which foo | length"#);
+        which foo | length");
 
     let length: i32 = actual.out.parse().unwrap();
     assert_eq!(length, 0);
