From e2c4ff8180f2352fa759408b82aa49c53911c5bc Mon Sep 17 00:00:00 2001 From: Darren Schroeder <343840+fdncred@users.noreply.github.com> Date: Mon, 16 Dec 2024 13:52:07 -0600 Subject: [PATCH 01/26] Revert "Feature: PWD-per-drive to facilitate working on multiple drives at Windows" (#14598) Reverts nushell/nushell#14411 --- .../src/completions/completion_common.rs | 8 - crates/nu-cli/src/repl.rs | 6 - crates/nu-command/src/filesystem/cd.rs | 2 +- crates/nu-engine/src/env.rs | 3 - crates/nu-engine/src/eval.rs | 5 - crates/nu-engine/src/eval_ir.rs | 34 +- crates/nu-path/src/lib.rs | 4 - crates/nu-path/src/pwd_per_drive.rs | 331 ------------------ crates/nu-protocol/src/engine/stack.rs | 35 -- src/run.rs | 31 -- 10 files changed, 29 insertions(+), 430 deletions(-) delete mode 100644 crates/nu-path/src/pwd_per_drive.rs diff --git a/crates/nu-cli/src/completions/completion_common.rs b/crates/nu-cli/src/completions/completion_common.rs index d96ca365eb..4b5ef857e1 100644 --- a/crates/nu-cli/src/completions/completion_common.rs +++ b/crates/nu-cli/src/completions/completion_common.rs @@ -174,14 +174,6 @@ pub fn complete_item( ) -> Vec { let cleaned_partial = surround_remove(partial); let isdir = cleaned_partial.ends_with(is_separator); - #[cfg(windows)] - let cleaned_partial = if let Some(absolute_partial) = - stack.pwd_per_drive.expand_pwd(Path::new(&cleaned_partial)) - { - absolute_partial.display().to_string() - } else { - cleaned_partial - }; let expanded_partial = expand_ndots(Path::new(&cleaned_partial)); let should_collapse_dots = expanded_partial != Path::new(&cleaned_partial); let mut partial = expanded_partial.to_string_lossy().to_string(); diff --git a/crates/nu-cli/src/repl.rs b/crates/nu-cli/src/repl.rs index 0476435aee..4bd86ddbf9 100644 --- a/crates/nu-cli/src/repl.rs +++ b/crates/nu-cli/src/repl.rs @@ -832,12 +832,6 @@ fn do_auto_cd( engine_state: &mut EngineState, span: Span, ) { - #[cfg(windows)] - let path = if let Some(abs_path) = stack.pwd_per_drive.expand_pwd(path.as_path()) { - abs_path - } else { - path - }; let path = { if !path.exists() { report_shell_error( diff --git a/crates/nu-command/src/filesystem/cd.rs b/crates/nu-command/src/filesystem/cd.rs index 3b5aa377fe..2af932bcb4 100644 --- a/crates/nu-command/src/filesystem/cd.rs +++ b/crates/nu-command/src/filesystem/cd.rs @@ -87,7 +87,7 @@ impl Command for Cd { }); } } else { - let path = stack.expand_path_with(path_no_whitespace, &cwd, true); + let path = nu_path::expand_path_with(path_no_whitespace, &cwd, true); if !path.exists() { return Err(ShellError::DirectoryNotFound { dir: path_no_whitespace.to_string(), diff --git a/crates/nu-engine/src/env.rs b/crates/nu-engine/src/env.rs index bec885173a..2ef7ecc1d9 100644 --- a/crates/nu-engine/src/env.rs +++ b/crates/nu-engine/src/env.rs @@ -156,9 +156,6 @@ pub fn env_to_strings( } } - #[cfg(windows)] - stack.pwd_per_drive.get_env_vars(&mut env_vars_str); - Ok(env_vars_str) } diff --git a/crates/nu-engine/src/eval.rs b/crates/nu-engine/src/eval.rs index d41e5753bd..0e3917290f 100644 --- a/crates/nu-engine/src/eval.rs +++ b/crates/nu-engine/src/eval.rs @@ -194,11 +194,6 @@ pub fn redirect_env(engine_state: &EngineState, caller_stack: &mut Stack, callee caller_stack.add_env_var(var, value); } - #[cfg(windows)] - { - caller_stack.pwd_per_drive = callee_stack.pwd_per_drive.clone(); - } - // set config to callee config, to capture any updates to that caller_stack.config.clone_from(&callee_stack.config); } diff --git a/crates/nu-engine/src/eval_ir.rs b/crates/nu-engine/src/eval_ir.rs index 7b4c0a28f5..afcb58a90a 100644 --- a/crates/nu-engine/src/eval_ir.rs +++ b/crates/nu-engine/src/eval_ir.rs @@ -1,6 +1,6 @@ use std::{borrow::Cow, fs::File, sync::Arc}; -use nu_path::AbsolutePathBuf; +use nu_path::{expand_path_with, AbsolutePathBuf}; use nu_protocol::{ ast::{Bits, Block, Boolean, CellPath, Comparison, Math, Operator}, debugger::DebugContext, @@ -14,7 +14,7 @@ use nu_protocol::{ }; use nu_utils::IgnoreCaseExt; -use crate::{eval::is_automatic_env_var, eval_block_with_early_return, redirect_env}; +use crate::{eval::is_automatic_env_var, eval_block_with_early_return}; /// Evaluate the compiled representation of a [`Block`]. pub fn eval_ir_block( @@ -870,7 +870,7 @@ fn literal_value( Value::string(path, span) } else { let cwd = ctx.engine_state.cwd(Some(ctx.stack))?; - let path = ctx.stack.expand_path_with(path, cwd, true); + let path = expand_path_with(path, cwd, true); Value::string(path.to_string_lossy(), span) } @@ -890,7 +890,7 @@ fn literal_value( .cwd(Some(ctx.stack)) .map(AbsolutePathBuf::into_std_path_buf) .unwrap_or_default(); - let path = ctx.stack.expand_path_with(path, cwd, true); + let path = expand_path_with(path, cwd, true); Value::string(path.to_string_lossy(), span) } @@ -1405,8 +1405,7 @@ enum RedirectionStream { /// Open a file for redirection fn open_file(ctx: &EvalContext<'_>, path: &Value, append: bool) -> Result, ShellError> { let path_expanded = - ctx.stack - .expand_path_with(path.as_str()?, ctx.engine_state.cwd(Some(ctx.stack))?, true); + expand_path_with(path.as_str()?, ctx.engine_state.cwd(Some(ctx.stack))?, true); let mut options = File::options(); if append { options.append(true); @@ -1486,3 +1485,26 @@ fn eval_iterate( eval_iterate(ctx, dst, stream, end_index) } } + +/// Redirect environment from the callee stack to the caller stack +fn redirect_env(engine_state: &EngineState, caller_stack: &mut Stack, callee_stack: &Stack) { + // TODO: make this more efficient + // Grab all environment variables from the callee + let caller_env_vars = caller_stack.get_env_var_names(engine_state); + + // remove env vars that are present in the caller but not in the callee + // (the callee hid them) + for var in caller_env_vars.iter() { + if !callee_stack.has_env_var(engine_state, var) { + caller_stack.remove_env_var(engine_state, var); + } + } + + // add new env vars from callee to caller + for (var, value) in callee_stack.get_stack_env_vars() { + caller_stack.add_env_var(var, value); + } + + // set config to callee config, to capture any updates to that + caller_stack.config.clone_from(&callee_stack.config); +} diff --git a/crates/nu-path/src/lib.rs b/crates/nu-path/src/lib.rs index 660fae4b22..cf31a5789f 100644 --- a/crates/nu-path/src/lib.rs +++ b/crates/nu-path/src/lib.rs @@ -6,8 +6,6 @@ pub mod expansions; pub mod form; mod helpers; mod path; -#[cfg(windows)] -pub mod pwd_per_drive; mod tilde; mod trailing_slash; @@ -15,7 +13,5 @@ pub use components::components; pub use expansions::{canonicalize_with, expand_path_with, expand_to_real_path, locate_in_dirs}; pub use helpers::{cache_dir, data_dir, home_dir, nu_config_dir}; pub use path::*; -#[cfg(windows)] -pub use pwd_per_drive::DriveToPwdMap; pub use tilde::expand_tilde; pub use trailing_slash::{has_trailing_slash, strip_trailing_slash}; diff --git a/crates/nu-path/src/pwd_per_drive.rs b/crates/nu-path/src/pwd_per_drive.rs deleted file mode 100644 index d7a395174f..0000000000 --- a/crates/nu-path/src/pwd_per_drive.rs +++ /dev/null @@ -1,331 +0,0 @@ -/// Usage for pwd_per_drive on windows -/// -/// let mut map = DriveToPwdMap::new(); -/// -/// Upon change PWD, call map.set_pwd() with absolute path -/// -/// Call map.expand_pwd() with relative path to get absolution path -/// -/// ``` -/// use std::path::{Path, PathBuf}; -/// use nu_path::DriveToPwdMap; -/// -/// let mut map = DriveToPwdMap::new(); -/// -/// // Set PWD for drive C -/// assert!(map.set_pwd(Path::new(r"C:\Users\Home")).is_ok()); -/// -/// // Expand a relative path -/// let expanded = map.expand_pwd(Path::new("c:test")); -/// assert_eq!(expanded, Some(PathBuf::from(r"C:\Users\Home\test"))); -/// -/// // Will NOT expand an absolute path -/// let expanded = map.expand_pwd(Path::new(r"C:\absolute\path")); -/// assert_eq!(expanded, None); -/// -/// // Expand with no drive letter -/// let expanded = map.expand_pwd(Path::new(r"\no_drive")); -/// assert_eq!(expanded, None); -/// -/// // Expand with no PWD set for the drive -/// let expanded = map.expand_pwd(Path::new("D:test")); -/// assert!(expanded.is_some()); -/// let abs_path = expanded.unwrap().as_path().to_str().expect("OK").to_string(); -/// assert!(abs_path.starts_with(r"D:\")); -/// assert!(abs_path.ends_with(r"\test")); -/// -/// // Get env vars for child process -/// use std::collections::HashMap; -/// let mut env = HashMap::::new(); -/// map.get_env_vars(&mut env); -/// assert_eq!(env.get("=C:").unwrap(), r"C:\Users\Home"); -/// ``` -use std::collections::HashMap; -use std::path::{Path, PathBuf}; - -#[derive(Debug, PartialEq)] -pub enum PathError { - InvalidDriveLetter, - InvalidPath, -} - -/// Helper to check if input path is relative path -/// with drive letter, it can be expanded with PWD-per-drive. -fn need_expand(path: &Path) -> bool { - if let Some(path_str) = path.to_str() { - let chars: Vec = path_str.chars().collect(); - if chars.len() >= 2 { - return chars[1] == ':' && (chars.len() == 2 || (chars[2] != '/' && chars[2] != '\\')); - } - } - false -} - -#[derive(Clone, Debug)] -pub struct DriveToPwdMap { - map: [Option; 26], // Fixed-size array for A-Z -} - -impl Default for DriveToPwdMap { - fn default() -> Self { - Self::new() - } -} - -impl DriveToPwdMap { - pub fn new() -> Self { - Self { - map: Default::default(), - } - } - - pub fn env_var_for_drive(drive_letter: char) -> String { - let drive_letter = drive_letter.to_ascii_uppercase(); - format!("={}:", drive_letter) - } - - /// Collect PWD-per-drive as env vars (for child process) - pub fn get_env_vars(&self, env: &mut HashMap) { - for (drive_index, drive_letter) in ('A'..='Z').enumerate() { - if let Some(pwd) = self.map[drive_index].clone() { - if pwd.len() > 3 { - let env_var_for_drive = Self::env_var_for_drive(drive_letter); - env.insert(env_var_for_drive, pwd); - } - } - } - } - - /// Set the PWD for the drive letter in the absolute path. - /// Return PathError for error. - pub fn set_pwd(&mut self, path: &Path) -> Result<(), PathError> { - if let (Some(drive_letter), Some(path_str)) = - (Self::extract_drive_letter(path), path.to_str()) - { - if drive_letter.is_ascii_alphabetic() { - let drive_letter = drive_letter.to_ascii_uppercase(); - // Make sure saved drive letter is upper case - let mut c = path_str.chars(); - match c.next() { - None => Err(PathError::InvalidDriveLetter), - Some(_) => { - let drive_index = drive_letter as usize - 'A' as usize; - let normalized_pwd = drive_letter.to_string() + c.as_str(); - self.map[drive_index] = Some(normalized_pwd); - Ok(()) - } - } - } else { - Err(PathError::InvalidDriveLetter) - } - } else { - Err(PathError::InvalidPath) - } - } - - /// Get the PWD for drive, if not yet, ask GetFullPathNameW() or omnipath, - /// or else return default r"X:\". - fn get_pwd(&self, drive_letter: char) -> Result { - if drive_letter.is_ascii_alphabetic() { - let drive_letter = drive_letter.to_ascii_uppercase(); - let drive_index = drive_letter as usize - 'A' as usize; - Ok(self.map[drive_index].clone().unwrap_or_else(|| { - if let Some(sys_pwd) = get_full_path_name_w(&format!("{}:", drive_letter)) { - sys_pwd - } else { - format!(r"{}:\", drive_letter) - } - })) - } else { - Err(PathError::InvalidDriveLetter) - } - } - - /// Expand a relative path using the PWD-per-drive, return PathBuf - /// of absolute path. - /// Return None if path is not valid or can't get drive letter. - pub fn expand_pwd(&self, path: &Path) -> Option { - if need_expand(path) { - let path_str = path.to_str()?; - if let Some(drive_letter) = Self::extract_drive_letter(path) { - if let Ok(pwd) = self.get_pwd(drive_letter) { - // Combine current PWD with the relative path - let mut base = PathBuf::from(Self::ensure_trailing_delimiter(&pwd)); - // need_expand() and extract_drive_letter() all ensure path_str.len() >= 2 - base.push(&path_str[2..]); // Join PWD with path parts after "C:" - return Some(base); - } - } - } - None // Invalid path or has no drive letter - } - - /// Helper to extract the drive letter from a path, keep case - /// (e.g., `C:test` -> `C`, `d:\temp` -> `d`) - fn extract_drive_letter(path: &Path) -> Option { - path.to_str() - .and_then(|s| s.chars().next()) - .filter(|c| c.is_ascii_alphabetic()) - } - - /// Ensure a path has a trailing `\\` or '/' - fn ensure_trailing_delimiter(path: &str) -> String { - if !path.ends_with('\\') && !path.ends_with('/') { - format!(r"{}\", path) - } else { - path.to_string() - } - } -} - -fn get_full_path_name_w(path_str: &str) -> Option { - use omnipath::sys_absolute; - if let Ok(path_sys_abs) = sys_absolute(Path::new(path_str)) { - Some(path_sys_abs.to_str()?.to_string()) - } else { - None - } -} - -/// Test for Drive2PWD map -#[cfg(test)] -mod tests { - use super::*; - - /// Test or demo usage of PWD-per-drive - /// In doctest, there's no get_full_path_name_w available so can't foresee - /// possible result, here can have more accurate test assert - #[test] - fn test_usage_for_pwd_per_drive() { - let mut map = DriveToPwdMap::new(); - - // Set PWD for drive E - assert!(map.set_pwd(Path::new(r"E:\Users\Home")).is_ok()); - - // Expand a relative path - let expanded = map.expand_pwd(Path::new("e:test")); - assert_eq!(expanded, Some(PathBuf::from(r"E:\Users\Home\test"))); - - // Will NOT expand an absolute path - let expanded = map.expand_pwd(Path::new(r"E:\absolute\path")); - assert_eq!(expanded, None); - - // Expand with no drive letter - let expanded = map.expand_pwd(Path::new(r"\no_drive")); - assert_eq!(expanded, None); - - // Expand with no PWD set for the drive - let expanded = map.expand_pwd(Path::new("F:test")); - if let Some(sys_abs) = get_full_path_name_w("F:") { - assert_eq!( - expanded, - Some(PathBuf::from(format!( - "{}test", - DriveToPwdMap::ensure_trailing_delimiter(&sys_abs) - ))) - ); - } else { - assert_eq!(expanded, Some(PathBuf::from(r"F:\test"))); - } - } - - #[test] - fn test_get_env_vars() { - let mut map = DriveToPwdMap::new(); - map.set_pwd(Path::new(r"I:\Home")).unwrap(); - map.set_pwd(Path::new(r"j:\User")).unwrap(); - - let mut env = HashMap::::new(); - map.get_env_vars(&mut env); - assert_eq!( - env.get(&DriveToPwdMap::env_var_for_drive('I')).unwrap(), - r"I:\Home" - ); - assert_eq!( - env.get(&DriveToPwdMap::env_var_for_drive('J')).unwrap(), - r"J:\User" - ); - } - - #[test] - fn test_expand_pwd() { - let mut drive_map = DriveToPwdMap::new(); - - // Set PWD for drive 'M:' - assert_eq!(drive_map.set_pwd(Path::new(r"M:\Users")), Ok(())); - // or 'm:' - assert_eq!(drive_map.set_pwd(Path::new(r"m:\Users\Home")), Ok(())); - - // Expand a relative path on "M:" - let expanded = drive_map.expand_pwd(Path::new(r"M:test")); - assert_eq!(expanded, Some(PathBuf::from(r"M:\Users\Home\test"))); - // or on "m:" - let expanded = drive_map.expand_pwd(Path::new(r"m:test")); - assert_eq!(expanded, Some(PathBuf::from(r"M:\Users\Home\test"))); - - // Expand an absolute path - let expanded = drive_map.expand_pwd(Path::new(r"m:\absolute\path")); - assert_eq!(expanded, None); - - // Expand with no drive letter - let expanded = drive_map.expand_pwd(Path::new(r"\no_drive")); - assert_eq!(expanded, None); - - // Expand with no PWD set for the drive - let expanded = drive_map.expand_pwd(Path::new("N:test")); - if let Some(pwd_on_drive) = get_full_path_name_w("N:") { - assert_eq!( - expanded, - Some(PathBuf::from(format!( - r"{}test", - DriveToPwdMap::ensure_trailing_delimiter(&pwd_on_drive) - ))) - ); - } else { - assert_eq!(expanded, Some(PathBuf::from(r"N:\test"))); - } - } - - #[test] - fn test_set_and_get_pwd() { - let mut drive_map = DriveToPwdMap::new(); - - // Set PWD for drive 'O' - assert!(drive_map.set_pwd(Path::new(r"O:\Users")).is_ok()); - // Or for drive 'o' - assert!(drive_map.set_pwd(Path::new(r"o:\Users\Example")).is_ok()); - // Get PWD for drive 'O' - assert_eq!(drive_map.get_pwd('O'), Ok(r"O:\Users\Example".to_string())); - // or 'o' - assert_eq!(drive_map.get_pwd('o'), Ok(r"O:\Users\Example".to_string())); - - // Get PWD for drive P (not set yet, but system might already - // have PWD on this drive) - if let Some(pwd_on_drive) = get_full_path_name_w("P:") { - assert_eq!(drive_map.get_pwd('P'), Ok(pwd_on_drive)); - } else { - assert_eq!(drive_map.get_pwd('P'), Ok(r"P:\".to_string())); - } - } - - #[test] - fn test_set_pwd_invalid_path() { - let mut drive_map = DriveToPwdMap::new(); - - // Invalid path (no drive letter) - let result = drive_map.set_pwd(Path::new(r"\InvalidPath")); - assert!(result.is_err()); - assert_eq!(result.unwrap_err(), PathError::InvalidPath); - } - - #[test] - fn test_get_pwd_invalid_drive() { - let drive_map = DriveToPwdMap::new(); - - // Get PWD for a drive not set (e.g., Z) - assert_eq!(drive_map.get_pwd('Z'), Ok(r"Z:\".to_string())); - - // Invalid drive letter (non-alphabetic) - assert_eq!(drive_map.get_pwd('1'), Err(PathError::InvalidDriveLetter)); - } -} diff --git a/crates/nu-protocol/src/engine/stack.rs b/crates/nu-protocol/src/engine/stack.rs index 66f59b78f6..ccd38cc5d3 100644 --- a/crates/nu-protocol/src/engine/stack.rs +++ b/crates/nu-protocol/src/engine/stack.rs @@ -9,7 +9,6 @@ use nu_utils::IgnoreCaseExt; use std::{ collections::{HashMap, HashSet}, fs::File, - path::{Path, PathBuf}, sync::Arc, }; @@ -54,8 +53,6 @@ pub struct Stack { /// Locally updated config. Use [`.get_config()`](Self::get_config) to access correctly. pub config: Option>, pub(crate) out_dest: StackOutDest, - #[cfg(windows)] - pub pwd_per_drive: nu_path::DriveToPwdMap, } impl Default for Stack { @@ -85,8 +82,6 @@ impl Stack { parent_deletions: vec![], config: None, out_dest: StackOutDest::new(), - #[cfg(windows)] - pwd_per_drive: nu_path::DriveToPwdMap::new(), } } @@ -107,8 +102,6 @@ impl Stack { parent_deletions: vec![], config: parent.config.clone(), out_dest: parent.out_dest.clone(), - #[cfg(windows)] - pwd_per_drive: parent.pwd_per_drive.clone(), parent_stack: Some(parent), } } @@ -135,10 +128,6 @@ impl Stack { unique_stack.env_hidden = child.env_hidden; unique_stack.active_overlays = child.active_overlays; unique_stack.config = child.config; - #[cfg(windows)] - { - unique_stack.pwd_per_drive = child.pwd_per_drive.clone(); - } unique_stack } @@ -330,8 +319,6 @@ impl Stack { parent_deletions: vec![], config: self.config.clone(), out_dest: self.out_dest.clone(), - #[cfg(windows)] - pwd_per_drive: self.pwd_per_drive.clone(), } } @@ -365,8 +352,6 @@ impl Stack { parent_deletions: vec![], config: self.config.clone(), out_dest: self.out_dest.clone(), - #[cfg(windows)] - pwd_per_drive: self.pwd_per_drive.clone(), } } @@ -776,29 +761,9 @@ impl Stack { let path = nu_path::strip_trailing_slash(path); let value = Value::string(path.to_string_lossy(), Span::unknown()); self.add_env_var("PWD".into(), value); - // Sync with PWD-per-drive - #[cfg(windows)] - { - let _ = self.pwd_per_drive.set_pwd(&path); - } Ok(()) } } - - // Helper stub/proxy for nu_path::expand_path_with::(path, relative_to, expand_tilde) - // Facilitates file system commands to easily gain the ability to expand PWD-per-drive - pub fn expand_path_with(&self, path: P, relative_to: Q, expand_tilde: bool) -> PathBuf - where - P: AsRef, - Q: AsRef, - { - #[cfg(windows)] - if let Some(absolute_path) = self.pwd_per_drive.expand_pwd(path.as_ref()) { - return absolute_path; - } - - nu_path::expand_path_with::(path, relative_to, expand_tilde) - } } #[cfg(test)] diff --git a/src/run.rs b/src/run.rs index eb2a9b2e98..89bad3866f 100644 --- a/src/run.rs +++ b/src/run.rs @@ -12,30 +12,6 @@ use nu_protocol::{ }; use nu_utils::perf; -#[cfg(windows)] -fn init_pwd_per_drive(engine_state: &EngineState, stack: &mut Stack) { - use nu_path::DriveToPwdMap; - use std::path::Path; - - // Read environment for PWD-per-drive - for drive_letter in 'A'..='Z' { - let env_var = DriveToPwdMap::env_var_for_drive(drive_letter); - if let Some(env_pwd) = engine_state.get_env_var(&env_var) { - if let Ok(pwd_str) = nu_engine::env_to_string(&env_var, env_pwd, engine_state, stack) { - trace!("Get Env({}) {}", env_var, pwd_str); - let _ = stack.pwd_per_drive.set_pwd(Path::new(&pwd_str)); - stack.remove_env_var(engine_state, &env_var); - } - } - } - - if let Ok(abs_pwd) = engine_state.cwd(None) { - if let Some(abs_pwd_str) = abs_pwd.to_str() { - let _ = stack.pwd_per_drive.set_pwd(Path::new(abs_pwd_str)); - } - } -} - pub(crate) fn run_commands( engine_state: &mut EngineState, parsed_nu_cli_args: command::NushellCliArgs, @@ -50,8 +26,6 @@ pub(crate) fn run_commands( let create_scaffold = nu_path::nu_config_dir().map_or(false, |p| !p.exists()); let mut stack = Stack::new(); - #[cfg(windows)] - init_pwd_per_drive(engine_state, &mut stack); // if the --no-config-file(-n) option is NOT passed, load the plugin file, // load the default env file or custom (depending on parsed_nu_cli_args.env_file), @@ -141,8 +115,6 @@ pub(crate) fn run_file( ) { trace!("run_file"); let mut stack = Stack::new(); - #[cfg(windows)] - init_pwd_per_drive(engine_state, &mut stack); // if the --no-config-file(-n) option is NOT passed, load the plugin file, // load the default env file or custom (depending on parsed_nu_cli_args.env_file), @@ -210,9 +182,6 @@ pub(crate) fn run_repl( ) -> Result<(), miette::ErrReport> { trace!("run_repl"); let mut stack = Stack::new(); - #[cfg(windows)] - init_pwd_per_drive(engine_state, &mut stack); - let start_time = std::time::Instant::now(); if parsed_nu_cli_args.no_config_file.is_none() { From 5615d21ce90f24722698fe664fa11d54073052c8 Mon Sep 17 00:00:00 2001 From: Bahex Date: Tue, 17 Dec 2024 00:59:18 +0300 Subject: [PATCH 02/26] remove `content_type` metadata from pipeline after `from ...` commands (#14602) # Description `from ...` conversions pass along all metadata except `content_type`, which they set to `None`. ## Rationale `open`ing a file results in no `content_type` metadata if it can be parsed into a nu data structure, and using `open --raw` results in `content_type` metadata. `from ...` commands should preserve metadata ***except*** for `content_type`, as after parsing it's no longer that `content_type` and just structured nu data. These commands should return identical data *and* identical metadata ```nushell open foo.csv ``` ```nushell open foo.csv --raw | from csv ``` # User-Facing Changes N/A # Tests + Formatting - :green_circle: toolkit fmt - :green_circle: toolkit clippy - :green_circle: toolkit test - :green_circle: toolkit test stdlib # After Submitting N/A --- crates/nu-command/src/formats/from/csv.rs | 33 +++++++++++ .../nu-command/src/formats/from/delimited.rs | 5 +- crates/nu-command/src/formats/from/json.rs | 58 ++++++++++++++----- crates/nu-command/src/formats/from/msgpack.rs | 40 ++++++++++++- .../nu-command/src/formats/from/msgpackz.rs | 6 +- crates/nu-command/src/formats/from/nuon.rs | 36 +++++++++++- crates/nu-command/src/formats/from/ods.rs | 3 +- crates/nu-command/src/formats/from/toml.rs | 35 ++++++++++- crates/nu-command/src/formats/from/tsv.rs | 33 +++++++++++ crates/nu-command/src/formats/from/xlsx.rs | 3 +- crates/nu-command/src/formats/from/xml.rs | 40 ++++++++++++- crates/nu-command/src/formats/from/yaml.rs | 36 +++++++++++- 12 files changed, 303 insertions(+), 25 deletions(-) diff --git a/crates/nu-command/src/formats/from/csv.rs b/crates/nu-command/src/formats/from/csv.rs index bde84c2c73..74b9bcd33f 100644 --- a/crates/nu-command/src/formats/from/csv.rs +++ b/crates/nu-command/src/formats/from/csv.rs @@ -204,12 +204,45 @@ fn from_csv( #[cfg(test)] mod test { + use nu_cmd_lang::eval_pipeline_without_terminal_expression; + use super::*; + use crate::{Metadata, MetadataSet}; + #[test] fn test_examples() { use crate::test_examples; test_examples(FromCsv {}) } + + #[test] + fn test_content_type_metadata() { + let mut engine_state = Box::new(EngineState::new()); + let delta = { + let mut working_set = StateWorkingSet::new(&engine_state); + + working_set.add_decl(Box::new(FromCsv {})); + working_set.add_decl(Box::new(Metadata {})); + working_set.add_decl(Box::new(MetadataSet {})); + + working_set.render() + }; + + engine_state + .merge_delta(delta) + .expect("Error merging delta"); + + let cmd = r#""a,b\n1,2" | metadata set --content-type 'text/csv' --datasource-ls | from csv | metadata | $in"#; + let result = eval_pipeline_without_terminal_expression( + cmd, + std::env::temp_dir().as_ref(), + &mut engine_state, + ); + assert_eq!( + Value::test_record(record!("source" => Value::test_string("ls"))), + result.expect("There should be a result") + ) + } } diff --git a/crates/nu-command/src/formats/from/delimited.rs b/crates/nu-command/src/formats/from/delimited.rs index 5dfdd4ad82..865bc79a41 100644 --- a/crates/nu-command/src/formats/from/delimited.rs +++ b/crates/nu-command/src/formats/from/delimited.rs @@ -93,9 +93,10 @@ pub(super) fn from_delimited_data( input: PipelineData, name: Span, ) -> Result { + let metadata = input.metadata().map(|md| md.with_content_type(None)); match input { PipelineData::Empty => Ok(PipelineData::Empty), - PipelineData::Value(value, metadata) => { + PipelineData::Value(value, ..) => { let string = value.into_string()?; let byte_stream = ByteStream::read_string(string, name, Signals::empty()); Ok(PipelineData::ListStream( @@ -109,7 +110,7 @@ pub(super) fn from_delimited_data( dst_span: name, src_span: list_stream.span(), }), - PipelineData::ByteStream(byte_stream, metadata) => Ok(PipelineData::ListStream( + PipelineData::ByteStream(byte_stream, ..) => Ok(PipelineData::ListStream( from_delimited_stream(config, byte_stream, name)?, metadata, )), diff --git a/crates/nu-command/src/formats/from/json.rs b/crates/nu-command/src/formats/from/json.rs index e3de2cbafd..36a05ea4e1 100644 --- a/crates/nu-command/src/formats/from/json.rs +++ b/crates/nu-command/src/formats/from/json.rs @@ -70,23 +70,22 @@ impl Command for FromJson { let span = call.head; let strict = call.has_flag(engine_state, stack, "strict")?; + let metadata = input.metadata().map(|md| md.with_content_type(None)); // TODO: turn this into a structured underline of the nu_json error if call.has_flag(engine_state, stack, "objects")? { // Return a stream of JSON values, one for each non-empty line match input { - PipelineData::Value(Value::String { val, .. }, metadata) => { - Ok(PipelineData::ListStream( - read_json_lines( - Cursor::new(val), - span, - strict, - engine_state.signals().clone(), - ), - metadata, - )) - } - PipelineData::ByteStream(stream, metadata) + PipelineData::Value(Value::String { val, .. }, ..) => Ok(PipelineData::ListStream( + read_json_lines( + Cursor::new(val), + span, + strict, + engine_state.signals().clone(), + ), + metadata, + )), + PipelineData::ByteStream(stream, ..) if stream.type_() != ByteStreamType::Binary => { if let Some(reader) = stream.reader() { @@ -107,7 +106,7 @@ impl Command for FromJson { } } else { // Return a single JSON value - let (string_input, span, metadata) = input.collect_string_strict(span)?; + let (string_input, span, ..) = input.collect_string_strict(span)?; if string_input.is_empty() { return Ok(Value::nothing(span).into_pipeline_data()); @@ -267,6 +266,10 @@ fn convert_string_to_value_strict(string_input: &str, span: Span) -> Result Value::test_string("ls"))), + result.expect("There should be a result") + ) + } } diff --git a/crates/nu-command/src/formats/from/msgpack.rs b/crates/nu-command/src/formats/from/msgpack.rs index a3538ea69f..4fe849b8fe 100644 --- a/crates/nu-command/src/formats/from/msgpack.rs +++ b/crates/nu-command/src/formats/from/msgpack.rs @@ -113,7 +113,8 @@ MessagePack: https://msgpack.org/ objects, signals: engine_state.signals().clone(), }; - match input { + let metadata = input.metadata().map(|md| md.with_content_type(None)); + let out = match input { // Deserialize from a byte buffer PipelineData::Value(Value::Binary { val: bytes, .. }, _) => { read_msgpack(Cursor::new(bytes), opts) @@ -136,7 +137,8 @@ MessagePack: https://msgpack.org/ dst_span: call.head, src_span: input.span().unwrap_or(call.head), }), - } + }; + out.map(|pd| pd.set_metadata(metadata)) } } @@ -510,6 +512,10 @@ fn assert_eof(input: &mut impl io::Read, span: Span) -> Result<(), ShellError> { #[cfg(test)] mod test { + use nu_cmd_lang::eval_pipeline_without_terminal_expression; + + use crate::{Metadata, MetadataSet, ToMsgpack}; + use super::*; #[test] @@ -518,4 +524,34 @@ mod test { test_examples(FromMsgpack {}) } + + #[test] + fn test_content_type_metadata() { + let mut engine_state = Box::new(EngineState::new()); + let delta = { + let mut working_set = StateWorkingSet::new(&engine_state); + + working_set.add_decl(Box::new(ToMsgpack {})); + working_set.add_decl(Box::new(FromMsgpack {})); + working_set.add_decl(Box::new(Metadata {})); + working_set.add_decl(Box::new(MetadataSet {})); + + working_set.render() + }; + + engine_state + .merge_delta(delta) + .expect("Error merging delta"); + + let cmd = r#"{a: 1 b: 2} | to msgpack | metadata set --datasource-ls | from msgpack | metadata | $in"#; + let result = eval_pipeline_without_terminal_expression( + cmd, + std::env::temp_dir().as_ref(), + &mut engine_state, + ); + assert_eq!( + Value::test_record(record!("source" => Value::test_string("ls"))), + result.expect("There should be a result") + ) + } } diff --git a/crates/nu-command/src/formats/from/msgpackz.rs b/crates/nu-command/src/formats/from/msgpackz.rs index 81cc901614..4f31f74ec6 100644 --- a/crates/nu-command/src/formats/from/msgpackz.rs +++ b/crates/nu-command/src/formats/from/msgpackz.rs @@ -43,7 +43,8 @@ impl Command for FromMsgpackz { objects, signals: engine_state.signals().clone(), }; - match input { + let metadata = input.metadata().map(|md| md.with_content_type(None)); + let out = match input { // Deserialize from a byte buffer PipelineData::Value(Value::Binary { val: bytes, .. }, _) => { let reader = brotli::Decompressor::new(Cursor::new(bytes), BUFFER_SIZE); @@ -68,6 +69,7 @@ impl Command for FromMsgpackz { dst_span: call.head, src_span: span, }), - } + }; + out.map(|pd| pd.set_metadata(metadata)) } } diff --git a/crates/nu-command/src/formats/from/nuon.rs b/crates/nu-command/src/formats/from/nuon.rs index 107ee9c3b4..7cb45c5721 100644 --- a/crates/nu-command/src/formats/from/nuon.rs +++ b/crates/nu-command/src/formats/from/nuon.rs @@ -49,7 +49,8 @@ impl Command for FromNuon { let (string_input, _span, metadata) = input.collect_string_strict(head)?; match nuon::from_nuon(&string_input, Some(head)) { - Ok(result) => Ok(result.into_pipeline_data_with_metadata(metadata)), + Ok(result) => Ok(result + .into_pipeline_data_with_metadata(metadata.map(|md| md.with_content_type(None)))), Err(err) => Err(ShellError::GenericError { error: "error when loading nuon text".into(), msg: "could not load nuon text".into(), @@ -63,6 +64,10 @@ impl Command for FromNuon { #[cfg(test)] mod test { + use nu_cmd_lang::eval_pipeline_without_terminal_expression; + + use crate::{Metadata, MetadataSet}; + use super::*; #[test] @@ -71,4 +76,33 @@ mod test { test_examples(FromNuon {}) } + + #[test] + fn test_content_type_metadata() { + let mut engine_state = Box::new(EngineState::new()); + let delta = { + let mut working_set = StateWorkingSet::new(&engine_state); + + working_set.add_decl(Box::new(FromNuon {})); + working_set.add_decl(Box::new(Metadata {})); + working_set.add_decl(Box::new(MetadataSet {})); + + working_set.render() + }; + + engine_state + .merge_delta(delta) + .expect("Error merging delta"); + + let cmd = r#"'[[a, b]; [1, 2]]' | metadata set --content-type 'application/x-nuon' --datasource-ls | from nuon | metadata | $in"#; + let result = eval_pipeline_without_terminal_expression( + cmd, + std::env::temp_dir().as_ref(), + &mut engine_state, + ); + assert_eq!( + Value::test_record(record!("source" => Value::test_string("ls"))), + result.expect("There should be a result") + ) + } } diff --git a/crates/nu-command/src/formats/from/ods.rs b/crates/nu-command/src/formats/from/ods.rs index c6f08f4481..f1f4252f1e 100644 --- a/crates/nu-command/src/formats/from/ods.rs +++ b/crates/nu-command/src/formats/from/ods.rs @@ -46,7 +46,8 @@ impl Command for FromOds { vec![] }; - from_ods(input, head, sel_sheets) + let metadata = input.metadata().map(|md| md.with_content_type(None)); + from_ods(input, head, sel_sheets).map(|pd| pd.set_metadata(metadata)) } fn examples(&self) -> Vec { diff --git a/crates/nu-command/src/formats/from/toml.rs b/crates/nu-command/src/formats/from/toml.rs index a61ced65a0..8f4242e5db 100644 --- a/crates/nu-command/src/formats/from/toml.rs +++ b/crates/nu-command/src/formats/from/toml.rs @@ -29,7 +29,8 @@ impl Command for FromToml { let span = call.head; let (mut string_input, span, metadata) = input.collect_string_strict(span)?; string_input.push('\n'); - Ok(convert_string_to_value(string_input, span)?.into_pipeline_data_with_metadata(metadata)) + Ok(convert_string_to_value(string_input, span)? + .into_pipeline_data_with_metadata(metadata.map(|md| md.with_content_type(None)))) } fn examples(&self) -> Vec { @@ -144,8 +145,11 @@ pub fn convert_string_to_value(string_input: String, span: Span) -> Result Value::test_string("ls"))), + result.expect("There should be a result") + ) + } } diff --git a/crates/nu-command/src/formats/from/tsv.rs b/crates/nu-command/src/formats/from/tsv.rs index 09bee4803f..2d77342307 100644 --- a/crates/nu-command/src/formats/from/tsv.rs +++ b/crates/nu-command/src/formats/from/tsv.rs @@ -165,6 +165,10 @@ fn from_tsv( #[cfg(test)] mod test { + use nu_cmd_lang::eval_pipeline_without_terminal_expression; + + use crate::{Metadata, MetadataSet}; + use super::*; #[test] @@ -173,4 +177,33 @@ mod test { test_examples(FromTsv {}) } + + #[test] + fn test_content_type_metadata() { + let mut engine_state = Box::new(EngineState::new()); + let delta = { + let mut working_set = StateWorkingSet::new(&engine_state); + + working_set.add_decl(Box::new(FromTsv {})); + working_set.add_decl(Box::new(Metadata {})); + working_set.add_decl(Box::new(MetadataSet {})); + + working_set.render() + }; + + engine_state + .merge_delta(delta) + .expect("Error merging delta"); + + let cmd = r#""a\tb\n1\t2" | metadata set --content-type 'text/tab-separated-values' --datasource-ls | from tsv | metadata | $in"#; + let result = eval_pipeline_without_terminal_expression( + cmd, + std::env::temp_dir().as_ref(), + &mut engine_state, + ); + assert_eq!( + Value::test_record(record!("source" => Value::test_string("ls"))), + result.expect("There should be a result") + ) + } } diff --git a/crates/nu-command/src/formats/from/xlsx.rs b/crates/nu-command/src/formats/from/xlsx.rs index 1e02cf432c..bc486d1f18 100644 --- a/crates/nu-command/src/formats/from/xlsx.rs +++ b/crates/nu-command/src/formats/from/xlsx.rs @@ -47,7 +47,8 @@ impl Command for FromXlsx { vec![] }; - from_xlsx(input, head, sel_sheets) + let metadata = input.metadata().map(|md| md.with_content_type(None)); + from_xlsx(input, head, sel_sheets).map(|pd| pd.set_metadata(metadata)) } fn examples(&self) -> Vec { diff --git a/crates/nu-command/src/formats/from/xml.rs b/crates/nu-command/src/formats/from/xml.rs index 4c0675a109..70c1048972 100644 --- a/crates/nu-command/src/formats/from/xml.rs +++ b/crates/nu-command/src/formats/from/xml.rs @@ -206,7 +206,9 @@ fn from_xml(input: PipelineData, info: &ParsingInfo) -> Result Ok(x.into_pipeline_data_with_metadata(metadata)), + Ok(x) => { + Ok(x.into_pipeline_data_with_metadata(metadata.map(|md| md.with_content_type(None)))) + } Err(err) => Err(process_xml_parse_error(err, span)), } } @@ -322,10 +324,14 @@ fn make_cant_convert_error(help: impl Into, span: Span) -> ShellError { #[cfg(test)] mod tests { + use crate::Metadata; + use crate::MetadataSet; + use super::*; use indexmap::indexmap; use indexmap::IndexMap; + use nu_cmd_lang::eval_pipeline_without_terminal_expression; fn string(input: impl Into) -> Value { Value::test_string(input) @@ -480,4 +486,36 @@ mod tests { test_examples(FromXml {}) } + + #[test] + fn test_content_type_metadata() { + let mut engine_state = Box::new(EngineState::new()); + let delta = { + let mut working_set = StateWorkingSet::new(&engine_state); + + working_set.add_decl(Box::new(FromXml {})); + working_set.add_decl(Box::new(Metadata {})); + working_set.add_decl(Box::new(MetadataSet {})); + + working_set.render() + }; + + engine_state + .merge_delta(delta) + .expect("Error merging delta"); + + let cmd = r#"' + + Event +' | metadata set --content-type 'application/xml' --datasource-ls | from xml | metadata | $in"#; + let result = eval_pipeline_without_terminal_expression( + cmd, + std::env::temp_dir().as_ref(), + &mut engine_state, + ); + assert_eq!( + Value::test_record(record!("source" => Value::test_string("ls"))), + result.expect("There should be a result") + ) + } } diff --git a/crates/nu-command/src/formats/from/yaml.rs b/crates/nu-command/src/formats/from/yaml.rs index 5c463cc2a9..649f7e83ac 100644 --- a/crates/nu-command/src/formats/from/yaml.rs +++ b/crates/nu-command/src/formats/from/yaml.rs @@ -235,14 +235,19 @@ fn from_yaml(input: PipelineData, head: Span) -> Result Ok(x.into_pipeline_data_with_metadata(metadata)), + Ok(x) => { + Ok(x.into_pipeline_data_with_metadata(metadata.map(|md| md.with_content_type(None)))) + } Err(other) => Err(other), } } #[cfg(test)] mod test { + use crate::{Metadata, MetadataSet}; + use super::*; + use nu_cmd_lang::eval_pipeline_without_terminal_expression; use nu_protocol::Config; #[test] @@ -395,4 +400,33 @@ mod test { assert!(result.ok().unwrap() == test_case.expected.ok().unwrap()); } } + + #[test] + fn test_content_type_metadata() { + let mut engine_state = Box::new(EngineState::new()); + let delta = { + let mut working_set = StateWorkingSet::new(&engine_state); + + working_set.add_decl(Box::new(FromYaml {})); + working_set.add_decl(Box::new(Metadata {})); + working_set.add_decl(Box::new(MetadataSet {})); + + working_set.render() + }; + + engine_state + .merge_delta(delta) + .expect("Error merging delta"); + + let cmd = r#""a: 1\nb: 2" | metadata set --content-type 'application/yaml' --datasource-ls | from yaml | metadata | $in"#; + let result = eval_pipeline_without_terminal_expression( + cmd, + std::env::temp_dir().as_ref(), + &mut engine_state, + ); + assert_eq!( + Value::test_record(record!("source" => Value::test_string("ls"))), + result.expect("There should be a result") + ) + } } From 6367fb6e9eb9af58e794e95f6c73854b00144bf8 Mon Sep 17 00:00:00 2001 From: Douglas <32344964+NotTheDr01ds@users.noreply.github.com> Date: Mon, 16 Dec 2024 19:20:54 -0500 Subject: [PATCH 03/26] Add missing color_config settings (#14603) # Description Fixes #14600 by adding a default value for missing keys in `default_config.nu`: * `$env.config.color_config.glob` * `$env.config.color_config.closure` # User-Facing Changes Will no longer error when accessing these keys. # Tests + Formatting - :green_circle: `toolkit fmt` - :green_circle: `toolkit clippy` - :green_circle: `toolkit test` - :green_circle: `toolkit test stdlib` # After Submitting N/A --- crates/nu-utils/src/default_files/default_config.nu | 2 ++ 1 file changed, 2 insertions(+) diff --git a/crates/nu-utils/src/default_files/default_config.nu b/crates/nu-utils/src/default_files/default_config.nu index dcefb70fff..ce60d29663 100644 --- a/crates/nu-utils/src/default_files/default_config.nu +++ b/crates/nu-utils/src/default_files/default_config.nu @@ -20,6 +20,8 @@ $env.config.color_config = { row_index: green_bold record: white list: white + closure: green_bold + glob:cyan_bold block: white hints: dark_gray search_result: { bg: red fg: white } From d94b344342d055434868dca8860fc0a9d1bbdbfb Mon Sep 17 00:00:00 2001 From: Darren Schroeder <343840+fdncred@users.noreply.github.com> Date: Tue, 17 Dec 2024 06:26:56 -0600 Subject: [PATCH 04/26] =?UTF-8?q?Revert=20"For=20`#`=20to=20start=20a=20co?= =?UTF-8?q?mment,=20then=20it=20either=20need=20to=20be=20the=20first=20ch?= =?UTF-8?q?ara=E2=80=A6"=20(#14606)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reverts nushell/nushell#14562 due to https://github.com/nushell/nushell/issues/14605 --- crates/nu-parser/src/lex.rs | 12 ++++----- crates/nu-parser/tests/test_lex.rs | 23 ----------------- tests/repl/test_parser.rs | 40 ------------------------------ 3 files changed, 6 insertions(+), 69 deletions(-) diff --git a/crates/nu-parser/src/lex.rs b/crates/nu-parser/src/lex.rs index 6d1adf28ef..f0802fcd7a 100644 --- a/crates/nu-parser/src/lex.rs +++ b/crates/nu-parser/src/lex.rs @@ -51,7 +51,7 @@ impl BlockKind { } // A baseline token is terminated if it's not nested inside of a paired -// delimiter and the next character is one of: `|`, `;` or any +// delimiter and the next character is one of: `|`, `;`, `#` or any // whitespace. fn is_item_terminator( block_level: &[BlockKind], @@ -115,7 +115,6 @@ pub fn lex_item( // character (whitespace, `|`, `;` or `#`) is encountered, the baseline // token is done. // - Otherwise, accumulate the character into the current baseline token. - let mut previous_char = None; while let Some(c) = input.get(*curr_offset) { let c = *c; @@ -148,9 +147,11 @@ pub fn lex_item( // Also need to check to make sure we aren't escaped quote_start = None; } - } else if c == b'#' && !in_comment { - // To start a comment, It either need to be the first character of the token or prefixed with space. - in_comment = previous_char.map(|pc| pc == b' ').unwrap_or(true); + } else if c == b'#' { + if is_item_terminator(&block_level, c, additional_whitespace, special_tokens) { + break; + } + in_comment = true; } else if c == b'\n' || c == b'\r' { in_comment = false; if is_item_terminator(&block_level, c, additional_whitespace, special_tokens) { @@ -253,7 +254,6 @@ pub fn lex_item( } *curr_offset += 1; - previous_char = Some(c); } let span = Span::new(span_offset + token_start, span_offset + *curr_offset); diff --git a/crates/nu-parser/tests/test_lex.rs b/crates/nu-parser/tests/test_lex.rs index 54ff674bb9..a14843f3f0 100644 --- a/crates/nu-parser/tests/test_lex.rs +++ b/crates/nu-parser/tests/test_lex.rs @@ -159,29 +159,6 @@ fn lex_comment() { ); } -#[test] -fn lex_not_comment_needs_space_in_front_of_hashtag() { - let file = b"1..10 | each {echo test#testing }"; - - let output = lex(file, 0, &[], &[], false); - - assert!(output.1.is_none()); -} - -#[test] -fn lex_comment_with_space_in_front_of_hashtag() { - let file = b"1..10 | each {echo test #testing }"; - - let output = lex(file, 0, &[], &[], false); - - assert!(output.1.is_some()); - assert!(matches!( - output.1.unwrap(), - ParseError::UnexpectedEof(missing_token, span) if missing_token == "}" - && span == Span::new(33, 34) - )); -} - #[test] fn lex_is_incomplete() { let file = b"let x = 300 | ;"; diff --git a/tests/repl/test_parser.rs b/tests/repl/test_parser.rs index ddc790324a..f77d431110 100644 --- a/tests/repl/test_parser.rs +++ b/tests/repl/test_parser.rs @@ -169,41 +169,6 @@ fn comment_skipping_in_pipeline_3() -> TestResult { ) } -#[test] -fn still_string_if_hashtag_is_middle_of_string() -> TestResult { - run_test(r#"echo test#testing"#, "test#testing") -} - -#[test] -fn non_comment_hashtag_in_comment_does_not_stop_comment() -> TestResult { - run_test(r#"# command_bar_text: { fg: '#C4C9C6' },"#, "") -} - -#[test] -fn non_comment_hashtag_in_comment_does_not_stop_comment_in_block() -> TestResult { - run_test( - r#"{ - explore: { - # command_bar_text: { fg: '#C4C9C6' }, - } - } | get explore | is-empty"#, - "true", - ) -} - -#[test] -fn still_string_if_hashtag_is_middle_of_string_inside_each() -> TestResult { - run_test( - r#"1..1 | each {echo test#testing } | get 0"#, - "test#testing", - ) -} - -#[test] -fn still_string_if_hashtag_is_middle_of_string_inside_each_also_with_dot() -> TestResult { - run_test(r#"1..1 | each {echo '.#testing' } | get 0"#, ".#testing") -} - #[test] fn bad_var_name() -> TestResult { fail_test(r#"let $"foo bar" = 4"#, "can't contain") @@ -317,11 +282,6 @@ fn raw_string_with_equals() -> TestResult { ) } -#[test] -fn raw_string_with_hashtag() -> TestResult { - run_test(r#"r##' one # two '##"#, "one # two") -} - #[test] fn list_quotes_with_equals() -> TestResult { run_test( From c266e6adaf63bbd72f16a8ba6df80e6cb0d6e3d1 Mon Sep 17 00:00:00 2001 From: Bahex Date: Tue, 17 Dec 2024 19:01:23 +0300 Subject: [PATCH 05/26] test(path self): Add tests (#14607) # Description Add tests for `path self`. I wasn't very familiar with the code base, especially the testing utilities, when I first implemented `path self`. It's been on my mind to add tests for it since then. --- crates/nu-command/tests/commands/path/mod.rs | 1 + .../nu-command/tests/commands/path/self_.rs | 64 +++++++++++++++++++ 2 files changed, 65 insertions(+) create mode 100644 crates/nu-command/tests/commands/path/self_.rs diff --git a/crates/nu-command/tests/commands/path/mod.rs b/crates/nu-command/tests/commands/path/mod.rs index d14cbde181..ada11426e8 100644 --- a/crates/nu-command/tests/commands/path/mod.rs +++ b/crates/nu-command/tests/commands/path/mod.rs @@ -4,6 +4,7 @@ mod exists; mod expand; mod join; mod parse; +mod self_; mod split; mod type_; diff --git a/crates/nu-command/tests/commands/path/self_.rs b/crates/nu-command/tests/commands/path/self_.rs new file mode 100644 index 0000000000..b0c47195c8 --- /dev/null +++ b/crates/nu-command/tests/commands/path/self_.rs @@ -0,0 +1,64 @@ +use std::path::Path; + +use itertools::Itertools; +use nu_test_support::{fs::Stub, nu, playground::Playground}; + +#[test] +fn self_path_const() { + Playground::setup("path_self_const", |dirs, sandbox| { + sandbox + .within("scripts") + .with_files(&[Stub::FileWithContentToBeTrimmed( + "foo.nu", + r#" + export const paths = { + self: (path self), + dir: (path self .), + sibling: (path self sibling), + parent_dir: (path self ..), + cousin: (path self ../cousin), + } + "#, + )]); + + let actual = nu!(cwd: dirs.test(), r#"use scripts/foo.nu; $foo.paths | values | str join (char nul)"#); + let (self_, dir, sibling, parent_dir, cousin) = actual + .out + .split("\0") + .collect_tuple() + .expect("should have 5 NUL separated paths"); + + let mut pathbuf = dirs.test().to_path_buf(); + + pathbuf.push("scripts"); + assert_eq!(pathbuf, Path::new(dir)); + + pathbuf.push("foo.nu"); + assert_eq!(pathbuf, Path::new(self_)); + + pathbuf.pop(); + pathbuf.push("sibling"); + assert_eq!(pathbuf, Path::new(sibling)); + + pathbuf.pop(); + pathbuf.pop(); + assert_eq!(pathbuf, Path::new(parent_dir)); + + pathbuf.push("cousin"); + assert_eq!(pathbuf, Path::new(cousin)); + }) +} + +#[test] +fn self_path_runtime() { + let actual = nu!("path self"); + assert!(!actual.status.success()); + assert!(actual.err.contains("can only run during parse-time")); +} + +#[test] +fn self_path_repl() { + let actual = nu!("const foo = path self; $foo"); + assert!(!actual.status.success()); + assert!(actual.err.contains("nu::shell::file_not_found")); +} From cc4da104e0e3994c79d0946b08b4e901b9697ba5 Mon Sep 17 00:00:00 2001 From: Perchun Pak Date: Tue, 17 Dec 2024 17:43:25 +0100 Subject: [PATCH 06/26] Fix issues in the example configs (#14601) For some reason, it had multiple syntax errors and other issues, like undefined options. Would be great to add a test for sourcing all example configs, but I don't know rust See also https://github.com/nushell/nushell/pull/14249#discussion_r1887192408 CC @NotTheDr01ds --- .../nu-utils/src/default_files/default_env.nu | 4 +- .../src/default_files/sample_config.nu | 109 +++++++++--------- .../nu-utils/src/default_files/sample_env.nu | 4 +- .../src/default_files/sample_login.nu | 3 + .../src/default_files/scaffold_config.nu | 5 +- .../src/default_files/scaffold_env.nu | 3 + 6 files changed, 72 insertions(+), 56 deletions(-) diff --git a/crates/nu-utils/src/default_files/default_env.nu b/crates/nu-utils/src/default_files/default_env.nu index ae2f1ae719..39dcd5c9d0 100644 --- a/crates/nu-utils/src/default_files/default_env.nu +++ b/crates/nu-utils/src/default_files/default_env.nu @@ -3,7 +3,7 @@ # # version = "0.100.1" -$env.PROMPT_COMMAND = $env.PROMPT_COMMAND? | default {|| +$env.PROMPT_COMMAND = $env.PROMPT_COMMAND? | default {|| let dir = match (do -i { $env.PWD | path relative-to $nu.home-path }) { null => $env.PWD '' => '~' @@ -17,7 +17,7 @@ $env.PROMPT_COMMAND = $env.PROMPT_COMMAND? | default {|| $path_segment | str replace --all (char path_sep) $"($separator_color)(char path_sep)($path_color)" } -$env.PROMPT_COMMAND_RIGHT = $env.PROMPT_COMMAND_RIGHT? | default {|| +$env.PROMPT_COMMAND_RIGHT = $env.PROMPT_COMMAND_RIGHT? | default {|| # create a right prompt in magenta with green separators and am/pm underlined let time_segment = ([ (ansi reset) diff --git a/crates/nu-utils/src/default_files/sample_config.nu b/crates/nu-utils/src/default_files/sample_config.nu index ceb65835cd..1a91b59f5e 100644 --- a/crates/nu-utils/src/default_files/sample_config.nu +++ b/crates/nu-utils/src/default_files/sample_config.nu @@ -1,6 +1,9 @@ -# Nushell Config File +# Nushell Sample Config File # -# version = "0.99.2" +# Warning: This file is intended for documentation purposes only and +# is not intended to be used as an actual configuration file as-is. +# +# version = "0.100.1" # # A `config.nu` file is used to override default Nushell settings, # define (or import) custom commands, or run any other startup tasks. @@ -86,14 +89,14 @@ $env.config.recursion_limit = 50 # --------------------------- # edit_mode (string) "vi" or "emacs" sets the editing behavior of Reedline -edit_mode: "emacs" +$env.config.edit_mode = "emacs" # Command that will be used to edit the current line buffer with Ctrl+O. # If unset, uses $env.VISUAL and then $env.EDITOR # # Tip: Set to "editor" to use the default editor on Unix platforms using # the Alternatives system or equivalent -buffer_editor: "editor" +$env.config.buffer_editor = "editor" # cursor_shape_* (string) # ----------------------- @@ -120,7 +123,7 @@ $env.config.completions.algorithm = "prefix" $env.config.completions.sort = "smart" # case_sensitive (bool): true/false to enable/disable case-sensitive completions -$env.config.completions.case_sensitive = false +$env.config.completions.case_sensitive = false # quick (bool): # true: auto-select the completion when only one remains @@ -132,7 +135,7 @@ $env.config.completions.quick = true # false: Do not partially complete # Partial Example: If a directory contains only files named "forage", "food", and "forest", # then typing "ls " and pressing will partially complete the first two -# letters, "f" and "o". If the directory also includes a file named "faster", +# letters, "f" and "o". If the directory also includes a file named "faster", # then only "f" would be partially completed. $env.config.completions.partial = true @@ -145,7 +148,7 @@ $env.config.completions.use_ls_colors = true # completions.external.*: Settings related to completing external commands # and additional completers -# external.exnable (bool) +# external.enable (bool) # true: search for external commands on the Path # false: disabling might be desired for performance if your path includes # directories on a slower filesystem @@ -156,16 +159,16 @@ $env.config.completions.external.enable = true $env.config.completions.external.max_results = 50 # completer (closure with a |spans| parameter): A command to call for *argument* completions -# to commands (internal or external). +# to commands (internal or external). # # The |spans| parameter is a list of strings representing the tokens (spans) -# on the current commandline. It is always a list of at least two strings - The +# on the current commandline. It is always a list of at least two strings - The # command being completed plus the first argument of that command ("" if no argument has # been partially typed yet), and additional strings for additional arguments beyond # the first. # # This setting is usually set to a closure which will call a third-party completion system, such -# as Carapace. +# as Carapace. # # Note: The following is an over-simplified completer command that will call Carapace if it # is installed. Please use the official Carapace completer, which can be generated automatically @@ -206,8 +209,8 @@ $env.config.shell_integration.osc9_9 = false # osc8 (bool): # When true, the `ls` command will generate clickable links that can be launched in another # application by the terminal. -# Note: This setting replaces the now deprecated `ls.show_clickable_links` -$env.config.shell.integration.osc8: true +# Note: This setting replaces the now deprecated `ls.clickable_links` +$env.config.shell_integration.osc8 = true # Deprecated # $env.config.ls.clickable_links = true @@ -229,13 +232,13 @@ $env.config.shell_integration.osc633 = true # reset_application_mode (bool): # true/false to enable/disable sending ESC[?1l to the terminal -# This sequence is commonly used to keep cursor key modes in sync between the local +# This sequence is commonly used to keep cursor key modes in sync between the local # terminal and a remove SSH host. $env.config.shell_integration.reset_application_mode = true # bracketed_paste (bool): # true/false to enable/disable the bracketed-paste feature, which allows multiple-lines -# to be pasted into Nushell at once without immediate execution. When disabled, +# to be pasted into Nushell at once without immediate execution. When disabled, # each pasted line is executed as it is received. # Note that bracketed paste is not currently supported on the Windows version of # Nushell. @@ -266,7 +269,7 @@ $env.config.display_errors.exit_code = false # display_errors.termination_signal (bool): # true/false to enable/disable displaying a Nushell error when a child process is -# terminated via any signal +# terminated via any signal $env.config.display_errors.termination_signal = true # ------------- @@ -282,7 +285,7 @@ $env.config.display_errors.termination_signal = true $env.config.footer_mode = 25 # table.* -# table_mode (string): +# table_mode (string): # One of: "default", "basic", "compact", "compact_double", "heavy", "light", "none", "reinforced", # "rounded", "thin", "with_love", "psql", "markdown", "dots", "restructured", "ascii_rounded", # or "basic_compact" @@ -344,7 +347,7 @@ $env.config.table.footer_inheritance = false # Datetime Display # ---------------- # datetime_format.* (string or nothing): -# Format strings that will be used for datetime values. +# Format strings that will be used for datetime values. # When set to `null`, the default behavior is to "humanize" the value (e.g., "now" or "a day ago") # datetime_format.table (string or nothing): @@ -389,7 +392,7 @@ $env.config.float_precision = 2 # ls.use_ls_colors (bool): # true: The `ls` command will apply the $env.LS_COLORS standard to filenames # false: Filenames in the `ls` table will use the color_config for strings -$env.config.ls = true +$env.config.ls.use_ls_colors = true # Hooks # ----- @@ -402,12 +405,19 @@ $env.config.ls = true # WARNING: A malformed display_output hook can suppress all Nushell output to the terminal. # It can be reset by assigning an empty string as below: -$env.config.hooks.pre_prompt = [] # Before each prompt is displayed -$env.config.hooks.pre_execution = [] # After is pressed; before the commandline - # is executed -$env.config.hooks.env_change = [] # When a specified environment variable changes -$env.config.hooks.display_output = "" # Before Nushell output is displayed in the terminal -$env.config.hooks.command_not_found = [] # When a command is not found +# Before each prompt is displayed +$env.config.hooks.pre_prompt = [] +# After is pressed; before the commandline is executed +$env.config.hooks.pre_execution = [] +# When a specified environment variable changes +$env.config.hooks.env_change = { + # run if the PWD environment is different since the last repl input + PWD: [{|before, after| null }] +} +# Before Nushell output is displayed in the terminal +$env.config.hooks.display_output = "if (term size).columns >= 100 { table -e } else { table }" +# When a command is not found +$env.config.hooks.command_not_found = [] # ----------- # Keybindings @@ -462,7 +472,7 @@ $env.config.menus ++= [ type: { layout: description columns: 4 - col_width: 20 # Optional value. If missing all the screen width is used to calculate column width + col_width: 20 # Optional value. If missing all the screen width is used to calculate column width col_padding: 2 selection_rows: 4 description_rows: 10 @@ -480,27 +490,22 @@ $env.config.menus ++= [ # Plugin behavior # --------------- # Per-plugin configuration. See https://www.nushell.sh/contributor-book/plugins.html#configuration. -plugins: {} $env.config.plugins + +# Configuration for plugin garbage collection $env.config.plugin_gc $env.config.plugin_gc.default +# true to enable stopping of inactive plugins $env.config.plugin_gc.default.enabled +# How long to wait after a plugin is inactive before stopping it $env.config.plugin_gc.default.stop_after -$env.config.plugin_gc.plugins - plugin_gc: { - # Configuration for plugin garbage collection - default: { - enabled: true # true to enable stopping of inactive plugins - stop_after: 10sec # how long to wait after a plugin is inactive to stop it - } - plugins: { - # alternate configuration for specific plugins, by name, for example: - # - # gstat: { - # enabled: false - # } - } - } +$env.config.plugin_gc.plugins = { + # Alternate configuration for specific plugins, by name, for example: + # + # gstat: { + # enabled: false + # } +} # ------------------------------------- @@ -532,12 +537,12 @@ use std/config dark-theme $env.config.color_config = (dark-theme) # Or, individual color settings can be configured or overridden. -# +# # Values can be one of: # - A color name such as "red" (see `ansi -l` for a list) # - A color RGB value in the form of "#C4C9C6" # - A record including: -# * `fg` (color) +# * `fg` (color) # * `bg` (color) # * `attr`: a string with one or more of: # - 'n': normal @@ -547,7 +552,7 @@ $env.config.color_config = (dark-theme) # - 'i': italics # - 'd': dimmed -# foreground, background, and cursor colors are not handled by Nushell, but can be used by +# foreground, background, and cursor colors are not handled by Nushell, but can be used by # custom-commands such as `theme` from the nu_scripts repository. That `theme` command can be # used to set the terminal foreground, background, and cursor colors. $env.config.color_config.foreground @@ -557,7 +562,7 @@ $env.config.color_config.cursor # ------------------------------------------------------------------------------------------------- # shape_: Applies syntax highlighting based on the "shape" (inferred or declared type) of an # element on the commandline. Nushell's parser can identify shapes based on many criteria, often -# as the commandline is being typed. +# as the commandline is being typed. # shape_string: Can appear as a single-or-quoted value, a bareword string, the key of a record, # an argument which has been declared as a string, and other parsed strings. @@ -733,7 +738,7 @@ $env.config.color_config.custom # Custom value (often from a plugin) $env.config.color_config.nothing # Not used, since a null is not displayed $env.config.color_config.date # datetime value $env.config.color_config.filesize # filesize value -$env.config.color_config.list # Not currently used. Lists are displayed using their +$env.config.color_config.list # Not currently used. Lists are displayed using their # members' styles $env.config.color_config.record # Not currently used. Records are displayed using their # member's styles @@ -828,7 +833,7 @@ $env.PROMPT_INDICATOR_VI_INSERT = ": " # When a commandline extends across multiple lines: $env.PROMPT_MULTILINE_INDICATOR = "::: " -# TRANSIENT_PROMPT_* +# TRANSIENT_PROMPT_* # ------------------ # Allows a different prompt to be shown after a command has been executed. This # can be useful if you have a 2-line prompt. Instead of each previously-entered @@ -855,10 +860,10 @@ $env.TRANSIENT_PROMPT_COMMAND_RIGHT = "" # # Note: The OS Path variable is automatically converted before env.nu loads, so it can # be treated a list in this file. -# +# # Note: Environment variables are not case-sensitive, so the following will work # for both Windows and Unix-like platforms. -# +# # By default, the internal conversion looks something like the following, so there # is no need to add this in your actual env.nu: $env.ENV_CONVERSIONS = { @@ -912,12 +917,12 @@ const NU_PLUGIN_DIRS = $NU_PLUGIN_DIRS ++ [($nu.default-config-dir | path join ' # Appending to the OS path is a common configuration task. # Because of the previous ENV_CONVERSIONS (performed internally -# before your config.nu loads), the path variable is a list that can +# before your config.nu loads), the path variable is a list that can # be appended to using, for example: -$env.path ++= "~/.local/bin" +$env.PATH ++= [ "~/.local/bin" ] # Or prepend using -$env.path = "~/.local/bin" ++ $env.path +$env.PATH = [ "~/.local/bin" ] ++ $env.PATH # The `path add` function from the Standard Library also provides # a convenience method for prepending to the path: diff --git a/crates/nu-utils/src/default_files/sample_env.nu b/crates/nu-utils/src/default_files/sample_env.nu index c081c87609..a69363cdf7 100644 --- a/crates/nu-utils/src/default_files/sample_env.nu +++ b/crates/nu-utils/src/default_files/sample_env.nu @@ -1,9 +1,11 @@ # Sample Nushell Environment Config File # +# version = "0.100.1" +# # Previously, environment variables were typically configured in `env.nu`. # In general, most configuration can and should be performed in `config.nu` # or one of the autoload directories. # To pretty-print the in-shell documentation for Nushell's various configuration # settings, you can run: -config nu --sample | nu-highlight | less -R \ No newline at end of file +config nu --sample | nu-highlight | less -R diff --git a/crates/nu-utils/src/default_files/sample_login.nu b/crates/nu-utils/src/default_files/sample_login.nu index 8e0a651499..511e3b803c 100644 --- a/crates/nu-utils/src/default_files/sample_login.nu +++ b/crates/nu-utils/src/default_files/sample_login.nu @@ -1,4 +1,7 @@ # Example Nushell Loginshell Config File +# +# version = "0.100.1" +# # - has to be as login.nu in the default config directory # - will be sourced after config.nu and env.nu in case of nushell started as login shell diff --git a/crates/nu-utils/src/default_files/scaffold_config.nu b/crates/nu-utils/src/default_files/scaffold_config.nu index 7bf7ecc632..fa6d834c73 100644 --- a/crates/nu-utils/src/default_files/scaffold_config.nu +++ b/crates/nu-utils/src/default_files/scaffold_config.nu @@ -1,11 +1,14 @@ # config.nu # +# Installed by: +# version = "0.100.1" +# # This file is used to override default Nushell settings, define # (or import) custom commands, or run any other startup tasks. # See https://www.nushell.sh/book/configuration.html # # This file is loaded after env.nu and before login.nu -# +# # You can open this file in your default editor using: # config nu # diff --git a/crates/nu-utils/src/default_files/scaffold_env.nu b/crates/nu-utils/src/default_files/scaffold_env.nu index 7071b9fb3d..d7a2e00627 100644 --- a/crates/nu-utils/src/default_files/scaffold_env.nu +++ b/crates/nu-utils/src/default_files/scaffold_env.nu @@ -1,5 +1,8 @@ # env.nu # +# Installed by: +# version = "0.100.1" +# # Previously, environment variables were typically configured in `env.nu`. # In general, most configuration can and should be performed in `config.nu` # or one of the autoload directories. From 981a000ee81b71692ac5d24ad7095a6460dabd94 Mon Sep 17 00:00:00 2001 From: Jack Wright <56345+ayax79@users.noreply.github.com> Date: Tue, 17 Dec 2024 09:55:42 -0800 Subject: [PATCH 07/26] Added flag --coalesce-columns to allow columns to be coalsced on full joins (#14578) - fixes #14572 # Description This allowed columns to be coalesced on full joins with `polars join`, providing functionality simlar to the old `--outer` join behavior. # User-Facing Changes - Provides a new flag `--coalesce-columns` on the `polars join` command --- .../src/dataframe/command/data/join.rs | 33 ++++++++++++++++++- 1 file changed, 32 insertions(+), 1 deletion(-) diff --git a/crates/nu_plugin_polars/src/dataframe/command/data/join.rs b/crates/nu_plugin_polars/src/dataframe/command/data/join.rs index b1c13fef00..bd140b5c2a 100644 --- a/crates/nu_plugin_polars/src/dataframe/command/data/join.rs +++ b/crates/nu_plugin_polars/src/dataframe/command/data/join.rs @@ -8,7 +8,10 @@ use nu_protocol::{ Category, Example, LabeledError, PipelineData, ShellError, Signature, Span, SyntaxShape, Type, Value, }; -use polars::prelude::{Expr, JoinType}; +use polars::{ + df, + prelude::{Expr, JoinCoalesce, JoinType}, +}; #[derive(Clone)] pub struct LazyJoin; @@ -37,6 +40,7 @@ impl PluginCommand for LazyJoin { .switch("left", "left join between lazyframes", Some('l')) .switch("full", "full join between lazyframes", Some('f')) .switch("cross", "cross join between lazyframes", Some('c')) + .switch("coalesce-columns", "Sets the join coalesce strategy to colesce columns. Most useful when used with --full, which will not otherwise coalesce.", None) .named( "suffix", SyntaxShape::String, @@ -172,6 +176,24 @@ impl PluginCommand for LazyJoin { .into_value(Span::test_data()), ), }, + Example { + description: "Perform a full join of two dataframes and coalesce columns", + example: r#"let table1 = [[A B]; ["common" "common"] ["table1" "only"]] | polars into-df + let table2 = [[A C]; ["common" "common"] ["table2" "only"]] | polars into-df + $table1 | polars join -f $table2 --coalesce-columns A A"#, + result: Some( + NuDataFrame::new( + false, + df!( + "A" => [Some("common"), Some("table2"), Some("table1")], + "B" => [Some("common"), None, Some("only")], + "C" => [Some("common"), Some("only"), None] + ) + .expect("Should have created a DataFrame"), + ) + .into_value(Span::test_data()), + ), + }, Example { description: "Join one eager dataframe with another using a cross join", example: r#"let tokens = [[monopoly_token]; [hat] [shoe] [boat]] | polars into-df @@ -279,9 +301,17 @@ impl PluginCommand for LazyJoin { let lazy = NuLazyFrame::try_from_value_coerce(plugin, &value)?; let from_eager = lazy.from_eager; let lazy = lazy.to_polars(); + + let coalesce = if call.has_flag("coalesce-columns")? { + JoinCoalesce::CoalesceColumns + } else { + JoinCoalesce::default() + }; + let lazy = if cross { lazy.join_builder() .with(other) + .coalesce(coalesce) .left_on(vec![]) .right_on(vec![]) .how(how) @@ -291,6 +321,7 @@ impl PluginCommand for LazyJoin { } else { lazy.join_builder() .with(other) + .coalesce(coalesce) .left_on(left_on) .right_on(right_on) .how(how) From f41c53fef117512601f5f4b91109f03aaef05cf0 Mon Sep 17 00:00:00 2001 From: Darren Schroeder <343840+fdncred@users.noreply.github.com> Date: Tue, 17 Dec 2024 13:15:16 -0600 Subject: [PATCH 08/26] allow `view source` to take `int` as a parameter (#14609) # Description This PR allows the `view source` command to view source based on an int value. I wrote this specifically to be able to see closures where the text is hidden. For example: ![image](https://github.com/user-attachments/assets/d8fe2692-0951-4366-9cb9-55f20044b68a) And then you can use those `` with the `view source` command like this. ![image](https://github.com/user-attachments/assets/f428c8ad-56a9-4e72-880e-e32fb9155531) # User-Facing Changes # Tests + Formatting # After Submitting --- crates/nu-command/src/debug/view_source.rs | 24 ++++++++++++++++++---- 1 file changed, 20 insertions(+), 4 deletions(-) diff --git a/crates/nu-command/src/debug/view_source.rs b/crates/nu-command/src/debug/view_source.rs index 417da5ec1e..2aadd4d361 100644 --- a/crates/nu-command/src/debug/view_source.rs +++ b/crates/nu-command/src/debug/view_source.rs @@ -33,6 +33,22 @@ impl Command for ViewSource { let arg_span = arg.span(); let source = match arg { + Value::Int { val, .. } => { + let block = engine_state.get_block(nu_protocol::BlockId::new(val as usize)); + if let Some(span) = block.span { + let contents = engine_state.get_span_contents(span); + Ok(Value::string(String::from_utf8_lossy(contents), call.head) + .into_pipeline_data()) + } else { + Err(ShellError::GenericError { + error: "Cannot view int value".to_string(), + msg: "the block does not have a viewable span".to_string(), + span: Some(arg_span), + help: None, + inner: vec![], + }) + } + } Value::String { val, .. } => { if let Some(decl_id) = engine_state.find_decl(val.as_bytes(), &[]) { // arg is a command @@ -130,7 +146,7 @@ impl Command for ViewSource { Ok(Value::string(final_contents, call.head).into_pipeline_data()) } else { Err(ShellError::GenericError { - error: "Cannot view value".to_string(), + error: "Cannot view string value".to_string(), msg: "the command does not have a viewable block span".to_string(), span: Some(arg_span), help: None, @@ -139,7 +155,7 @@ impl Command for ViewSource { } } else { Err(ShellError::GenericError { - error: "Cannot view value".to_string(), + error: "Cannot view string decl value".to_string(), msg: "the command does not have a viewable block".to_string(), span: Some(arg_span), help: None, @@ -155,7 +171,7 @@ impl Command for ViewSource { .into_pipeline_data()) } else { Err(ShellError::GenericError { - error: "Cannot view value".to_string(), + error: "Cannot view string module value".to_string(), msg: "the module does not have a viewable block".to_string(), span: Some(arg_span), help: None, @@ -164,7 +180,7 @@ impl Command for ViewSource { } } else { Err(ShellError::GenericError { - error: "Cannot view value".to_string(), + error: "Cannot view string value".to_string(), msg: "this name does not correspond to a viewable value".to_string(), span: Some(arg_span), help: None, From c0ad659985d7d45c1fe0589ed84461ba38515de9 Mon Sep 17 00:00:00 2001 From: Douglas <32344964+NotTheDr01ds@users.noreply.github.com> Date: Tue, 17 Dec 2024 15:18:23 -0500 Subject: [PATCH 09/26] Doc file fixes (#14608) # Description With great thanks to @fdncred and especially @PerchunPak (see #14601) for finding and fixing a number of issues that I pulled in here due to the filename changes and upcoming freeze. This PR primarily fixes a poor wording choice in the new filenames and `config` command options. The fact that these were called `sample_config.nu` (etc.) and accessed via `config --sample` created a great deal of confusion. These were never intended to be used as-is as config files, but rather as in-shell documentation. As such, I've renamed them: * `sample_config.nu` becomes `doc_config.nu` * `sample_env.nu` becomes `doc_env.nu` * `config nu --sample` becomes `config nu --doc` * `config env --sample` because `config env --doc` Also the following: * Updates `doc_config.nu` with a few additional comment-fixes on top of @PerchunPak's changes. * Adds version numbers to all files - Will need to update the version script to add some files after this PR. * Additional doc on plugin and plugin_gc configuration which I had failed to previously completely update from the older wording * Updated the comments in the `scaffold_*.nu` files to point people to `help config`/`help nu` so that, if things change in the future, it will become more difficult for the comments to be outdated. * # User-Facing Changes Mostly doc. `config nu` and `config env` changes update new behavior previously added in 0.100.1 # Tests + Formatting - :green_circle: `toolkit fmt` - :green_circle: `toolkit clippy` - :green_circle: `toolkit test` - :green_circle: `toolkit test stdlib` # After Submitting * Update configuration chapter of doc * Update the blog entry on migrating config * Update `bump-version.nu` --- .../nu-command/src/env/config/config_env.rs | 22 +++++----- crates/nu-command/src/env/config/config_nu.rs | 22 +++++----- crates/nu-utils/src/default_files/README.md | 18 ++++---- .../{sample_config.nu => doc_config.nu} | 41 ++++++++++--------- .../{sample_env.nu => doc_env.nu} | 6 ++- .../src/default_files/scaffold_config.nu | 6 +-- .../src/default_files/scaffold_env.nu | 6 +-- crates/nu-utils/src/lib.rs | 4 +- crates/nu-utils/src/utils.rs | 8 ++-- 9 files changed, 65 insertions(+), 68 deletions(-) rename crates/nu-utils/src/default_files/{sample_config.nu => doc_config.nu} (97%) rename crates/nu-utils/src/default_files/{sample_env.nu => doc_env.nu} (74%) diff --git a/crates/nu-command/src/env/config/config_env.rs b/crates/nu-command/src/env/config/config_env.rs index 5fdc39a903..f078b365d2 100644 --- a/crates/nu-command/src/env/config/config_env.rs +++ b/crates/nu-command/src/env/config/config_env.rs @@ -18,8 +18,8 @@ impl Command for ConfigEnv { Some('d'), ) .switch( - "sample", - "Print a commented, sample `env.nu` file instead.", + "doc", + "Print a commented `env.nu` with documentation instead.", Some('s'), ) // TODO: Signature narrower than what run actually supports theoretically @@ -37,8 +37,8 @@ impl Command for ConfigEnv { result: None, }, Example { - description: "pretty-print a commented, sample `env.nu` that explains common settings", - example: "config env --sample | nu-highlight,", + description: "pretty-print a commented `env.nu` that explains common settings", + example: "config env --doc | nu-highlight,", result: None, }, Example { @@ -57,13 +57,13 @@ impl Command for ConfigEnv { _input: PipelineData, ) -> Result { let default_flag = call.has_flag(engine_state, stack, "default")?; - let sample_flag = call.has_flag(engine_state, stack, "sample")?; - if default_flag && sample_flag { + let doc_flag = call.has_flag(engine_state, stack, "doc")?; + if default_flag && doc_flag { return Err(ShellError::IncompatibleParameters { left_message: "can't use `--default` at the same time".into(), left_span: call.get_flag_span(stack, "default").expect("has flag"), - right_message: "because of `--sample`".into(), - right_span: call.get_flag_span(stack, "sample").expect("has flag"), + right_message: "because of `--doc`".into(), + right_span: call.get_flag_span(stack, "doc").expect("has flag"), }); } // `--default` flag handling @@ -72,10 +72,10 @@ impl Command for ConfigEnv { return Ok(Value::string(nu_utils::get_default_env(), head).into_pipeline_data()); } - // `--sample` flag handling - if sample_flag { + // `--doc` flag handling + if doc_flag { let head = call.head; - return Ok(Value::string(nu_utils::get_sample_env(), head).into_pipeline_data()); + return Ok(Value::string(nu_utils::get_doc_env(), head).into_pipeline_data()); } super::config_::start_editor("env-path", engine_state, stack, call) diff --git a/crates/nu-command/src/env/config/config_nu.rs b/crates/nu-command/src/env/config/config_nu.rs index 75d971f36e..66e24e2a94 100644 --- a/crates/nu-command/src/env/config/config_nu.rs +++ b/crates/nu-command/src/env/config/config_nu.rs @@ -18,8 +18,8 @@ impl Command for ConfigNu { Some('d'), ) .switch( - "sample", - "Print a commented, sample `config.nu` file instead.", + "doc", + "Print a commented `config.nu` with documentation instead.", Some('s'), ) // TODO: Signature narrower than what run actually supports theoretically @@ -37,8 +37,8 @@ impl Command for ConfigNu { result: None, }, Example { - description: "pretty-print a commented, sample `config.nu` that explains common settings", - example: "config nu --sample | nu-highlight", + description: "pretty-print a commented `config.nu` that explains common settings", + example: "config nu --doc | nu-highlight", result: None, }, Example { @@ -58,13 +58,13 @@ impl Command for ConfigNu { _input: PipelineData, ) -> Result { let default_flag = call.has_flag(engine_state, stack, "default")?; - let sample_flag = call.has_flag(engine_state, stack, "sample")?; - if default_flag && sample_flag { + let doc_flag = call.has_flag(engine_state, stack, "doc")?; + if default_flag && doc_flag { return Err(ShellError::IncompatibleParameters { left_message: "can't use `--default` at the same time".into(), left_span: call.get_flag_span(stack, "default").expect("has flag"), - right_message: "because of `--sample`".into(), - right_span: call.get_flag_span(stack, "sample").expect("has flag"), + right_message: "because of `--doc`".into(), + right_span: call.get_flag_span(stack, "doc").expect("has flag"), }); } @@ -74,10 +74,10 @@ impl Command for ConfigNu { return Ok(Value::string(nu_utils::get_default_config(), head).into_pipeline_data()); } - // `--sample` flag handling - if sample_flag { + // `--doc` flag handling + if doc_flag { let head = call.head; - return Ok(Value::string(nu_utils::get_sample_config(), head).into_pipeline_data()); + return Ok(Value::string(nu_utils::get_doc_config(), head).into_pipeline_data()); } super::config_::start_editor("config-path", engine_state, stack, call) diff --git a/crates/nu-utils/src/default_files/README.md b/crates/nu-utils/src/default_files/README.md index 68a36b865d..926870d20a 100644 --- a/crates/nu-utils/src/default_files/README.md +++ b/crates/nu-utils/src/default_files/README.md @@ -9,7 +9,7 @@ * During a startup where the user specifies an alternative `env.nu` via `nu --env-config ` * During a `nu -c ` or `nu