Make Hooks fields non-optional to match the new config defaults (#14345)

# Description
Follow up to #14341. Changes the fields of `Hooks` to `Vec` or `Hashmap`
to match the new config defaults.

# User-Facing Changes
Mostly the same as #14341. `pre_prompt` and `pre_execution` must now be
a list, and `env_change` must be a record.
This commit is contained in:
Ian Manske 2024-11-29 13:11:09 -08:00 committed by GitHub
parent 91bb566ee6
commit 6bc695f251
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
7 changed files with 156 additions and 121 deletions

View File

@ -16,7 +16,7 @@ use crate::{
use crossterm::cursor::SetCursorStyle;
use log::{error, trace, warn};
use miette::{ErrReport, IntoDiagnostic, Result};
use nu_cmd_base::{hook::eval_hook, util::get_editor};
use nu_cmd_base::util::get_editor;
use nu_color_config::StyleComputer;
#[allow(deprecated)]
use nu_engine::{convert_env_values, current_dir_str, env_to_strings};
@ -313,20 +313,26 @@ fn loop_iteration(ctx: LoopContext) -> (bool, Stack, Reedline) {
perf!("reset signals", start_time, use_color);
start_time = std::time::Instant::now();
// Right before we start our prompt and take input from the user,
// fire the "pre_prompt" hook
if let Some(hook) = engine_state.get_config().hooks.pre_prompt.clone() {
if let Err(err) = eval_hook(engine_state, &mut stack, None, vec![], &hook, "pre_prompt") {
report_shell_error(engine_state, &err);
}
// Right before we start our prompt and take input from the user, fire the "pre_prompt" hook
if let Err(err) = hook::eval_hooks(
engine_state,
&mut stack,
vec![],
&engine_state.get_config().hooks.pre_prompt.clone(),
"pre_prompt",
) {
report_shell_error(engine_state, &err);
}
perf!("pre-prompt hook", start_time, use_color);
start_time = std::time::Instant::now();
// Next, check all the environment variables they ask for
// fire the "env_change" hook
let env_change = engine_state.get_config().hooks.env_change.clone();
if let Err(error) = hook::eval_env_change_hook(env_change, engine_state, &mut stack) {
if let Err(error) = hook::eval_env_change_hook(
&engine_state.get_config().hooks.env_change.clone(),
engine_state,
&mut stack,
) {
report_shell_error(engine_state, &error)
}
perf!("env-change hook", start_time, use_color);
@ -511,18 +517,17 @@ fn loop_iteration(ctx: LoopContext) -> (bool, Stack, Reedline) {
// Right before we start running the code the user gave us, fire the `pre_execution`
// hook
if let Some(hook) = config.hooks.pre_execution.clone() {
{
// Set the REPL buffer to the current command for the "pre_execution" hook
let mut repl = engine_state.repl_state.lock().expect("repl state mutex");
repl.buffer = repl_cmd_line_text.to_string();
drop(repl);
if let Err(err) = eval_hook(
if let Err(err) = hook::eval_hooks(
engine_state,
&mut stack,
None,
vec![],
&hook,
&engine_state.get_config().hooks.pre_execution.clone(),
"pre_execution",
) {
report_shell_error(engine_state, &err);

View File

@ -7,49 +7,55 @@ use nu_protocol::{
engine::{Closure, EngineState, Stack, StateWorkingSet},
PipelineData, PositionalArg, ShellError, Span, Type, Value, VarId,
};
use std::sync::Arc;
use std::{collections::HashMap, sync::Arc};
pub fn eval_env_change_hook(
env_change_hook: Option<Value>,
env_change_hook: &HashMap<String, Vec<Value>>,
engine_state: &mut EngineState,
stack: &mut Stack,
) -> Result<(), ShellError> {
if let Some(hook) = env_change_hook {
match hook {
Value::Record { val, .. } => {
for (env_name, hook_value) in &*val {
let before = engine_state.previous_env_vars.get(env_name);
let after = stack.get_env_var(engine_state, env_name);
if before != after {
let before = before.cloned().unwrap_or_default();
let after = after.cloned().unwrap_or_default();
for (env, hooks) in env_change_hook {
let before = engine_state.previous_env_vars.get(env);
let after = stack.get_env_var(engine_state, env);
if before != after {
let before = before.cloned().unwrap_or_default();
let after = after.cloned().unwrap_or_default();
eval_hook(
engine_state,
stack,
None,
vec![("$before".into(), before), ("$after".into(), after.clone())],
hook_value,
"env_change",
)?;
eval_hooks(
engine_state,
stack,
vec![("$before".into(), before), ("$after".into(), after.clone())],
hooks,
"env_change",
)?;
Arc::make_mut(&mut engine_state.previous_env_vars)
.insert(env_name.clone(), after);
}
}
}
x => {
return Err(ShellError::TypeMismatch {
err_message: "record for the 'env_change' hook".to_string(),
span: x.span(),
});
}
Arc::make_mut(&mut engine_state.previous_env_vars).insert(env.clone(), after);
}
}
Ok(())
}
pub fn eval_hooks(
engine_state: &mut EngineState,
stack: &mut Stack,
arguments: Vec<(String, Value)>,
hooks: &[Value],
hook_name: &str,
) -> Result<(), ShellError> {
for hook in hooks {
eval_hook(
engine_state,
stack,
None,
arguments.clone(),
hook,
&format!("{hook_name} list, recursive"),
)?;
}
Ok(())
}
pub fn eval_hook(
engine_state: &mut EngineState,
stack: &mut Stack,
@ -127,16 +133,7 @@ pub fn eval_hook(
}
}
Value::List { vals, .. } => {
for val in vals {
eval_hook(
engine_state,
stack,
None,
arguments.clone(),
val,
&format!("{hook_name} list, recursive"),
)?;
}
eval_hooks(engine_state, stack, arguments, vals, hook_name)?;
}
Value::Record { val, .. } => {
// Hooks can optionally be a record in this form:

View File

@ -45,6 +45,7 @@ On Windows based systems, Nushell will wait for the command to finish and then e
call.head,
engine_state,
stack,
&cwd,
));
};
executable

View File

@ -1,6 +1,6 @@
use nu_cmd_base::hook::eval_hook;
use nu_engine::{command_prelude::*, env_to_strings, get_eval_expression};
use nu_path::{dots::expand_ndots, expand_tilde};
use nu_path::{dots::expand_ndots, expand_tilde, AbsolutePath};
use nu_protocol::{did_you_mean, process::ChildProcess, ByteStream, NuGlob, OutDest, Signals};
use nu_system::ForegroundChild;
use nu_utils::IgnoreCaseExt;
@ -126,7 +126,13 @@ impl Command for External {
// effect if it's an absolute path already
let paths = nu_engine::env::path_str(engine_state, stack, call.head)?;
let Some(executable) = which(&expanded_name, &paths, cwd.as_ref()) else {
return Err(command_not_found(&name_str, call.head, engine_state, stack));
return Err(command_not_found(
&name_str,
call.head,
engine_state,
stack,
&cwd,
));
};
executable
};
@ -433,6 +439,7 @@ pub fn command_not_found(
span: Span,
engine_state: &EngineState,
stack: &mut Stack,
cwd: &AbsolutePath,
) -> ShellError {
// Run the `command_not_found` hook if there is one.
if let Some(hook) = &stack.get_config(engine_state).hooks.command_not_found {
@ -543,12 +550,12 @@ pub fn command_not_found(
}
// If we find a file, it's likely that the user forgot to set permissions
if Path::new(name).is_file() {
if cwd.join(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 set execute permissions?"),
span,
};
label: format!("Command `{name}` not found"),
help: format!("`{name}` refers to a file that is not executable. Did you forget to set execute permissions?"),
span,
};
}
// We found nothing useful. Give up and return a generic error message.

View File

@ -1,13 +1,13 @@
use super::prelude::*;
use crate as nu_protocol;
use crate::Record;
use std::collections::HashMap;
/// Definition of a parsed hook from the config object
#[derive(Clone, Debug, IntoValue, PartialEq, Serialize, Deserialize)]
pub struct Hooks {
pub pre_prompt: Option<Value>,
pub pre_execution: Option<Value>,
pub env_change: Option<Value>,
pub pre_prompt: Vec<Value>,
pub pre_execution: Vec<Value>,
pub env_change: HashMap<String, Vec<Value>>,
pub display_output: Option<Value>,
pub command_not_found: Option<Value>,
}
@ -15,14 +15,14 @@ pub struct Hooks {
impl Hooks {
pub fn new() -> Self {
Self {
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())),
pre_prompt: Vec::new(),
pre_execution: Vec::new(),
env_change: HashMap::new(),
display_output: Some(Value::string(
"if (term size).columns >= 100 { table -e } else { table }",
Span::unknown(),
)),
command_not_found: Some(Value::list(vec![], Span::unknown())),
command_not_found: None,
}
}
}
@ -40,14 +40,6 @@ impl UpdateFromValue for Hooks {
path: &mut ConfigPath<'a>,
errors: &mut ConfigErrors,
) {
fn update_option(field: &mut Option<Value>, value: &Value) {
if value.is_nothing() {
*field = None;
} else {
*field = Some(value.clone());
}
}
let Value::Record { val: record, .. } = value else {
errors.type_mismatch(path, Type::record(), value);
return;
@ -56,11 +48,57 @@ impl UpdateFromValue for Hooks {
for (col, val) in record.iter() {
let path = &mut path.push(col);
match col.as_str() {
"pre_prompt" => update_option(&mut self.pre_prompt, val),
"pre_execution" => update_option(&mut self.pre_execution, val),
"env_change" => update_option(&mut self.env_change, val),
"display_output" => update_option(&mut self.display_output, val),
"command_not_found" => update_option(&mut self.command_not_found, val),
"pre_prompt" => {
if let Ok(hooks) = val.as_list() {
self.pre_prompt = hooks.into()
} else {
errors.type_mismatch(path, Type::list(Type::Any), val);
}
}
"pre_execution" => {
if let Ok(hooks) = val.as_list() {
self.pre_execution = hooks.into()
} else {
errors.type_mismatch(path, Type::list(Type::Any), val);
}
}
"env_change" => {
if let Ok(record) = val.as_record() {
self.env_change = record
.iter()
.map(|(key, val)| {
let old = self.env_change.remove(key).unwrap_or_default();
let new = if let Ok(hooks) = val.as_list() {
hooks.into()
} else {
errors.type_mismatch(
&path.push(key),
Type::list(Type::Any),
val,
);
old
};
(key.as_str().into(), new)
})
.collect();
} else {
errors.type_mismatch(path, Type::record(), val);
}
}
"display_output" => {
self.display_output = if val.is_nothing() {
None
} else {
Some(val.clone())
}
}
"command_not_found" => {
self.command_not_found = if val.is_nothing() {
None
} else {
Some(val.clone())
}
}
_ => errors.unknown_option(path, val),
}
}

View File

@ -1,4 +1,4 @@
use nu_cmd_base::hook::{eval_env_change_hook, eval_hook};
use nu_cmd_base::hook::{eval_env_change_hook, eval_hooks};
use nu_engine::eval_block;
use nu_parser::parse;
use nu_protocol::{
@ -250,24 +250,14 @@ pub fn nu_repl() {
}
// Check for pre_prompt hook
let config = engine_state.get_config();
if let Some(hook) = config.hooks.pre_prompt.clone() {
if let Err(err) = eval_hook(
&mut engine_state,
&mut stack,
None,
vec![],
&hook,
"pre_prompt",
) {
outcome_err(&engine_state, &err);
}
let hook = engine_state.get_config().hooks.pre_prompt.clone();
if let Err(err) = eval_hooks(&mut engine_state, &mut stack, vec![], &hook, "pre_prompt") {
outcome_err(&engine_state, &err);
}
// Check for env change hook
let config = engine_state.get_config();
if let Err(err) = eval_env_change_hook(
config.hooks.env_change.clone(),
&engine_state.get_config().hooks.env_change.clone(),
&mut engine_state,
&mut stack,
) {
@ -275,7 +265,6 @@ pub fn nu_repl() {
}
// Check for pre_execution hook
let config = engine_state.get_config();
engine_state
.repl_state
@ -283,17 +272,15 @@ pub fn nu_repl() {
.expect("repl state mutex")
.buffer = line.to_string();
if let Some(hook) = config.hooks.pre_execution.clone() {
if let Err(err) = eval_hook(
&mut engine_state,
&mut stack,
None,
vec![],
&hook,
"pre_execution",
) {
outcome_err(&engine_state, &err);
}
let hook = engine_state.get_config().hooks.pre_execution.clone();
if let Err(err) = eval_hooks(
&mut engine_state,
&mut stack,
vec![],
&hook,
"pre_execution",
) {
outcome_err(&engine_state, &err);
}
// Eval the REPL line

View File

@ -28,7 +28,7 @@ fn env_change_hook(name: &str, code: &str) -> String {
"$env.config = {{
hooks: {{
env_change: {{
{name} : {code}
{name}: [{code}]
}}
}}
}}"
@ -40,9 +40,9 @@ fn env_change_hook_code(name: &str, code: &str) -> String {
"$env.config = {{
hooks: {{
env_change: {{
{name} : {{
{name}: [{{
code: {code}
}}
}}]
}}
}}
}}"
@ -54,10 +54,10 @@ fn env_change_hook_code_condition(name: &str, condition: &str, code: &str) -> St
"$env.config = {{
hooks: {{
env_change: {{
{name} : {{
{name}: [{{
condition: {condition}
code: {code}
}}
}}]
}}
}}
}}"
@ -68,7 +68,7 @@ fn pre_prompt_hook(code: &str) -> String {
format!(
"$env.config = {{
hooks: {{
pre_prompt: {code}
pre_prompt: [{code}]
}}
}}"
)
@ -78,9 +78,9 @@ fn pre_prompt_hook_code(code: &str) -> String {
format!(
"$env.config = {{
hooks: {{
pre_prompt: {{
pre_prompt: [{{
code: {code}
}}
}}]
}}
}}"
)
@ -90,7 +90,7 @@ fn pre_execution_hook(code: &str) -> String {
format!(
"$env.config = {{
hooks: {{
pre_execution: {code}
pre_execution: [{code}]
}}
}}"
)
@ -100,9 +100,9 @@ fn pre_execution_hook_code(code: &str) -> String {
format!(
"$env.config = {{
hooks: {{
pre_execution: {{
pre_execution: [{{
code: {code}
}}
}}]
}}
}}"
)
@ -536,9 +536,9 @@ fn err_hook_parse_error() {
r#"$env.config = {
hooks: {
env_change: {
FOO : {
FOO: [{
code: "def foo { 'foo' }"
}
}]
}
}
}"#,