Fix config creation during printing (#9353)

This commit is contained in:
Jakub Žádník 2023-06-04 22:04:28 +03:00 committed by GitHub
parent df15fc24fe
commit 82e6873702
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
15 changed files with 66 additions and 100 deletions

View file

@ -100,12 +100,7 @@ pub fn eval_config_contents(
// Merge the environment in case env vars changed in the config
match nu_engine::env::current_dir(engine_state, stack) {
Ok(cwd) => {
if let Err(e) = engine_state.merge_env(stack) {
let working_set = StateWorkingSet::new(engine_state);
report_error(&working_set, &e);
}
if let Err(e) = engine_state.set_current_working_dir(cwd) {
if let Err(e) = engine_state.merge_env(stack, cwd) {
let working_set = StateWorkingSet::new(engine_state);
report_error(&working_set, &e);
}

View file

@ -53,12 +53,6 @@ Since this command has no output, there is no point in piping it with other comm
let no_newline = call.has_flag("no-newline");
let to_stderr = call.has_flag("stderr");
// We merge stack to make sure we render the changes if any were made in the `block`
//
// CONSIDERED TO BE A CODE SMELL AND IT BETTER BE RESOLVED UPWARDS THE CALLING STACK
let engine = engine_state.clone_with_env(stack)?;
let engine_state = &engine;
// This will allow for easy printing of pipelines as well
if !args.is_empty() {
for arg in args {

View file

@ -172,8 +172,7 @@ pub fn evaluate_repl(
PipelineData::empty(),
false,
);
engine_state.merge_env(stack)?;
engine_state.set_current_working_dir(get_guaranteed_cwd(engine_state, stack))?;
engine_state.merge_env(stack, get_guaranteed_cwd(engine_state, stack))?;
}
engine_state.set_startup_time(entire_start_time.elapsed().as_nanos() as i64);
@ -192,18 +191,14 @@ pub fn evaluate_repl(
loop {
let loop_start_time = std::time::Instant::now();
let cwd = get_guaranteed_cwd(engine_state, stack);
start_time = std::time::Instant::now();
// Before doing anything, merge the environment from the previous REPL iteration into the
// permanent state.
if let Err(err) = engine_state.merge_env(stack) {
if let Err(err) = engine_state.merge_env(stack, cwd) {
report_error_new(engine_state, &err);
}
let cwd = get_guaranteed_cwd(engine_state, stack);
if let Err(err) = engine_state.set_current_working_dir(cwd) {
report_error_new(engine_state, &err);
}
perf(
"merge env",
start_time,

View file

@ -246,10 +246,6 @@ pub fn eval_source(
match b {
Ok(pipeline_data) => {
// we merge stack here because the block could change the envirenemnt,
// and we need to render it while do print.
let _ = engine_state.merge_env(stack);
let config = engine_state.get_config();
let result;
if let PipelineData::ExternalStream {

View file

@ -768,8 +768,7 @@ fn run_external_completion(block: &str, input: &str) -> Vec<Suggestion> {
assert!(engine_state.merge_delta(delta).is_ok());
// Merge environment into the permanent state
assert!(engine_state.merge_env(&mut stack).is_ok());
assert!(engine_state.set_current_working_dir(&dir).is_ok());
assert!(engine_state.merge_env(&mut stack, &dir).is_ok());
let latest_block_id = engine_state.num_blocks() - 1;

View file

@ -61,8 +61,8 @@ pub fn new_engine() -> (PathBuf, String, EngineState, Stack) {
);
// Merge environment into the permanent state
assert!(engine_state.merge_env(&mut stack).is_ok());
assert!(engine_state.set_current_working_dir(&dir).is_ok());
let merge_result = engine_state.merge_env(&mut stack, &dir);
assert!(merge_result.is_ok());
(dir, dir_str, engine_state, stack)
}
@ -100,8 +100,8 @@ pub fn new_quote_engine() -> (PathBuf, String, EngineState, Stack) {
);
// Merge environment into the permanent state
assert!(engine_state.merge_env(&mut stack).is_ok());
assert!(engine_state.set_current_working_dir(&dir).is_ok());
let merge_result = engine_state.merge_env(&mut stack, &dir);
assert!(merge_result.is_ok());
(dir, dir_str, engine_state, stack)
}
@ -171,8 +171,5 @@ pub fn merge_input(
.is_ok());
// Merge environment into the permanent state
engine_state.merge_env(stack)?;
engine_state.set_current_working_dir(&dir)?;
Ok(())
engine_state.merge_env(stack, &dir)
}

View file

@ -167,13 +167,9 @@ pub fn check_example_evaluates_to_expected_output(
stack.add_env_var("PWD".to_string(), Value::test_string(cwd.to_string_lossy()));
engine_state
.merge_env(&mut stack)
.merge_env(&mut stack, cwd)
.expect("Error merging environment");
engine_state
.set_current_working_dir(cwd)
.expect("Error setting CWD");
let empty_input = PipelineData::empty();
let result = eval(example.example, empty_input, cwd, engine_state);

View file

@ -1,7 +1,7 @@
use crate::text_style::Alignment;
use crate::{color_record_to_nustyle, lookup_ansi_color_style, TextStyle};
use nu_ansi_term::{Color, Style};
use nu_engine::eval_block;
use nu_engine::{env::get_config, eval_block};
use nu_protocol::{
engine::{EngineState, Stack, StateWorkingSet},
CliError, IntoPipelineData, Value,
@ -138,7 +138,7 @@ impl<'a> StyleComputer<'a> {
// The main constructor.
pub fn from_config(engine_state: &'a EngineState, stack: &'a Stack) -> StyleComputer<'a> {
let config = engine_state.get_config();
let config = get_config(engine_state, stack);
// Create the hashmap
#[rustfmt::skip]

View file

@ -281,10 +281,8 @@ pub fn eval_hook(
}
}
engine_state.merge_env(stack)?;
let cwd = get_guaranteed_cwd(engine_state, stack);
engine_state.set_current_working_dir(cwd)?;
engine_state.merge_env(stack, cwd)?;
Ok(output)
}

View file

@ -1,7 +1,7 @@
use lscolors::{LsColors, Style};
use nu_color_config::color_from_hex;
use nu_color_config::{StyleComputer, TextStyle};
use nu_engine::{env_to_string, CallExt};
use nu_engine::{env::get_config, env_to_string, CallExt};
use nu_protocol::{
ast::Call,
engine::{Command, EngineState, Stack},
@ -102,12 +102,6 @@ impl Command for Table {
call: &Call,
input: PipelineData,
) -> Result<PipelineData, ShellError> {
// We merge stack to make sure we render the changes if any were made in the `block`
//
// CONSIDERED TO BE A CODE SMELL AND IT BETTER BE RESOLVED UPWARDS THE CALLING STACK
let engine = engine_state.clone_with_env(stack)?;
let engine_state = &engine;
let start_num: Option<i64> = call.get_flag(engine_state, stack, "start-number")?;
let row_offset = start_num.unwrap_or_default() as usize;
let list: bool = call.has_flag("list");
@ -237,7 +231,7 @@ fn handle_table_command(
term_width: Option<i64>,
) -> Result<PipelineData, ShellError> {
let ctrlc = engine_state.ctrlc.clone();
let config = engine_state.get_config();
let config = get_config(engine_state, stack);
match input {
PipelineData::ExternalStream { .. } => Ok(input),
@ -293,7 +287,7 @@ fn handle_table_command(
table_view,
term_width,
ctrlc,
config,
&config,
)
}
PipelineData::Value(Value::LazyRecord { val, .. }, ..) => {
@ -454,7 +448,7 @@ fn handle_row_stream(
Some(PipelineMetadata {
data_source: DataSource::Ls,
}) => {
let config = engine_state.config.clone();
let config = get_config(engine_state, stack);
let ctrlc = ctrlc.clone();
let ls_colors_env_str = match stack.get_env_var(engine_state, "LS_COLORS") {
Some(v) => Some(env_to_string("LS_COLORS", &v, engine_state, stack)?),
@ -652,13 +646,13 @@ impl PagingTableCreator {
return Ok(None);
}
let config = self.engine_state.get_config();
let config = get_config(&self.engine_state, &self.stack);
let style_computer = StyleComputer::from_config(&self.engine_state, &self.stack);
let term_width = get_width_param(self.width_param);
let ctrlc = self.ctrlc.clone();
let span = self.head;
let opts = BuildConfig::new(ctrlc, config, &style_computer, span, term_width);
let opts = BuildConfig::new(ctrlc, &config, &style_computer, span, term_width);
let view = TableView::Expanded {
limit,
flatten,
@ -673,24 +667,24 @@ impl PagingTableCreator {
return Ok(None);
}
let config = self.engine_state.get_config();
let config = get_config(&self.engine_state, &self.stack);
let style_computer = StyleComputer::from_config(&self.engine_state, &self.stack);
let term_width = get_width_param(self.width_param);
let ctrlc = self.ctrlc.clone();
let span = self.head;
let opts = BuildConfig::new(ctrlc, config, &style_computer, span, term_width);
let opts = BuildConfig::new(ctrlc, &config, &style_computer, span, term_width);
build_table_batch(batch, TableView::Collapsed, 0, opts)
}
fn build_general(&mut self, batch: Vec<Value>) -> StringResult {
let term_width = get_width_param(self.width_param);
let config = &self.engine_state.get_config();
let config = get_config(&self.engine_state, &self.stack);
let style_computer = StyleComputer::from_config(&self.engine_state, &self.stack);
let ctrlc = self.ctrlc.clone();
let span = self.head;
let row_offset = self.row_offset;
let opts = BuildConfig::new(ctrlc, config, &style_computer, span, term_width);
let opts = BuildConfig::new(ctrlc, &config, &style_computer, span, term_width);
build_table_batch(batch, TableView::General, row_offset, opts)
}
@ -762,7 +756,7 @@ impl Iterator for PagingTableCreator {
match table {
Ok(Some(table)) => {
let table = maybe_strip_color(table, self.engine_state.get_config());
let table = maybe_strip_color(table, &get_config(&self.engine_state, &self.stack));
let mut bytes = table.as_bytes().to_vec();
bytes.push(b'\n'); // nu-table tables don't come with a newline on the end
@ -898,7 +892,7 @@ fn create_empty_placeholder(
engine_state: &EngineState,
stack: &Stack,
) -> String {
let config = engine_state.get_config();
let config = get_config(engine_state, stack);
if !config.table_show_empty {
return String::new();
}
@ -910,7 +904,7 @@ fn create_empty_placeholder(
let out = TableOutput::new(table, false, false);
let style_computer = &StyleComputer::from_config(engine_state, stack);
let config = create_table_config(config, style_computer, &out);
let config = create_table_config(&config, style_computer, &out);
out.table
.draw(config, termwidth)

View file

@ -3,7 +3,7 @@ use std::path::{Path, PathBuf};
use nu_protocol::ast::{Call, Expr, PathMember};
use nu_protocol::engine::{EngineState, Stack};
use nu_protocol::{PipelineData, ShellError, Span, Value, VarId};
use nu_protocol::{Config, PipelineData, ShellError, Span, Value, VarId};
use nu_path::canonicalize_with;
@ -303,6 +303,18 @@ pub fn find_in_dirs_env(
Ok(check_dir(lib_dirs).or_else(|| check_dir(lib_dirs_fallback)))
}
/// Get config
///
/// This combines config stored in permanent state and any runtime updates to the environment. This
/// is the canonical way to fetch config at runtime when you have Stack available.
pub fn get_config(engine_state: &EngineState, stack: &Stack) -> Config {
if let Some(mut config_record) = stack.get_env_var(engine_state, "config") {
config_record.into_config(engine_state.get_config()).0
} else {
engine_state.get_config().clone()
}
}
fn get_converted_value(
engine_state: &EngineState,
stack: &Stack,

View file

@ -279,7 +279,11 @@ impl EngineState {
}
/// Merge the environment from the runtime Stack into the engine state
pub fn merge_env(&mut self, stack: &mut Stack) -> Result<(), ShellError> {
pub fn merge_env(
&mut self,
stack: &mut Stack,
cwd: impl AsRef<Path>,
) -> Result<(), ShellError> {
for mut scope in stack.env_vars.drain(..) {
for (overlay_name, mut env) in scope.drain() {
if let Some(env_vars) = self.env_vars.get_mut(&overlay_name) {
@ -306,27 +310,12 @@ impl EngineState {
}
}
Ok(())
}
/// Set a CWD.
pub fn set_current_working_dir(&mut self, cwd: impl AsRef<Path>) -> Result<(), ShellError> {
// TODO: better error
std::env::set_current_dir(cwd)?;
Ok(())
}
/// Merge the environment from the runtime Stack into the engine state
///
/// A merge which does not consume the stack env.
pub fn clone_with_env(&self, stack: &Stack) -> Result<Self, ShellError> {
let mut engine = self.clone();
let mut stack = stack.clone();
engine.merge_env(&mut stack)?;
Ok(engine)
}
/// Mark a starting point if it is a script (e.g., nu spam.nu)
pub fn start_in_file(&mut self, file_path: Option<&str>) {
self.currently_parsed_cwd = if let Some(path) = file_path {
@ -1665,6 +1654,10 @@ impl<'a> StateWorkingSet<'a> {
self.permanent_state.get_env_var(name)
}
/// Returns a reference to the config stored at permanent state
///
/// At runtime, you most likely want to call nu_engine::env::get_config because this method
/// does not capture environment updates during runtime.
pub fn get_config(&self) -> &Config {
&self.permanent_state.config
}

View file

@ -98,8 +98,7 @@ use std dirs [
)?;
let cwd = current_dir(engine_state, &stack)?;
engine_state.merge_env(&mut stack)?;
engine_state.set_current_working_dir(cwd)?;
engine_state.merge_env(&mut stack, cwd)?;
Ok(())
}

View file

@ -136,7 +136,18 @@ pub(crate) fn read_default_env_file(engine_state: &mut EngineState, stack: &mut
info!("read_config_file {}:{}:{}", file!(), line!(), column!());
// Merge the environment in case env vars changed in the config
merge_env(engine_state, stack)
match nu_engine::env::current_dir(engine_state, stack) {
Ok(cwd) => {
if let Err(e) = engine_state.merge_env(stack, cwd) {
let working_set = StateWorkingSet::new(engine_state);
report_error(&working_set, &e);
}
}
Err(e) => {
let working_set = StateWorkingSet::new(engine_state);
report_error(&working_set, &e);
}
}
}
fn eval_default_config(
@ -161,18 +172,9 @@ fn eval_default_config(
);
// Merge the environment in case env vars changed in the config
merge_env(engine_state, stack)
}
fn merge_env(engine_state: &mut EngineState, stack: &mut Stack) {
match nu_engine::env::current_dir(engine_state, stack) {
Ok(cwd) => {
if let Err(e) = engine_state.merge_env(stack) {
let working_set = StateWorkingSet::new(engine_state);
report_error(&working_set, &e);
}
if let Err(e) = engine_state.set_current_working_dir(cwd) {
if let Err(e) = engine_state.merge_env(stack, cwd) {
let working_set = StateWorkingSet::new(engine_state);
report_error(&working_set, &e);
}

View file

@ -188,11 +188,7 @@ pub fn nu_repl() {
// Before doing anything, merge the environment from the previous REPL iteration into the
// permanent state.
if let Err(err) = engine_state.merge_env(&mut stack) {
outcome_err(&engine_state, &err);
}
if let Err(err) = engine_state.set_current_working_dir(&cwd) {
if let Err(err) = engine_state.merge_env(&mut stack, &cwd) {
outcome_err(&engine_state, &err);
}