From aed4b626b8a9d42c2f2e53cff26cd382aa25230c Mon Sep 17 00:00:00 2001 From: Ian Manske Date: Wed, 8 Nov 2023 22:57:24 +0000 Subject: [PATCH] Refactor env conversion, yeet `Value::follow_cell_path_not...` (#10926) # Description Replaces the only usage of `Value::follow_cell_path_not_from_user_input` with some `Record::get`s. # User-Facing Changes Breaking change for `nu-protocol`, since `Value::follow_cell_path_not_from_user_input` was deleted. Nushell now reports errors for when environment conversions are not closures. --- crates/nu-engine/src/env.rs | 87 +++++++++++++---------------- crates/nu-protocol/src/value/mod.rs | 41 ++------------ 2 files changed, 44 insertions(+), 84 deletions(-) diff --git a/crates/nu-engine/src/env.rs b/crates/nu-engine/src/env.rs index af3a7177e..352994360 100644 --- a/crates/nu-engine/src/env.rs +++ b/crates/nu-engine/src/env.rs @@ -1,7 +1,7 @@ use std::collections::HashMap; use std::path::{Path, PathBuf}; -use nu_protocol::ast::{Call, Expr, PathMember}; +use nu_protocol::ast::{Call, Expr}; use nu_protocol::engine::{EngineState, Stack, StateWorkingSet, PWD_ENV}; use nu_protocol::{Config, PipelineData, ShellError, Span, Value, VarId}; @@ -367,59 +367,48 @@ fn get_converted_value( orig_val: &Value, direction: &str, ) -> ConversionResult { - if let Some(env_conversions) = stack.get_env_var(engine_state, ENV_CONVERSIONS) { - let env_span = env_conversions.span(); - let val_span = orig_val.span(); + let conversions = stack.get_env_var(engine_state, ENV_CONVERSIONS); + let conversion = conversions + .as_ref() + .and_then(|val| val.as_record().ok()) + .and_then(|record| record.get(name)) + .and_then(|val| val.as_record().ok()) + .and_then(|record| record.get(direction)); - let path_members = &[ - PathMember::String { - val: name.to_string(), - span: env_span, - optional: false, - }, - PathMember::String { - val: direction.to_string(), - span: env_span, - optional: false, - }, - ]; + if let Some(conversion) = conversion { + let from_span = conversion.span(); + match conversion.as_closure() { + Ok(val) => { + let block = engine_state.get_block(val.block_id); - if let Ok(v) = env_conversions.follow_cell_path_not_from_user_input(path_members, false) { - let from_span = v.span(); - match v { - Value::Closure { val, .. } => { - let block = engine_state.get_block(val.block_id); - - if let Some(var) = block.signature.get_positional(0) { - let mut stack = stack.gather_captures(engine_state, &block.captures); - if let Some(var_id) = &var.var_id { - stack.add_var(*var_id, orig_val.clone()); - } - - let result = eval_block( - engine_state, - &mut stack, - block, - PipelineData::new_with_metadata(None, val_span), - true, - true, - ); - - match result { - Ok(data) => ConversionResult::Ok(data.into_value(val_span)), - Err(e) => ConversionResult::ConversionError(e), - } - } else { - ConversionResult::ConversionError(ShellError::MissingParameter { - param_name: "block input".into(), - span: from_span, - }) + if let Some(var) = block.signature.get_positional(0) { + let mut stack = stack.gather_captures(engine_state, &block.captures); + if let Some(var_id) = &var.var_id { + stack.add_var(*var_id, orig_val.clone()); } + + let val_span = orig_val.span(); + let result = eval_block( + engine_state, + &mut stack, + block, + PipelineData::new_with_metadata(None, val_span), + true, + true, + ); + + match result { + Ok(data) => ConversionResult::Ok(data.into_value(val_span)), + Err(e) => ConversionResult::ConversionError(e), + } + } else { + ConversionResult::ConversionError(ShellError::MissingParameter { + param_name: "block input".into(), + span: from_span, + }) } - _ => ConversionResult::CellPathError, } - } else { - ConversionResult::CellPathError + Err(e) => ConversionResult::ConversionError(e), } } else { ConversionResult::CellPathError diff --git a/crates/nu-protocol/src/value/mod.rs b/crates/nu-protocol/src/value/mod.rs index a271a63c9..c317a3133 100644 --- a/crates/nu-protocol/src/value/mod.rs +++ b/crates/nu-protocol/src/value/mod.rs @@ -919,23 +919,6 @@ impl Value { self, cell_path: &[PathMember], insensitive: bool, - ) -> Result { - self.follow_cell_path_helper(cell_path, insensitive, true) - } - - pub fn follow_cell_path_not_from_user_input( - self, - cell_path: &[PathMember], - insensitive: bool, - ) -> Result { - self.follow_cell_path_helper(cell_path, insensitive, false) - } - - fn follow_cell_path_helper( - self, - cell_path: &[PathMember], - insensitive: bool, - from_user_input: bool, ) -> Result { let mut current = self; @@ -1033,17 +1016,11 @@ impl Value { current = found.1.clone(); } else if *optional { return Ok(Value::nothing(*origin_span)); // short-circuit + } else if let Some(suggestion) = + did_you_mean(val.columns(), column_name) + { + return Err(ShellError::DidYouMean(suggestion, *origin_span)); } else { - if from_user_input { - if let Some(suggestion) = - did_you_mean(val.columns(), column_name) - { - return Err(ShellError::DidYouMean( - suggestion, - *origin_span, - )); - } - } return Err(ShellError::CantFindColumn { col_name: column_name.to_string(), span: *origin_span, @@ -1058,15 +1035,9 @@ impl Value { current = val.get_column_value(column_name)?; } else if *optional { return Ok(Value::nothing(*origin_span)); // short-circuit + } else if let Some(suggestion) = did_you_mean(&columns, column_name) { + return Err(ShellError::DidYouMean(suggestion, *origin_span)); } else { - if from_user_input { - if let Some(suggestion) = did_you_mean(&columns, column_name) { - return Err(ShellError::DidYouMean( - suggestion, - *origin_span, - )); - } - } return Err(ShellError::CantFindColumn { col_name: column_name.to_string(), span: *origin_span,