From 8c1ab7e0a3b803279e09c93c70a02e3636e0146b Mon Sep 17 00:00:00 2001
From: Douglas <32344964+NotTheDr01ds@users.noreply.github.com>
Date: Thu, 14 Nov 2024 23:27:26 -0500
Subject: [PATCH] Add proper config defaults for hooks (#14341)

# Release Notes Excerpt

* Hooks now default to an empty value of the proper type (e.g., `[]` or
`{}`) when not otherwise specified

# Description

```nushell
# Start with no config
nu -n
# Populate with defaults
$env.config = {}
$env.config.hooks
```

* Before: All hooks other than `display_output` were set to `null`.
Attempting to append a hook using `++=` would fail unless it had already
been assigned.
* After:
* `pre_prompt`, `pre_execution`, and `command_not_found` are set to
empty lists. This allows the user to simply append new hooks using
`++=`.
* `env_change` is set to an empty record. This allows the user to add
new hooks using `merge`, although a "helper" command would still be
useful (TODO: stdlib).

Also fixed a typo in an error message.

# User-Facing Changes

There shouldn't be any breaking changes since (before) there were no
guarantees of the hook's value/type. Previously, users would have to
check for `null` and `default` to an empty list before appending. Any
user-strategies for dealing with the problem should continue to work
after this change.

# Tests + Formatting

- :green_circle: `toolkit fmt`
- :green_circle: `toolkit clippy`
- :green_circle: `toolkit test`
- :green_circle: `toolkit test stdlib`

Note that, for reasons I cannot ascertain, this PR appears to have
*fixed* the `command_not_found_error_recognizes_non_executable_file`
test that was previously broken by #12953. That PR essentially rewrote
the test to match the new behavior, but it no longer tested what it was
intended to test.

Now, the test is working again as designed (and as it works in the
REPL).

# After Submitting

This will be covered in the Configuration update for #14249. This PR
will simplify several examples in the doc.
---
 crates/nu-command/src/system/run_external.rs | 2 +-
 crates/nu-protocol/src/config/hooks.rs       | 9 +++++----
 tests/shell/pipeline/commands/external.rs    | 6 +++---
 3 files changed, 9 insertions(+), 8 deletions(-)

diff --git a/crates/nu-command/src/system/run_external.rs b/crates/nu-command/src/system/run_external.rs
index 013d3d8374..32543716f1 100644
--- a/crates/nu-command/src/system/run_external.rs
+++ b/crates/nu-command/src/system/run_external.rs
@@ -484,7 +484,7 @@ pub fn command_not_found(
     if Path::new(name).is_file() {
         return ShellError::ExternalCommand {
                         label: format!("Command `{name}` not found"),
-                        help: format!("`{name}` refers to a file that is not executable. Did you forget to to set execute permissions?"),
+                        help: format!("`{name}` refers to a file that is not executable. Did you forget to set execute permissions?"),
                         span,
                     };
     }
diff --git a/crates/nu-protocol/src/config/hooks.rs b/crates/nu-protocol/src/config/hooks.rs
index 9b9860a87d..374b3818d6 100644
--- a/crates/nu-protocol/src/config/hooks.rs
+++ b/crates/nu-protocol/src/config/hooks.rs
@@ -1,5 +1,6 @@
 use super::prelude::*;
 use crate as nu_protocol;
+use crate::Record;
 
 /// Definition of a parsed hook from the config object
 #[derive(Clone, Debug, IntoValue, PartialEq, Serialize, Deserialize)]
@@ -14,14 +15,14 @@ pub struct Hooks {
 impl Hooks {
     pub fn new() -> Self {
         Self {
-            pre_prompt: None,
-            pre_execution: None,
-            env_change: None,
+            pre_prompt: Some(Value::list(vec![], Span::unknown())),
+            pre_execution: Some(Value::list(vec![], Span::unknown())),
+            env_change: Some(Value::record(Record::default(), Span::unknown())),
             display_output: Some(Value::string(
                 "if (term size).columns >= 100 { table -e } else { table }",
                 Span::unknown(),
             )),
-            command_not_found: None,
+            command_not_found: Some(Value::list(vec![], Span::unknown())),
         }
     }
 }
diff --git a/tests/shell/pipeline/commands/external.rs b/tests/shell/pipeline/commands/external.rs
index 207a94766b..172ac8cbfc 100644
--- a/tests/shell/pipeline/commands/external.rs
+++ b/tests/shell/pipeline/commands/external.rs
@@ -131,9 +131,9 @@ fn command_not_found_error_suggests_typo_fix() {
 #[test]
 fn command_not_found_error_recognizes_non_executable_file() {
     let actual = nu!("./Cargo.toml");
-    assert!(actual
-        .err
-        .contains("is neither a Nushell built-in or a known external command"));
+    assert!(actual.err.contains(
+        "refers to a file that is not executable. Did you forget to set execute permissions?"
+    ));
 }
 
 #[test]