From a56915be5331c0fb33ce2f687b15c5f812e28d96 Mon Sep 17 00:00:00 2001 From: Xavier L'Heureux <xavier.lheureux@icloud.com> Date: Thu, 30 May 2019 16:43:27 -0400 Subject: [PATCH] Remove the GetVariables Trait The GetVariables Trait encouraged cloning rather than using references. Since it is rarely the case that cloning is actually needed, it is better to replace it entirely. Furthermore, most of its usage included matching, so it does not significantly increase general verbosity of the code. To account for the env vars that must be fetched, a new method, get_str, was introduced. --- src/binary/history.rs | 59 ++-- src/binary/mod.rs | 8 +- src/binary/prompt.rs | 5 +- src/lib/builtins/command_info.rs | 39 ++- src/lib/builtins/exists.rs | 491 ++++++++++++++++--------------- src/lib/builtins/is.rs | 6 +- src/lib/builtins/mod.rs | 3 +- src/lib/builtins/set.rs | 26 +- src/lib/builtins/variables.rs | 2 +- src/lib/shell/assignments.rs | 3 +- src/lib/shell/directory_stack.rs | 6 +- src/lib/shell/flow.rs | 24 +- src/lib/shell/fork.rs | 2 +- src/lib/shell/job.rs | 8 +- src/lib/shell/mod.rs | 29 +- src/lib/shell/pipe_exec/mod.rs | 34 ++- src/lib/shell/shell_expand.rs | 76 ++--- src/lib/shell/variables/mod.rs | 140 +++------ 18 files changed, 465 insertions(+), 496 deletions(-) diff --git a/src/binary/history.rs b/src/binary/history.rs index 8681b0a4..e5a432f3 100644 --- a/src/binary/history.rs +++ b/src/binary/history.rs @@ -1,5 +1,5 @@ use super::InteractiveBinary; -use ion_shell::{status::*, types}; +use ion_shell::{status::*, Value}; use regex::Regex; use std::time::{SystemTime, UNIX_EPOCH}; @@ -26,40 +26,49 @@ impl<'a> InteractiveBinary<'a> { /// Updates the history ignore patterns. Call this whenever HISTORY_IGNORE /// is changed. pub fn ignore_patterns(&self) -> IgnoreSetting { - let patterns: types::Array = self.shell.borrow().variables().get("HISTORY_IGNORE").unwrap(); - let mut settings = IgnoreSetting::default(); - let mut regexes = Vec::new(); - // for convenience and to avoid typos - let regex_prefix = "regex:"; - for pattern in patterns.into_iter() { - let pattern = format!("{}", pattern); - match pattern.as_ref() { - "all" => settings.all = true, - "no_such_command" => settings.no_such_command = true, - "whitespace" => settings.whitespace = true, - "duplicates" => settings.duplicates = true, - // The length check is there to just ignore empty regex definitions - _ if pattern.starts_with(regex_prefix) && pattern.len() > regex_prefix.len() => { - settings.based_on_regex = true; - let regex_string = &pattern[regex_prefix.len()..]; - // We save the compiled regexes, as compiling them can be an expensive task - if let Ok(regex) = Regex::new(regex_string) { - regexes.push(regex); + if let Some(Value::Array(patterns)) = + self.shell.borrow().variables().get_ref("HISTORY_IGNORE") + { + let mut settings = IgnoreSetting::default(); + let mut regexes = Vec::new(); + // for convenience and to avoid typos + let regex_prefix = "regex:"; + for pattern in patterns.into_iter() { + let pattern = format!("{}", pattern); + match pattern.as_ref() { + "all" => settings.all = true, + "no_such_command" => settings.no_such_command = true, + "whitespace" => settings.whitespace = true, + "duplicates" => settings.duplicates = true, + // The length check is there to just ignore empty regex definitions + _ if pattern.starts_with(regex_prefix) + && pattern.len() > regex_prefix.len() => + { + settings.based_on_regex = true; + let regex_string = &pattern[regex_prefix.len()..]; + // We save the compiled regexes, as compiling them can be an expensive task + if let Ok(regex) = Regex::new(regex_string) { + regexes.push(regex); + } } + _ => continue, } - _ => continue, } - } - settings.regexes = if !regexes.is_empty() { Some(regexes) } else { None }; + settings.regexes = if !regexes.is_empty() { Some(regexes) } else { None }; - settings + settings + } else { + panic!("HISTORY_IGNORE is not set!"); + } } /// Saves a command in the history, depending on @HISTORY_IGNORE. Should be called /// immediately after `on_command()` pub fn save_command_in_history(&self, command: &str) { if self.should_save_command(command) { - if self.shell.borrow().variables().get_str_or_empty("HISTORY_TIMESTAMP") == "1" { + if self.shell.borrow().variables().get_str("HISTORY_TIMESTAMP").unwrap_or_default() + == "1" + { // Get current time stamp let since_unix_epoch = SystemTime::now().duration_since(UNIX_EPOCH).unwrap().as_secs(); diff --git a/src/binary/mod.rs b/src/binary/mod.rs index 0203e9ec..26578da7 100644 --- a/src/binary/mod.rs +++ b/src/binary/mod.rs @@ -10,7 +10,7 @@ use ion_shell::{ builtins::man_pages, parser::{Expander, Terminator}, status::{FAILURE, SUCCESS}, - types, Shell, + Shell, }; use itertools::Itertools; use liner::{Buffer, Context, KeyBindings}; @@ -57,8 +57,8 @@ impl<'a> InteractiveBinary<'a> { pub fn new(shell: Shell<'a>) -> Self { let mut context = Context::new(); context.word_divider_fn = Box::new(word_divide); - if shell.variables().get_str_or_empty("HISTFILE_ENABLED") == "1" { - let path = shell.get::<types::Str>("HISTFILE").expect("shell didn't set HISTFILE"); + if shell.variables().get_str("HISTFILE_ENABLED") == Some("1".into()) { + let path = shell.variables().get_str("HISTFILE").expect("shell didn't set HISTFILE"); if !Path::new(path.as_str()).exists() { eprintln!("ion: creating history file at \"{}\"", path); } @@ -96,7 +96,7 @@ impl<'a> InteractiveBinary<'a> { // If `RECORD_SUMMARY` is set to "1" (True, Yes), then write a summary of the // pipline just executed to the the file and context histories. At the // moment, this means record how long it took. - if "1" == shell.variables().get_str_or_empty("RECORD_SUMMARY") { + if Some("1".into()) == shell.variables().get_str("RECORD_SUMMARY") { let summary = format!( "#summary# elapsed real time: {}.{:09} seconds", elapsed.as_secs(), diff --git a/src/binary/prompt.rs b/src/binary/prompt.rs index 0212a566..df03a257 100644 --- a/src/binary/prompt.rs +++ b/src/binary/prompt.rs @@ -6,7 +6,10 @@ pub fn prompt(shell: &Shell) -> String { if blocks == 0 { prompt_fn(&shell).unwrap_or_else(|| { - shell.get_string(&shell.variables().get_str_or_empty("PROMPT")).as_str().into() + shell + .get_string(&shell.variables().get_str("PROMPT").unwrap_or_default()) + .as_str() + .into() }) } else { " ".repeat(blocks) diff --git a/src/lib/builtins/command_info.rs b/src/lib/builtins/command_info.rs index 3a23d537..45d3ce27 100644 --- a/src/lib/builtins/command_info.rs +++ b/src/lib/builtins/command_info.rs @@ -1,7 +1,7 @@ use crate::{ builtins::man_pages::*, - shell::{flow_control::Function, status::*, Shell}, - sys, types, + shell::{status::*, Shell, Value}, + sys, }; use small; @@ -22,8 +22,8 @@ pub fn which(args: &[small::String], shell: &mut Shell) -> Result<i32, ()> { match get_command_info(command, shell) { Ok(c_type) => match c_type.as_ref() { "alias" => { - if let Some(alias) = shell.variables().get::<types::Alias>(&**command) { - println!("{}: alias to {}", command, &*alias); + if let Some(Value::Alias(ref alias)) = shell.variables().get_ref(&**command) { + println!("{}: alias to {}", command, &**alias); } } "function" => println!("{}: function", command), @@ -49,8 +49,8 @@ pub fn find_type(args: &[small::String], shell: &mut Shell) -> Result<i32, ()> { Ok(c_type) => { match c_type.as_ref() { "alias" => { - if let Some(alias) = shell.variables().get::<types::Alias>(&**command) { - println!("{} is aliased to `{}`", command, &*alias); + if let Some(Value::Alias(alias)) = shell.variables().get_ref(&**command) { + println!("{} is aliased to `{}`", command, &**alias); } } // TODO Make it print the function. @@ -67,21 +67,20 @@ pub fn find_type(args: &[small::String], shell: &mut Shell) -> Result<i32, ()> { } pub fn get_command_info<'a>(command: &str, shell: &mut Shell) -> Result<Cow<'a, str>, ()> { - if shell.variables().get::<types::Alias>(command).is_some() { - return Ok("alias".into()); - } else if shell.variables().get::<Function>(command).is_some() { - return Ok("function".into()); - } else if shell.builtins().contains(command) { - return Ok("builtin".into()); - } else { - for path in - env::var("PATH").unwrap_or_else(|_| String::from("/bin")).split(sys::PATH_SEPARATOR) - { - let executable = Path::new(path).join(command); - if executable.is_file() { - return Ok(executable.display().to_string().into()); + match shell.variables().get_ref(command) { + Some(Value::Alias(_)) => Ok("alias".into()), + Some(Value::Function(_)) => Ok("function".into()), + _ if shell.builtins().contains(command) => Ok("builtin".into()), + _ => { + for path in + env::var("PATH").unwrap_or_else(|_| String::from("/bin")).split(sys::PATH_SEPARATOR) + { + let executable = Path::new(path).join(command); + if executable.is_file() { + return Ok(executable.display().to_string().into()); + } } + Err(()) } } - Err(()) } diff --git a/src/lib/builtins/exists.rs b/src/lib/builtins/exists.rs index 6283d787..265cda27 100644 --- a/src/lib/builtins/exists.rs +++ b/src/lib/builtins/exists.rs @@ -1,11 +1,6 @@ use std::{fs, os::unix::fs::PermissionsExt}; -#[cfg(test)] -use crate::shell::{self, flow_control::Statement}; -use crate::{ - shell::{flow_control::Function, Shell}, - types, -}; +use crate::shell::{Shell, Value}; use small; pub fn exists(args: &[small::String], shell: &Shell) -> Result<bool, small::String> { @@ -76,7 +71,7 @@ fn binary_is_in_path(binaryname: &str, shell: &Shell) -> bool { // TODO: Right now they use an entirely different logic which means that it // *might* be possible TODO: that `exists` reports a binary to be in the // path, while the shell cannot find it or TODO: vice-versa - if let Some(path) = shell.get::<types::Str>("PATH") { + if let Some(path) = shell.variables().get_str("PATH") { for fname in path.split(':').map(|dir| format!("{}/{}", dir, binaryname)) { if let Ok(metadata) = fs::metadata(&fname) { if metadata.is_file() && file_has_execute_permission(&fname) { @@ -113,15 +108,15 @@ fn string_is_nonzero(string: &str) -> bool { !string.is_empty() } /// Returns true if the variable is an array and the array is not empty fn array_var_is_not_empty(arrayvar: &str, shell: &Shell) -> bool { - match shell.variables().get::<types::Array>(arrayvar) { - Some(array) => !array.is_empty(), - None => false, + match shell.variables().get_ref(arrayvar) { + Some(Value::Array(array)) => !array.is_empty(), + _ => false, } } /// Returns true if the variable is a string and the string is not empty fn string_var_is_not_empty(stringvar: &str, shell: &Shell) -> bool { - match shell.variables().get::<types::Str>(stringvar) { + match shell.variables().get_str(stringvar) { Some(string) => !string.is_empty(), None => false, } @@ -129,246 +124,278 @@ fn string_var_is_not_empty(stringvar: &str, shell: &Shell) -> bool { /// Returns true if a function with the given name is defined fn function_is_defined(function: &str, shell: &Shell) -> bool { - shell.variables().get::<Function>(function).is_some() -} - -#[test] -fn test_evaluate_arguments() { - use crate::lexers::assignments::{KeyBuf, Primitive}; - let mut shell = shell::Shell::library(); - - // assert_eq!(exists(&["ion".into(), ], &mut sink, &shell), Ok(false)); - // no parameters - assert_eq!(exists(&["ion".into()], &shell), Ok(false)); - // multiple arguments - // ignores all but the first argument - assert_eq!(exists(&["ion".into(), "foo".into(), "bar".into()], &shell), Ok(true)); - - // check `exists STRING` - assert_eq!(exists(&["ion".into(), "".into()], &shell), Ok(false)); - assert_eq!(exists(&["ion".into(), "string".into()], &shell), Ok(true)); - assert_eq!(exists(&["ion".into(), "string with space".into()], &shell), Ok(true)); - assert_eq!(exists(&["ion".into(), "-startswithdash".into()], &shell), Ok(true)); - - // check `exists -a` - // no argument means we treat it as a string - assert_eq!(exists(&["ion".into(), "-a".into()], &shell), Ok(true)); - shell.variables_mut().set("emptyarray", types::Array::new()); - assert_eq!(exists(&["ion".into(), "-a".into(), "emptyarray".into()], &shell), Ok(false)); - let mut array = types::Array::new(); - array.push("element".into()); - shell.variables_mut().set("array", array); - assert_eq!(exists(&["ion".into(), "-a".into(), "array".into()], &shell), Ok(true)); - shell.variables_mut().remove_variable("array"); - assert_eq!(exists(&["ion".into(), "-a".into(), "array".into()], &shell), Ok(false)); - - // check `exists -b` - // TODO: see test_binary_is_in_path() - // no argument means we treat it as a string - assert_eq!(exists(&["ion".into(), "-b".into()], &shell), Ok(true)); - let oldpath = shell.get::<types::Str>("PATH").unwrap_or_else(|| "/usr/bin".into()); - shell.set("PATH", "testing/"); - - assert_eq!(exists(&["ion".into(), "-b".into(), "executable_file".into()], &shell), Ok(true)); - assert_eq!(exists(&["ion".into(), "-b".into(), "empty_file".into()], &shell), Ok(false)); - assert_eq!( - exists(&["ion".into(), "-b".into(), "file_does_not_exist".into()], &shell), - Ok(false) - ); - - // restore original PATH. Not necessary for the currently defined test cases - // but this might change in the future? Better safe than sorry! - shell.set("PATH", oldpath); - - // check `exists -d` - // no argument means we treat it as a string - assert_eq!(exists(&["ion".into(), "-d".into()], &shell), Ok(true)); - assert_eq!(exists(&["ion".into(), "-d".into(), "testing/".into()], &shell), Ok(true)); - assert_eq!( - exists(&["ion".into(), "-d".into(), "testing/empty_file".into()], &shell), - Ok(false) - ); - assert_eq!(exists(&["ion".into(), "-d".into(), "does/not/exist/".into()], &shell), Ok(false)); - - // check `exists -f` - // no argument means we treat it as a string - assert_eq!(exists(&["ion".into(), "-f".into()], &shell), Ok(true)); - assert_eq!(exists(&["ion".into(), "-f".into(), "testing/".into()], &shell), Ok(false)); - assert_eq!(exists(&["ion".into(), "-f".into(), "testing/empty_file".into()], &shell), Ok(true)); - assert_eq!(exists(&["ion".into(), "-f".into(), "does-not-exist".into()], &shell), Ok(false)); - - // check `exists -s` - // no argument means we treat it as a string - assert_eq!(exists(&["ion".into(), "-s".into()], &shell), Ok(true)); - shell.set("emptyvar", "".to_string()); - assert_eq!(exists(&["ion".into(), "-s".into(), "emptyvar".into()], &shell), Ok(false)); - shell.set("testvar", "foobar".to_string()); - assert_eq!(exists(&["ion".into(), "-s".into(), "testvar".into()], &shell), Ok(true)); - shell.variables_mut().remove_variable("testvar"); - assert_eq!(exists(&["ion".into(), "-s".into(), "testvar".into()], &shell), Ok(false)); - // also check that it doesn't trigger on arrays - let mut array = types::Array::new(); - array.push("element".into()); - shell.variables_mut().remove_variable("array"); - shell.variables_mut().set("array", array); - assert_eq!(exists(&["ion".into(), "-s".into(), "array".into()], &shell), Ok(false)); - - // check `exists --fn` - let name_str = "test_function"; - let name = small::String::from(name_str); - let mut args = Vec::new(); - args.push(KeyBuf { name: "testy".into(), kind: Primitive::Str }); - let mut statements = Vec::new(); - statements.push(Statement::End); - let description: small::String = "description".into(); - - shell - .variables_mut() - .set(&name, Function::new(Some(description), name.clone(), args, statements)); - - assert_eq!(exists(&["ion".into(), "--fn".into(), name_str.into()], &shell), Ok(true)); - shell.variables_mut().remove_variable(name_str); - assert_eq!(exists(&["ion".into(), "--fn".into(), name_str.into()], &shell), Ok(false)); - - // check invalid flags / parameters (should all be treated as strings and - // therefore succeed) - assert_eq!(exists(&["ion".into(), "--foo".into()], &shell), Ok(true)); - assert_eq!(exists(&["ion".into(), "-x".into()], &shell), Ok(true)); + if let Some(Value::Function(_)) = shell.variables().get_ref(function) { + true + } else { + false + } } +#[cfg(test)] +mod tests { + use super::*; + use crate::{ + flow_control::Function, + lexers::assignments::{KeyBuf, Primitive}, + shell::flow_control::Statement, + types, + }; + use small; + + #[test] + fn test_evaluate_arguments() { + let mut shell = Shell::library(); + + // assert_eq!(exists(&["ion".into(), ], &mut sink, &shell), Ok(false)); + // no parameters + assert_eq!(exists(&["ion".into()], &shell), Ok(false)); + // multiple arguments + // ignores all but the first argument + assert_eq!(exists(&["ion".into(), "foo".into(), "bar".into()], &shell), Ok(true)); + + // check `exists STRING` + assert_eq!(exists(&["ion".into(), "".into()], &shell), Ok(false)); + assert_eq!(exists(&["ion".into(), "string".into()], &shell), Ok(true)); + assert_eq!(exists(&["ion".into(), "string with space".into()], &shell), Ok(true)); + assert_eq!(exists(&["ion".into(), "-startswithdash".into()], &shell), Ok(true)); + + // check `exists -a` + // no argument means we treat it as a string + assert_eq!(exists(&["ion".into(), "-a".into()], &shell), Ok(true)); + shell.variables_mut().set("emptyarray", types::Array::new()); + assert_eq!(exists(&["ion".into(), "-a".into(), "emptyarray".into()], &shell), Ok(false)); + let mut array = types::Array::new(); + array.push("element".into()); + shell.variables_mut().set("array", array); + assert_eq!(exists(&["ion".into(), "-a".into(), "array".into()], &shell), Ok(true)); + shell.variables_mut().remove_variable("array"); + assert_eq!(exists(&["ion".into(), "-a".into(), "array".into()], &shell), Ok(false)); + + // check `exists -b` + // TODO: see test_binary_is_in_path() + // no argument means we treat it as a string + assert_eq!(exists(&["ion".into(), "-b".into()], &shell), Ok(true)); + let oldpath = shell.variables().get_str("PATH").unwrap_or_else(|| "/usr/bin".into()); + shell.variables_mut().set("PATH", "testing/"); + + assert_eq!( + exists(&["ion".into(), "-b".into(), "executable_file".into()], &shell), + Ok(true) + ); + assert_eq!(exists(&["ion".into(), "-b".into(), "empty_file".into()], &shell), Ok(false)); + assert_eq!( + exists(&["ion".into(), "-b".into(), "file_does_not_exist".into()], &shell), + Ok(false) + ); + + // restore original PATH. Not necessary for the currently defined test cases + // but this might change in the future? Better safe than sorry! + shell.variables_mut().set("PATH", oldpath); + + // check `exists -d` + // no argument means we treat it as a string + assert_eq!(exists(&["ion".into(), "-d".into()], &shell), Ok(true)); + assert_eq!(exists(&["ion".into(), "-d".into(), "testing/".into()], &shell), Ok(true)); + assert_eq!( + exists(&["ion".into(), "-d".into(), "testing/empty_file".into()], &shell), + Ok(false) + ); + assert_eq!( + exists(&["ion".into(), "-d".into(), "does/not/exist/".into()], &shell), + Ok(false) + ); + + // check `exists -f` + // no argument means we treat it as a string + assert_eq!(exists(&["ion".into(), "-f".into()], &shell), Ok(true)); + assert_eq!(exists(&["ion".into(), "-f".into(), "testing/".into()], &shell), Ok(false)); + assert_eq!( + exists(&["ion".into(), "-f".into(), "testing/empty_file".into()], &shell), + Ok(true) + ); + assert_eq!( + exists(&["ion".into(), "-f".into(), "does-not-exist".into()], &shell), + Ok(false) + ); + + // check `exists -s` + // no argument means we treat it as a string + assert_eq!(exists(&["ion".into(), "-s".into()], &shell), Ok(true)); + shell.variables_mut().set("emptyvar", "".to_string()); + assert_eq!(exists(&["ion".into(), "-s".into(), "emptyvar".into()], &shell), Ok(false)); + shell.variables_mut().set("testvar", "foobar".to_string()); + assert_eq!(exists(&["ion".into(), "-s".into(), "testvar".into()], &shell), Ok(true)); + shell.variables_mut().remove_variable("testvar"); + assert_eq!(exists(&["ion".into(), "-s".into(), "testvar".into()], &shell), Ok(false)); + // also check that it doesn't trigger on arrays + let mut array = types::Array::new(); + array.push("element".into()); + shell.variables_mut().remove_variable("array"); + shell.variables_mut().set("array", array); + assert_eq!(exists(&["ion".into(), "-s".into(), "array".into()], &shell), Ok(false)); + + // check `exists --fn` + let name_str = "test_function"; + let name = small::String::from(name_str); + let mut args = Vec::new(); + args.push(KeyBuf { name: "testy".into(), kind: Primitive::Str }); + let mut statements = Vec::new(); + statements.push(Statement::End); + let description: small::String = "description".into(); + + shell + .variables_mut() + .set(&name, Function::new(Some(description), name.clone(), args, statements)); + + assert_eq!(exists(&["ion".into(), "--fn".into(), name_str.into()], &shell), Ok(true)); + shell.variables_mut().remove_variable(name_str); + assert_eq!(exists(&["ion".into(), "--fn".into(), name_str.into()], &shell), Ok(false)); + + // check invalid flags / parameters (should all be treated as strings and + // therefore succeed) + assert_eq!(exists(&["ion".into(), "--foo".into()], &shell), Ok(true)); + assert_eq!(exists(&["ion".into(), "-x".into()], &shell), Ok(true)); + } -#[test] -fn test_match_flag_argument() { - let shell = shell::Shell::library(); - - // we don't really care about the passed values, as long as both sited return - // the same value - assert_eq!(match_flag_argument('a', "ARRAY", &shell), array_var_is_not_empty("ARRAY", &shell)); - assert_eq!(match_flag_argument('b', "binary", &shell), binary_is_in_path("binary", &shell)); - assert_eq!(match_flag_argument('d', "path", &shell), path_is_directory("path")); - assert_eq!(match_flag_argument('f', "file", &shell), path_is_file("file")); - assert_eq!(match_flag_argument('s', "STR", &shell), string_var_is_not_empty("STR", &shell)); - - // Any flag which is not implemented - assert_eq!(match_flag_argument('x', "ARG", &shell), false); -} + #[test] + fn test_match_flag_argument() { + let shell = Shell::library(); + + // we don't really care about the passed values, as long as both sited return + // the same value + assert_eq!( + match_flag_argument('a', "ARRAY", &shell), + array_var_is_not_empty("ARRAY", &shell) + ); + assert_eq!(match_flag_argument('b', "binary", &shell), binary_is_in_path("binary", &shell)); + assert_eq!(match_flag_argument('d', "path", &shell), path_is_directory("path")); + assert_eq!(match_flag_argument('f', "file", &shell), path_is_file("file")); + assert_eq!(match_flag_argument('s', "STR", &shell), string_var_is_not_empty("STR", &shell)); + + // Any flag which is not implemented + assert_eq!(match_flag_argument('x', "ARG", &shell), false); + } -#[test] -fn test_match_option_argument() { - let shell = shell::Shell::library(); + #[test] + fn test_match_option_argument() { + let shell = Shell::library(); - // we don't really care about the passed values, as long as both sited return - // the same value - assert_eq!(match_option_argument("fn", "FUN", &shell), array_var_is_not_empty("FUN", &shell)); + // we don't really care about the passed values, as long as both sited return + // the same value + assert_eq!( + match_option_argument("fn", "FUN", &shell), + array_var_is_not_empty("FUN", &shell) + ); - // Any option which is not implemented - assert_eq!(match_option_argument("foo", "ARG", &shell), false); -} + // Any option which is not implemented + assert_eq!(match_option_argument("foo", "ARG", &shell), false); + } -#[test] -fn test_path_is_file() { - assert_eq!(path_is_file("testing/empty_file"), true); - assert_eq!(path_is_file("this-does-not-exist"), false); -} + #[test] + fn test_path_is_file() { + assert_eq!(path_is_file("testing/empty_file"), true); + assert_eq!(path_is_file("this-does-not-exist"), false); + } -#[test] -fn test_path_is_directory() { - assert_eq!(path_is_directory("testing"), true); - assert_eq!(path_is_directory("testing/empty_file"), false); -} + #[test] + fn test_path_is_directory() { + assert_eq!(path_is_directory("testing"), true); + assert_eq!(path_is_directory("testing/empty_file"), false); + } -#[test] -fn test_binary_is_in_path() { - let mut shell = shell::Shell::library(); - - // TODO: We should probably also test with more complex PATH-variables: - // TODO: multiple/:directories/ - // TODO: PATH containing directories which do not exist - // TODO: PATH containing directories without read permission (for user) - // TODO: PATH containing directories without execute ("enter") permission (for - // user) TODO: empty PATH? - shell.set("PATH", "testing/".to_string()); - - assert_eq!(binary_is_in_path("executable_file", &shell), true); - assert_eq!(binary_is_in_path("empty_file", &shell), false); - assert_eq!(binary_is_in_path("file_does_not_exist", &shell), false); -} + #[test] + fn test_binary_is_in_path() { + let mut shell = Shell::library(); + + // TODO: We should probably also test with more complex PATH-variables: + // TODO: multiple/:directories/ + // TODO: PATH containing directories which do not exist + // TODO: PATH containing directories without read permission (for user) + // TODO: PATH containing directories without execute ("enter") permission (for + // user) TODO: empty PATH? + shell.variables_mut().set("PATH", "testing/".to_string()); + + assert_eq!(binary_is_in_path("executable_file", &shell), true); + assert_eq!(binary_is_in_path("empty_file", &shell), false); + assert_eq!(binary_is_in_path("file_does_not_exist", &shell), false); + } -#[test] -fn test_file_has_execute_permission() { - assert_eq!(file_has_execute_permission("testing/executable_file"), true); - assert_eq!(file_has_execute_permission("testing"), true); - assert_eq!(file_has_execute_permission("testing/empty_file"), false); - assert_eq!(file_has_execute_permission("this-does-not-exist"), false); -} + #[test] + fn test_file_has_execute_permission() { + assert_eq!(file_has_execute_permission("testing/executable_file"), true); + assert_eq!(file_has_execute_permission("testing"), true); + assert_eq!(file_has_execute_permission("testing/empty_file"), false); + assert_eq!(file_has_execute_permission("this-does-not-exist"), false); + } -#[test] -fn test_string_is_nonzero() { - assert_eq!(string_is_nonzero("NOT ZERO"), true); - assert_eq!(string_is_nonzero(""), false); -} + #[test] + fn test_string_is_nonzero() { + assert_eq!(string_is_nonzero("NOT ZERO"), true); + assert_eq!(string_is_nonzero(""), false); + } -#[test] -fn test_array_var_is_not_empty() { - let mut shell = shell::Shell::library(); + #[test] + fn test_array_var_is_not_empty() { + let mut shell = Shell::library(); - shell.variables_mut().set("EMPTY_ARRAY", types::Array::new()); - assert_eq!(array_var_is_not_empty("EMPTY_ARRAY", &shell), false); + shell.variables_mut().set("EMPTY_ARRAY", types::Array::new()); + assert_eq!(array_var_is_not_empty("EMPTY_ARRAY", &shell), false); - let mut not_empty_array = types::Array::new(); - not_empty_array.push("array not empty".into()); - shell.variables_mut().set("NOT_EMPTY_ARRAY", not_empty_array); - assert_eq!(array_var_is_not_empty("NOT_EMPTY_ARRAY", &shell), true); + let mut not_empty_array = types::Array::new(); + not_empty_array.push("array not empty".into()); + shell.variables_mut().set("NOT_EMPTY_ARRAY", not_empty_array); + assert_eq!(array_var_is_not_empty("NOT_EMPTY_ARRAY", &shell), true); - // test for array which does not even exist - shell.variables_mut().remove_variable("NOT_EMPTY_ARRAY"); - assert_eq!(array_var_is_not_empty("NOT_EMPTY_ARRAY", &shell), false); + // test for array which does not even exist + shell.variables_mut().remove_variable("NOT_EMPTY_ARRAY"); + assert_eq!(array_var_is_not_empty("NOT_EMPTY_ARRAY", &shell), false); - // array_var_is_not_empty should NOT match for non-array variables with the - // same name - shell.set("VARIABLE", "notempty-variable"); - assert_eq!(array_var_is_not_empty("VARIABLE", &shell), false); -} + // array_var_is_not_empty should NOT match for non-array variables with the + // same name + shell.variables_mut().set("VARIABLE", "notempty-variable"); + assert_eq!(array_var_is_not_empty("VARIABLE", &shell), false); + } -#[test] -fn test_string_var_is_not_empty() { - let mut shell = shell::Shell::library(); + #[test] + fn test_string_var_is_not_empty() { + let mut shell = Shell::library(); - shell.set("EMPTY", ""); - assert_eq!(string_var_is_not_empty("EMPTY", &shell), false); + shell.variables_mut().set("EMPTY", ""); + assert_eq!(string_var_is_not_empty("EMPTY", &shell), false); - shell.set("NOT_EMPTY", "notempty"); - assert_eq!(string_var_is_not_empty("NOT_EMPTY", &shell), true); + shell.variables_mut().set("NOT_EMPTY", "notempty"); + assert_eq!(string_var_is_not_empty("NOT_EMPTY", &shell), true); - // string_var_is_not_empty should NOT match for arrays with the same name - let mut array = types::Array::new(); - array.push("not-empty".into()); - shell.variables_mut().set("ARRAY_NOT_EMPTY", array); - assert_eq!(string_var_is_not_empty("ARRAY_NOT_EMPTY", &shell), false); + // string_var_is_not_empty should NOT match for arrays with the same name + let mut array = types::Array::new(); + array.push("not-empty".into()); + shell.variables_mut().set("ARRAY_NOT_EMPTY", array); + assert_eq!(string_var_is_not_empty("ARRAY_NOT_EMPTY", &shell), false); - // test for a variable which does not even exist - shell.variables_mut().remove_variable("NOT_EMPTY"); - assert_eq!(string_var_is_not_empty("NOT_EMPTY", &shell), false); -} + // test for a variable which does not even exist + shell.variables_mut().remove_variable("NOT_EMPTY"); + assert_eq!(string_var_is_not_empty("NOT_EMPTY", &shell), false); + } -#[test] -fn test_function_is_defined() { - use crate::lexers::assignments::{KeyBuf, Primitive}; - let mut shell = shell::Shell::library(); - - // create a simple dummy function - let name_str = "test_function"; - let name: small::String = name_str.into(); - let mut args = Vec::new(); - args.push(KeyBuf { name: "testy".into(), kind: Primitive::Str }); - let mut statements = Vec::new(); - statements.push(Statement::End); - let description: small::String = "description".into(); - - shell - .variables_mut() - .set(&name, Function::new(Some(description), name.clone(), args, statements)); - - assert_eq!(function_is_defined(name_str, &shell), true); - shell.variables_mut().remove_variable(name_str); - assert_eq!(function_is_defined(name_str, &shell), false); + #[test] + fn test_function_is_defined() { + use crate::lexers::assignments::{KeyBuf, Primitive}; + let mut shell = Shell::library(); + + // create a simple dummy function + let name_str = "test_function"; + let name: small::String = name_str.into(); + let mut args = Vec::new(); + args.push(KeyBuf { name: "testy".into(), kind: Primitive::Str }); + let mut statements = Vec::new(); + statements.push(Statement::End); + let description: small::String = "description".into(); + + shell + .variables_mut() + .set(&name, Function::new(Some(description), name.clone(), args, statements)); + + assert_eq!(function_is_defined(name_str, &shell), true); + shell.variables_mut().remove_variable(name_str); + assert_eq!(function_is_defined(name_str, &shell), false); + } } diff --git a/src/lib/builtins/is.rs b/src/lib/builtins/is.rs index c5f94fef..3cc785b6 100644 --- a/src/lib/builtins/is.rs +++ b/src/lib/builtins/is.rs @@ -35,7 +35,7 @@ fn get_var_string(name: &str, shell: &mut Shell) -> types::Str { return "".into(); } - match shell.variables().get::<types::Str>(&name[1..]) { + match shell.variables().get_str(&name[1..]) { Some(s) => s, None => "".into(), } @@ -45,8 +45,8 @@ fn get_var_string(name: &str, shell: &mut Shell) -> types::Str { fn test_is() { fn vec_string(args: &[&str]) -> Vec<small::String> { args.iter().map(|&s| s.into()).collect() } let mut shell = Shell::library(); - shell.set("x", "value"); - shell.set("y", "0"); + shell.variables_mut().set("x", "value"); + shell.variables_mut().set("y", "0"); // Four arguments assert_eq!( diff --git a/src/lib/builtins/mod.rs b/src/lib/builtins/mod.rs index e94042ea..a0ab28ad 100644 --- a/src/lib/builtins/mod.rs +++ b/src/lib/builtins/mod.rs @@ -288,8 +288,7 @@ fn builtin_bool(args: &[small::String], shell: &mut Shell) -> i32 { return FAILURE; } - let opt = - if args[1].is_empty() { None } else { shell.variables().get::<types::Str>(&args[1][1..]) }; + let opt = if args[1].is_empty() { None } else { shell.variables().get_str(&args[1][1..]) }; match opt.as_ref().map(types::Str::as_str) { Some("1") => (), diff --git a/src/lib/builtins/set.rs b/src/lib/builtins/set.rs index 73983095..15115563 100644 --- a/src/lib/builtins/set.rs +++ b/src/lib/builtins/set.rs @@ -59,20 +59,22 @@ pub fn set(args: &[small::String], shell: &mut Shell) -> i32 { match positionals { None => (), Some(kind) => { - let command = shell.variables().get::<types::Array>("args").unwrap()[0].clone(); - // This used to take a `&[String]` but cloned them all, so although - // this is non-ideal and could probably be better done with `Rc`, it - // hasn't got any slower. - let arguments: types::Array = - iter::once(command).chain(args_iter.cloned().map(Value::Str)).collect(); - match kind { - UnsetIfNone => { - shell.variables_mut().set("args", arguments); - } - RetainIfNone => { - if arguments.len() != 1 { + if let Some(Value::Array(array)) = shell.variables().get_ref("args") { + let command = array[0].clone(); + // This used to take a `&[String]` but cloned them all, so although + // this is non-ideal and could probably be better done with `Rc`, it + // hasn't got any slower. + let arguments: types::Array = + iter::once(command).chain(args_iter.cloned().map(Value::Str)).collect(); + match kind { + UnsetIfNone => { shell.variables_mut().set("args", arguments); } + RetainIfNone => { + if arguments.len() != 1 { + shell.variables_mut().set("args", arguments); + } + } } } } diff --git a/src/lib/builtins/variables.rs b/src/lib/builtins/variables.rs index 6bd45835..d03c45b6 100644 --- a/src/lib/builtins/variables.rs +++ b/src/lib/builtins/variables.rs @@ -151,7 +151,7 @@ mod test { struct VariableExpander<'a>(pub Variables<'a>); impl<'a> Expander for VariableExpander<'a> { - fn string(&self, var: &str) -> Option<types::Str> { self.0.get::<types::Str>(var) } + fn string(&self, var: &str) -> Option<types::Str> { self.0.get_str(var) } } // TODO: Rewrite tests now that let is part of the grammar. diff --git a/src/lib/shell/assignments.rs b/src/lib/shell/assignments.rs index 027dffc9..6b5dba35 100644 --- a/src/lib/shell/assignments.rs +++ b/src/lib/shell/assignments.rs @@ -7,7 +7,6 @@ use crate::{ lexers::assignments::{Key, Operator, Primitive}, parser::{assignments::*, is_valid_name}, shell::variables::{EuclDiv, Modifications, OpError, Pow, Value}, - types, }; use std::{ env, @@ -81,7 +80,7 @@ impl<'b> Shell<'b> { SUCCESS } - ExportAction::LocalExport(ref key) => match self.get::<types::Str>(key) { + ExportAction::LocalExport(ref key) => match self.variables.get_str(key) { Some(var) => { env::set_var(key, &*var); SUCCESS diff --git a/src/lib/shell/directory_stack.rs b/src/lib/shell/directory_stack.rs index 0b677ff9..e8d22e47 100644 --- a/src/lib/shell/directory_stack.rs +++ b/src/lib/shell/directory_stack.rs @@ -207,7 +207,11 @@ impl DirectoryStack { /// variable, /// else it will return a default value of 1000. fn get_size(variables: &Variables) -> usize { - variables.get_str_or_empty("DIRECTORY_STACK_SIZE").parse::<usize>().unwrap_or(1000) + variables + .get_str("DIRECTORY_STACK_SIZE") + .unwrap_or_default() + .parse::<usize>() + .unwrap_or(1000) } /// Create a new `DirectoryStack` containing the current working directory, diff --git a/src/lib/shell/flow.rs b/src/lib/shell/flow.rs index b053ad6f..131c0d34 100644 --- a/src/lib/shell/flow.rs +++ b/src/lib/shell/flow.rs @@ -69,7 +69,7 @@ impl<'a> Shell<'a> { ($chunk:expr, $def:expr) => { for (key, value) in variables.iter().zip($chunk.chain(::std::iter::repeat($def))) { if key != "_" { - self.set(key, value.clone()); + self.variables_mut().set(key, value.clone()); } } @@ -91,7 +91,7 @@ impl<'a> Shell<'a> { } ForValueExpression::Normal(value) => { if &variables[0] != "_" { - self.set(&variables[0], value.clone()); + self.variables_mut().set(&variables[0], value.clone()); } match self.execute_statements(statements) { @@ -242,7 +242,7 @@ impl<'a> Shell<'a> { _ => (), } let previous_status = self.previous_status.to_string(); - self.set("?", previous_status); + self.variables_mut().set("?", previous_status); } Statement::Break => return Condition::Break, Statement::Continue => return Condition::Continue, @@ -306,15 +306,21 @@ impl<'a> Shell<'a> { // let pattern_is_array = is_array(&value); let previous_bind = case.binding.as_ref().and_then(|bind| { if is_array { - let out = self.variables.get::<types::Array>(bind).map(Value::Array); - self.set( + let out = if let Some(Value::Array(array)) = + self.variables.get_ref(bind).cloned() + { + Some(Value::Array(array)) + } else { + None + }; + self.variables_mut().set( &bind, value.iter().cloned().map(Value::Str).collect::<types::Array>(), ); out } else { - let out = self.variables.get::<types::Str>(bind).map(Value::Str); - self.set(&bind, value.join(" ")); + let out = self.variables.get_str(bind).map(Value::Str); + self.variables_mut().set(&bind, value.join(" ")); out } }); @@ -332,7 +338,7 @@ impl<'a> Shell<'a> { if let Some(value) = previous_bind { match value { Value::HashMap(_) | Value::Array(_) | Value::Str(_) => { - self.set(bind, value); + self.variables_mut().set(bind, value); } _ => (), } @@ -382,7 +388,7 @@ fn expand_pipeline<'a>( let mut statements = Vec::new(); while let Some(item) = item_iter.next() { - if let Some(alias) = shell.variables.get::<types::Alias>(item.command()) { + if let Some(Value::Alias(alias)) = shell.variables.get_ref(item.command()) { statements = StatementSplitter::new(alias.0.as_str()) .map(|stmt| parse_and_validate(stmt, &shell.builtins)) .collect(); diff --git a/src/lib/shell/fork.rs b/src/lib/shell/fork.rs index 513edfcc..52a24bcc 100644 --- a/src/lib/shell/fork.rs +++ b/src/lib/shell/fork.rs @@ -123,7 +123,7 @@ impl<'a, 'b> Fork<'a, 'b> { // Obtain ownership of the child's copy of the shell, and then configure it. let mut shell: Shell = unsafe { (self.shell as *const Shell).read() }; - shell.set("PID", sys::getpid().unwrap_or(0).to_string()); + shell.variables_mut().set("PID", sys::getpid().unwrap_or(0).to_string()); // Execute the given closure within the child's shell. child_func(&mut shell); diff --git a/src/lib/shell/job.rs b/src/lib/shell/job.rs index ae65eb86..a08d86ad 100644 --- a/src/lib/shell/job.rs +++ b/src/lib/shell/job.rs @@ -2,8 +2,7 @@ use super::Shell; use crate::{ builtins::BuiltinFunction, parser::{pipelines::RedirectFrom, Expander}, - shell::flow_control::Function, - types, + types, Value, }; use std::{fmt, fs::File, str}; @@ -21,8 +20,9 @@ impl<'a> Job<'a> { /// Takes the current job's arguments and expands them, one argument at a /// time, returning a new `Job` with the expanded arguments. pub fn expand(&mut self, shell: &Shell) { - if shell.variables.get::<Function>(&self.args[0]).is_none() { - self.args = self.args.drain().flat_map(|arg| expand_arg(&arg, shell)).collect(); + match shell.variables.get_ref(&self.args[0]) { + Some(Value::Function(_)) => {} + _ => self.args = self.args.drain().flat_map(|arg| expand_arg(&arg, shell)).collect(), } } diff --git a/src/lib/shell/mod.rs b/src/lib/shell/mod.rs index e97779c9..c55234ce 100644 --- a/src/lib/shell/mod.rs +++ b/src/lib/shell/mod.rs @@ -15,12 +15,12 @@ pub mod variables; pub(crate) use self::job::Job; use self::{ directory_stack::DirectoryStack, - flow_control::{Block, Function, FunctionError, Statement}, + flow_control::{Block, FunctionError, Statement}, foreground::ForegroundSignals, fork::{Fork, IonResult}, pipe_exec::{foreground, job_control::BackgroundProcess}, status::*, - variables::{GetVariable, Variables}, + variables::Variables, }; pub use self::{fork::Capture, pipe_exec::job_control::ProcessState, variables::Value}; use crate::{ @@ -265,12 +265,14 @@ impl<'a> Shell<'a> { name: &str, args: &[S], ) -> Result<i32, IonError> { - self.variables.get::<Function>(name).ok_or(IonError::DoesNotExist).and_then(|function| { + if let Some(Value::Function(function)) = self.variables.get_ref(name).cloned() { function .execute(self, args) .map(|_| self.previous_status) .map_err(|err| IonError::Function { why: err }) - }) + } else { + Err(IonError::DoesNotExist) + } } /// A method for executing scripts in the Ion shell without capturing. Given a `Path`, this @@ -316,19 +318,6 @@ impl<'a> Shell<'a> { } } - /// Gets any variable, if it exists within the shell's variable map. - pub fn get<T>(&self, name: &str) -> Option<T> - where - Variables<'a>: GetVariable<T>, - { - self.variables.get::<T>(name) - } - - /// Sets a variable of `name` with the given `value` in the shell's variable map. - pub fn set<T: Into<Value<'a>>>(&mut self, name: &str, value: T) { - self.variables.set(name, value); - } - /// Executes a pipeline and returns the final exit status of the pipeline. pub fn run_pipeline(&mut self, mut pipeline: Pipeline<'a>) -> Option<i32> { let command_start_time = SystemTime::now(); @@ -350,8 +339,8 @@ impl<'a> Shell<'a> { Some(self.execute_pipeline(pipeline)) } // Branch else if -> input == shell function and set the exit_status - } else if let Some(function) = - self.variables.get::<Function>(&pipeline.items[0].job.args[0]) + } else if let Some(Value::Function(function)) = + self.variables.get_ref(&pipeline.items[0].job.args[0]).cloned() { if !pipeline.requires_piping() { match function.execute(self, &pipeline.items[0].job.args) { @@ -384,7 +373,7 @@ impl<'a> Shell<'a> { // Retrieve the exit_status and set the $? variable and history.previous_status if let Some(code) = exit_status { - self.set("?", code.to_string()); + self.variables_mut().set("?", code.to_string()); self.previous_status = code; } diff --git a/src/lib/shell/pipe_exec/mod.rs b/src/lib/shell/pipe_exec/mod.rs index 2de00334..232a0dc4 100644 --- a/src/lib/shell/pipe_exec/mod.rs +++ b/src/lib/shell/pipe_exec/mod.rs @@ -17,11 +17,11 @@ use self::{ streams::{duplicate_streams, redirect_streams}, }; use super::{ - flow_control::{Function, FunctionError}, + flow_control::FunctionError, job::{Job, JobVariant, RefinedJob, TeeItem}, signals::{self, SignalHandler}, status::*, - Shell, + Shell, Value, }; use crate::{ builtins::{self, BuiltinFunction}, @@ -276,19 +276,23 @@ impl<'b> Shell<'b> { } fn exec_function<S: AsRef<str>>(&mut self, name: &str, args: &[S]) -> i32 { - match self.variables.get::<Function>(name).unwrap().execute(self, args) { - Ok(()) => SUCCESS, - Err(FunctionError::InvalidArgumentCount) => { - eprintln!("ion: invalid number of function arguments supplied"); - FAILURE - } - Err(FunctionError::InvalidArgumentType(expected_type, value)) => { - eprintln!( - "ion: function argument has invalid type: expected {}, found value \'{}\'", - expected_type, value - ); - FAILURE + if let Some(Value::Function(function)) = self.variables.get_ref(name).cloned() { + match function.execute(self, args) { + Ok(()) => SUCCESS, + Err(FunctionError::InvalidArgumentCount) => { + eprintln!("ion: invalid number of function arguments supplied"); + FAILURE + } + Err(FunctionError::InvalidArgumentType(expected_type, value)) => { + eprintln!( + "ion: function argument has invalid type: expected {}, found value \'{}\'", + expected_type, value + ); + FAILURE + } } + } else { + unreachable!() } } @@ -340,7 +344,7 @@ impl<'b> Shell<'b> { &builtins::builtin_cd, iter::once("cd".into()).chain(job.args).collect(), ) - } else if self.variables.get::<Function>(&job.args[0]).is_some() { + } else if let Some(Value::Function(_)) = self.variables.get_ref(&job.args[0]) { RefinedJob::function(job.args) } else if let Some(builtin) = job.builtin { RefinedJob::builtin(builtin, job.args) diff --git a/src/lib/shell/shell_expand.rs b/src/lib/shell/shell_expand.rs index 1352cb5f..e327ad5d 100644 --- a/src/lib/shell/shell_expand.rs +++ b/src/lib/shell/shell_expand.rs @@ -38,43 +38,40 @@ impl<'a, 'b> Expander for Shell<'b> { if name == "?" { Some(types::Str::from(self.previous_status.to_string())) } else { - self.get::<types::Str>(name) + self.variables().get_str(name) } } /// Expand an array variable with some selection fn array(&self, name: &str, selection: &Select) -> Option<types::Args> { - if let Some(array) = self.variables.get::<types::Array>(name) { - match selection { + match self.variables.get_ref(name) { + Some(Value::Array(array)) => match selection { Select::All => { - return Some(types::Args::from_iter( - array.iter().map(|x| format!("{}", x).into()), - )) - } - Select::Index(ref id) => { - return id - .resolve(array.len()) - .and_then(|n| array.get(n)) - .map(|x| types::Args::from_iter(Some(format!("{}", x).into()))); + Some(types::Args::from_iter(array.iter().map(|x| format!("{}", x).into()))) } + Select::Index(ref id) => id + .resolve(array.len()) + .and_then(|n| array.get(n)) + .map(|x| args![types::Str::from(format!("{}", x))]), Select::Range(ref range) => { - if let Some((start, length)) = range.bounds(array.len()) { + range.bounds(array.len()).and_then(|(start, length)| { if array.len() > start { - return Some( + Some( array .iter() .skip(start) .take(length) .map(|var| format!("{}", var).into()) .collect(), - ); + ) + } else { + None } - } + }) } - _ => (), - } - } else if let Some(hmap) = self.variables.get::<types::HashMap>(name) { - match selection { + _ => None, + }, + Some(Value::HashMap(hmap)) => match selection { Select::All => { let mut array = types::Args::new(); for (key, value) in hmap.iter() { @@ -90,17 +87,14 @@ impl<'a, 'b> Expander for Shell<'b> { _ => (), } } - return Some(array); + Some(array) } Select::Key(key) => { - return Some(args![format!( - "{}", - hmap.get(&*key).unwrap_or(&Value::Str("".into())) - )]); + Some(args![format!("{}", hmap.get(&*key).unwrap_or(&Value::Str("".into())))]) } Select::Index(index) => { use crate::ranges::Index; - return Some(args![format!( + Some(args![format!( "{}", hmap.get(&types::Str::from( match index { @@ -110,12 +104,11 @@ impl<'a, 'b> Expander for Shell<'b> { .to_string() )) .unwrap_or(&Value::Str("".into())) - )]); + )]) } - _ => (), - } - } else if let Some(bmap) = self.variables.get::<types::BTreeMap>(name) { - match selection { + _ => None, + }, + Some(Value::BTreeMap(bmap)) => match selection { Select::All => { let mut array = types::Args::new(); for (key, value) in bmap.iter() { @@ -131,17 +124,14 @@ impl<'a, 'b> Expander for Shell<'b> { _ => (), } } - return Some(array); + Some(array) } Select::Key(key) => { - return Some(args![format!( - "{}", - bmap.get(&*key).unwrap_or(&Value::Str("".into())) - )]); + Some(args![format!("{}", bmap.get(&*key).unwrap_or(&Value::Str("".into())))]) } Select::Index(index) => { use crate::ranges::Index; - return Some(args![format!( + Some(args![format!( "{}", bmap.get(&types::Str::from( match index { @@ -151,12 +141,12 @@ impl<'a, 'b> Expander for Shell<'b> { .to_string() )) .unwrap_or(&Value::Str("".into())) - )]); + )]) } - _ => (), - } + _ => None, + }, + _ => None, } - None } fn map_keys(&self, name: &str, sel: &Select) -> Option<types::Args> { @@ -195,9 +185,7 @@ impl<'a, 'b> Expander for Shell<'b> { match tilde_prefix { "" => sys_env::home_dir().map(|home| home.to_string_lossy().to_string() + rest), "+" => Some(env::var("PWD").unwrap_or_else(|_| "?".to_string()) + rest), - "-" => { - self.variables.get::<types::Str>("OLDPWD").map(|oldpwd| oldpwd.to_string() + rest) - } + "-" => self.variables.get_str("OLDPWD").map(|oldpwd| oldpwd.to_string() + rest), _ => { let (neg, tilde_num) = if tilde_prefix.starts_with('+') { (false, &tilde_prefix[1..]) diff --git a/src/lib/shell/variables/mod.rs b/src/lib/shell/variables/mod.rs index 9b141d1a..9c610e94 100644 --- a/src/lib/shell/variables/mod.rs +++ b/src/lib/shell/variables/mod.rs @@ -81,26 +81,6 @@ type_from_value!(Function<'a> : Function else ) ); -macro_rules! eq { - ($lhs:ty : $variant:ident) => { - impl<'a> PartialEq<Value<'a>> for $lhs { - fn eq(&self, other: &Value<'a>) -> bool { - match other { - Value::$variant(ref inner) => inner == self, - _ => false, - } - } - } - }; -} - -eq!(types::Str: Str); -eq!(types::Alias: Alias); -eq!(types::Array<'a>: Array); -eq!(types::HashMap<'a>: HashMap); -eq!(types::BTreeMap<'a>: BTreeMap); -eq!(Function<'a>: Function); - impl<'a> Eq for Value<'a> {} // this one’s only special because of the lifetime parameter @@ -257,7 +237,7 @@ impl<'a> Variables<'a> { /// Useful for getting smaller prompts, this will produce a simplified variant of the /// working directory which the leading `HOME` prefix replaced with a tilde character. fn get_simplified_directory(&self) -> types::Str { - let home = self.get::<types::Str>("HOME").unwrap_or_else(|| "?".into()); + let home = self.get_str("HOME").unwrap_or_else(|| "?".into()); env::var("PWD").unwrap().replace(&*home, "~").into() } @@ -269,10 +249,6 @@ impl<'a> Variables<'a> { c.is_alphanumeric() || c == '_' || c == '?' || c == '.' || c == '-' || c == '+' } - pub fn get_str_or_empty(&self, name: &str) -> types::Str { - self.get::<types::Str>(name).unwrap_or_default() - } - pub fn remove_variable(&mut self, name: &str) -> Option<Value<'a>> { if name.starts_with("super::") || name.starts_with("global::") { // Cannot mutate outer namespace @@ -289,6 +265,42 @@ impl<'a> Variables<'a> { self.0.get_mut(name) } + pub fn get_str(&self, name: &str) -> Option<types::Str> { + match name { + "MWD" => return Some(self.get_minimal_directory()), + "SWD" => return Some(self.get_simplified_directory()), + _ => (), + } + // If the parsed name contains the '::' pattern, then a namespace was + // designated. Find it. + match name.find("::").map(|pos| (&name[..pos], &name[pos + 2..])) { + Some(("c", variable)) | Some(("color", variable)) => { + Colors::collect(variable).into_string().map(|s| s.into()) + } + Some(("x", variable)) | Some(("hex", variable)) => { + match u8::from_str_radix(variable, 16) { + Ok(c) => Some((c as char).to_string().into()), + Err(why) => { + eprintln!("ion: hex parse error: {}: {}", variable, why); + None + } + } + } + Some(("env", variable)) => env::var(variable).map(Into::into).ok().map(|s| s), + Some(("super", _)) | Some(("global", _)) | None => { + // Otherwise, it's just a simple variable name. + match self.get_ref(name) { + Some(Value::Str(val)) => Some(val.clone()), + _ => env::var(name).ok().map(|s| s.into()), + } + } + Some((..)) => { + eprintln!("ion: unsupported namespace: '{}'", name); + None + } + } + } + pub fn get_ref(&self, mut name: &str) -> Option<&Value<'a>> { const GLOBAL_NS: &str = "global::"; const SUPER_NS: &str = "super::"; @@ -310,13 +322,6 @@ impl<'a> Variables<'a> { }; self.0.get_ref(name, namespace) } - - pub fn get<T>(&self, name: &str) -> Option<T> - where - Self: GetVariable<T>, - { - GetVariable::<T>::get(self, name) - } } impl<'a> Default for Variables<'a> { @@ -365,71 +370,6 @@ impl<'a> Default for Variables<'a> { } } -pub trait GetVariable<T> { - fn get(&self, name: &str) -> Option<T>; -} - -impl<'a> GetVariable<types::Str> for Variables<'a> { - fn get(&self, name: &str) -> Option<types::Str> { - use crate::types::Str; - - match name { - "MWD" => return Some(Str::from(Value::Str(self.get_minimal_directory()))), - "SWD" => return Some(Str::from(Value::Str(self.get_simplified_directory()))), - _ => (), - } - // If the parsed name contains the '::' pattern, then a namespace was - // designated. Find it. - match name.find("::").map(|pos| (&name[..pos], &name[pos + 2..])) { - Some(("c", variable)) | Some(("color", variable)) => { - Colors::collect(variable).into_string().map(|s| Str::from(Value::Str(s.into()))) - } - Some(("x", variable)) | Some(("hex", variable)) => { - match u8::from_str_radix(variable, 16) { - Ok(c) => Some(Str::from(Value::Str((c as char).to_string().into()))), - Err(why) => { - eprintln!("ion: hex parse error: {}: {}", variable, why); - None - } - } - } - Some(("env", variable)) => { - env::var(variable).map(Into::into).ok().map(|s| Str::from(Value::Str(s))) - } - Some(("super", _)) | Some(("global", _)) | None => { - // Otherwise, it's just a simple variable name. - match self.get_ref(name) { - Some(Value::Str(val)) => Some(Str::from(Value::Str(val.clone()))), - _ => env::var(name).ok().map(|s| Str::from(Value::Str(s.into()))), - } - } - Some((..)) => { - eprintln!("ion: unsupported namespace: '{}'", name); - None - } - } - } -} - -macro_rules! get_var { - ($types:ty, $variant:ident($inner:ident) => $ret:expr) => { - impl<'a> GetVariable<$types> for Variables<'a> { - fn get(&self, name: &str) -> Option<$types> { - match self.get_ref(name) { - Some(Value::$variant($inner)) => Some($ret.clone()), - _ => None, - } - } - } - }; -} - -get_var!(types::Alias, Alias(alias) => (*alias)); -get_var!(types::Array<'a>, Array(array) => array); -get_var!(types::HashMap<'a>, HashMap(hmap) => hmap); -get_var!(types::BTreeMap<'a>, BTreeMap(bmap) => bmap); -get_var!(Function<'a>, Function(func) => func); - #[cfg(test)] mod trait_test; @@ -442,7 +382,7 @@ mod tests { struct VariableExpander<'a>(pub Variables<'a>); impl<'a> Expander for VariableExpander<'a> { - fn string(&self, var: &str) -> Option<types::Str> { self.0.get::<types::Str>(var) } + fn string(&self, var: &str) -> Option<types::Str> { self.0.get_str(var) } } #[test] @@ -467,7 +407,7 @@ mod tests { env::set_var("PWD", "/var/log/nix"); assert_eq!( types::Str::from("v/l/nix"), - variables.get::<types::Str>("MWD").expect("no value returned"), + variables.get_str("MWD").expect("no value returned"), ); } @@ -478,7 +418,7 @@ mod tests { env::set_var("PWD", "/var/log"); assert_eq!( types::Str::from("/var/log"), - variables.get::<types::Str>("MWD").expect("no value returned"), + variables.get_str("MWD").expect("no value returned"), ); } } -- GitLab