From 8da5bb424f593486b28c3f3638b536b8d184f81f Mon Sep 17 00:00:00 2001
From: Xavier L'Heureux <xavier.lheureux@icloud.com>
Date: Tue, 25 Jun 2019 10:28:05 -0400
Subject: [PATCH] Remove more thin wrappers
---
src/binary/history.rs | 2 +-
src/binary/lexer.rs | 2 +-
src/lib/builtins/helpers.rs | 16 +++--
src/lib/builtins/mod.rs | 39 +----------
src/lib/builtins/test.rs | 49 +++++++-------
src/lib/builtins/variables.rs | 89 +++++++++++++-------------
src/lib/expansion/words/mod.rs | 2 +-
src/lib/shell/mod.rs | 4 +-
src/lib/shell/pipe_exec/foreground.rs | 4 +-
src/lib/shell/pipe_exec/job_control.rs | 4 +-
src/lib/shell/shell_expand.rs | 16 +++--
11 files changed, 103 insertions(+), 124 deletions(-)
diff --git a/src/binary/history.rs b/src/binary/history.rs
index 016d504d..27a78654 100644
--- a/src/binary/history.rs
+++ b/src/binary/history.rs
@@ -31,7 +31,7 @@ impl<'a> InteractiveBinary<'a> {
let mut settings = IgnoreSetting::default();
// for convenience and to avoid typos
let regex_prefix = "regex:";
- for pattern in patterns.into_iter() {
+ for pattern in patterns.iter() {
let pattern = format!("{}", pattern);
match pattern.as_ref() {
"all" => settings.all = true,
diff --git a/src/binary/lexer.rs b/src/binary/lexer.rs
index 7bd470cf..e6707ea1 100644
--- a/src/binary/lexer.rs
+++ b/src/binary/lexer.rs
@@ -52,7 +52,7 @@ impl<'a> Iterator for DesignatorLexer<'a> {
return Some(DesignatorToken::Text(self.grab_and_shorten(id)));
}
}
- b' ' | b'\t' | b'\'' | b'"' | b'a'...b'z' | b'A'...b'Z' if self.design => {
+ b' ' | b'\t' | b'\'' | b'"' | b'a'..=b'z' | b'A'..=b'Z' if self.design => {
self.design = false;
return Some(DesignatorToken::Designator(self.grab_and_shorten(id)));
}
diff --git a/src/lib/builtins/helpers.rs b/src/lib/builtins/helpers.rs
index f50fdd94..87ecd71d 100644
--- a/src/lib/builtins/helpers.rs
+++ b/src/lib/builtins/helpers.rs
@@ -11,12 +11,10 @@ impl Status {
pub const TERMINATED: Self = Status(143);
pub const TRUE: Self = Status(0);
- pub fn from_signal(signal: i32) -> Self { Status(128 + signal) }
+ pub fn from_signal(signal: u8) -> Self { Status(i32::from(128 + signal)) }
pub fn from_exit_code(code: i32) -> Self { Status(code) }
- pub fn from_bool(b: bool) -> Self { Status(!b as i32) }
-
pub fn error<T: AsRef<str>>(err: T) -> Self {
let err = err.as_ref();
if !err.is_empty() {
@@ -37,7 +35,7 @@ impl Status {
pub fn is_failure(self) -> bool { self.0 != 0 }
- pub fn as_os_code(self) -> i32 { self.0 }
+ pub fn as_os_code(self) -> i32 { self.0 as i32 }
pub fn toggle(&mut self) { self.0 = if self.is_success() { 1 } else { 0 }; }
}
@@ -58,3 +56,13 @@ impl From<std::io::Result<()>> for Status {
}
}
}
+
+impl From<bool> for Status {
+ fn from(success: bool) -> Self {
+ if success {
+ Self::TRUE
+ } else {
+ Self::FALSE
+ }
+ }
+}
diff --git a/src/lib/builtins/mod.rs b/src/lib/builtins/mod.rs
index 25c872a8..285b5d56 100644
--- a/src/lib/builtins/mod.rs
+++ b/src/lib/builtins/mod.rs
@@ -28,8 +28,8 @@ pub use self::{
set::builtin_set,
source::builtin_source,
status::builtin_status,
- test::test,
- variables::{builtin_alias, builtin_unalias, drop_array, drop_variable},
+ test::builtin_test,
+ variables::{builtin_alias, builtin_drop, builtin_unalias},
};
use crate as ion_shell;
use crate::{
@@ -599,29 +599,6 @@ pub fn read(args: &[types::Str], shell: &mut Shell<'_>) -> Status {
Status::SUCCESS
}
-#[builtin(
- desc = "delete some variables or arrays",
- man = "
-SYNOPSIS
- drop [ -a ] VARIABLES...
-
-DESCRIPTION
- Deletes the variables given to it as arguments. The variables name must be supplied.
- Instead of '$x' use 'x'.
-
-OPTIONS
- -a
- Instead of deleting variables deletes arrays.
-"
-)]
-pub fn drop(args: &[types::Str], shell: &mut Shell<'_>) -> Status {
- if args.len() >= 2 && args[1] == "-a" {
- drop_array(shell.variables_mut(), args)
- } else {
- drop_variable(shell.variables_mut(), args)
- }
-}
-
#[builtin(
desc = "evaluates the specified commands",
man = "
@@ -638,16 +615,6 @@ pub fn eval(args: &[types::Str], shell: &mut Shell<'_>) -> Status {
})
}
-pub fn builtin_test(args: &[types::Str], _: &mut Shell<'_>) -> Status {
- // Do not use `check_help` for the `test` builtin. The
- // `test` builtin contains a "-h" option.
- match test(args) {
- Ok(true) => Status::TRUE,
- Ok(false) => Status::FALSE,
- Err(why) => Status::error(why),
- }
-}
-
// TODO create manpage.
pub fn builtin_calc(args: &[types::Str], _: &mut Shell<'_>) -> Status {
match calc::calc(&args[1..]) {
@@ -776,7 +743,7 @@ pub fn builtin_help(args: &[types::Str], shell: &mut Shell<'_>) -> Status {
println!("Command helper not found [run 'help']...");
}
} else {
- println!("{}", shell.builtins().keys().format(""));
+ println!("{}", shell.builtins().keys().format("\n"));
}
Status::SUCCESS
}
diff --git a/src/lib/builtins/test.rs b/src/lib/builtins/test.rs
index e283fa74..e01bc53d 100644
--- a/src/lib/builtins/test.rs
+++ b/src/lib/builtins/test.rs
@@ -1,14 +1,20 @@
-use small;
use std::{
fs,
os::unix::fs::{FileTypeExt, MetadataExt, PermissionsExt},
path::Path,
time::SystemTime,
};
+use crate as ion_shell;
+use builtins_proc::builtin;
+use super::Status;
+use crate::{types, Shell};
-pub const MAN_TEST: &str = r#"NAME
- test - perform tests on files and text
+const QUICK_GUIDE: &str = r#"Usage: test [EXPRESSION]
+Try 'test --help' for more information."#;
+#[builtin(
+ desc = "perform tests on files and text",
+ man = r#"
SYNOPSIS
test [EXPRESSION]
@@ -111,18 +117,17 @@ EXAMPLES
test $(getconf LONG_BIT) = 64 && echo "64-bit OS" || echo "32-bit OS"
AUTHOR
- Written by Michael Murphy.
-"#;
-
-const QUICK_GUIDE: &str = r#"Usage: test [EXPRESSION]
-Try 'test --help' for more information."#;
-
-pub fn test(args: &[small::String]) -> Result<bool, small::String> {
- let arguments = &args[1..];
- evaluate_arguments(arguments)
+ Written by Michael Murphy."#
+)]
+pub fn test(args: &[types::Str], _: &mut Shell<'_>) -> Status {
+ match evaluate_arguments(&args[1..]) {
+ Ok(true) => Status::TRUE,
+ Ok(false) => Status::FALSE,
+ Err(why) => Status::error(why),
+ }
}
-fn evaluate_arguments(arguments: &[small::String]) -> Result<bool, small::String> {
+fn evaluate_arguments(arguments: &[types::Str]) -> Result<bool, types::Str> {
match arguments.first() {
Some(ref s) if s.starts_with('-') && s[1..].starts_with(char::is_alphabetic) => {
// Access the second character in the flag string: this will be type of the
@@ -136,19 +141,13 @@ fn evaluate_arguments(arguments: &[small::String]) -> Result<bool, small::String
})
})
}
- Some(ref s) if *s == "--help" => {
- // "--help" only makes sense if it is the first option. Only look for it
- // in the first position.
- println!("{}", MAN_TEST);
- Ok(true)
- }
Some(arg) => {
// If there is no operator, check if the first argument is non-zero
arguments.get(1).map_or(Ok(string_is_nonzero(arg)), |operator| {
// If there is no right hand argument, a condition was expected
let right_arg = arguments
.get(2)
- .ok_or_else(|| small::String::from("parse error: condition expected"))?;
+ .ok_or_else(|| types::Str::from("parse error: condition expected"))?;
evaluate_expression(arg, operator, right_arg)
})
}
@@ -159,7 +158,7 @@ fn evaluate_arguments(arguments: &[small::String]) -> Result<bool, small::String
}
}
-fn evaluate_expression(first: &str, operator: &str, second: &str) -> Result<bool, small::String> {
+fn evaluate_expression(first: &str, operator: &str, second: &str) -> Result<bool, types::Str> {
match operator {
"=" | "==" => Ok(first == second),
"!=" => Ok(first != second),
@@ -223,8 +222,8 @@ fn get_modified_file_time(filename: &str) -> Option<SystemTime> {
fn parse_integers(
left: &str,
right: &str,
-) -> Result<(Option<isize>, Option<isize>), small::String> {
- let parse_integer = |input: &str| -> Result<Option<isize>, small::String> {
+) -> Result<(Option<isize>, Option<isize>), types::Str> {
+ let parse_integer = |input: &str| -> Result<Option<isize>, types::Str> {
match input
.parse::<isize>()
.map_err(|_| format!("test: integer expression expected: {:?}", input))
@@ -375,14 +374,14 @@ fn test_strings() {
#[test]
fn test_empty_str() {
- let eval = |args: Vec<small::String>| evaluate_arguments(&args);
+ let eval = |args: Vec<types::Str>| evaluate_arguments(&args);
assert_eq!(eval(vec!["".into()]), Ok(false));
assert_eq!(eval(vec!["c".into(), "=".into(), "".into()]), Ok(false));
}
#[test]
fn test_integers_arguments() {
- fn vec_string(args: &[&str]) -> Vec<small::String> {
+ fn vec_string(args: &[&str]) -> Vec<types::Str> {
args.iter().map(|s| (*s).into()).collect()
}
// Equal To
diff --git a/src/lib/builtins/variables.rs b/src/lib/builtins/variables.rs
index 82fa484b..5ebb04cd 100644
--- a/src/lib/builtins/variables.rs
+++ b/src/lib/builtins/variables.rs
@@ -3,7 +3,9 @@
use std::io::{self, Write};
use super::Status;
+use crate as ion_shell;
use crate::{shell::variables::Variables, types, Shell};
+use builtins_proc::builtin;
fn print_list(vars: &Variables<'_>) {
let stdout = io::stdout();
@@ -92,33 +94,26 @@ pub fn builtin_unalias(args: &[types::Str], shell: &mut Shell<'_>) -> Status {
Status::SUCCESS
}
-/// Dropping an array will erase it from the shell.
-pub fn drop_array<S: AsRef<str>>(vars: &mut Variables<'_>, args: &[S]) -> Status {
- if args.len() <= 2 {
- return Status::error("ion: you must specify an array name".to_string());
- }
-
- if args[1].as_ref() != "-a" {
- return Status::error("ion: drop_array must be used with -a option".to_string());
- }
-
- for array in args.iter().skip(2) {
- if vars.remove(array.as_ref()).is_none() {
- return Status::error(format!("ion: undefined array: {}", array.as_ref()));
- }
- }
- Status::SUCCESS
-}
-
+#[builtin(
+ desc = "delete some variables or arrays",
+ man = "
+SYNOPSIS
+ drop VARIABLES...
+
+DESCRIPTION
+ Deletes the variables given to it as arguments. The variables name must be supplied.
+ Instead of '$x' use 'x'.
+"
+)]
/// Dropping a variable will erase it from the shell.
-pub fn drop_variable<S: AsRef<str>>(vars: &mut Variables<'_>, args: &[S]) -> Status {
+pub fn drop(args: &[types::Str], shell: &mut Shell<'_>) -> Status {
if args.len() <= 1 {
return Status::error("ion: you must specify a variable name".to_string());
}
for variable in args.iter().skip(1) {
- if vars.remove(variable.as_ref()).is_none() {
- return Status::error(format!("ion: undefined variable: {}", variable.as_ref()));
+ if shell.variables_mut().remove(variable.as_ref()).is_none() {
+ return Status::error(format!("ion: undefined variable: {}", variable));
}
}
@@ -128,12 +123,16 @@ pub fn drop_variable<S: AsRef<str>>(vars: &mut Variables<'_>, args: &[S]) -> Sta
#[cfg(test)]
mod test {
use super::*;
- use crate::{expansion::Expander, shell::variables::tests::VariableExpander};
+ use crate::{expansion::Expander};
+
+ fn vec_string(args: &[&str]) -> Vec<types::Str> {
+ args.iter().map(|s| (*s).into()).collect()
+ }
// TODO: Rewrite tests now that let is part of the grammar.
// #[test]
// fn let_and_expand_a_variable() {
- // let mut variables = Variables::default();
+ // let mut shell = Shell::default();
// let dir_stack = new_dir_stack();
// let_(&mut variables, vec!["let", "FOO", "=", "BAR"]);
// let expanded = expand_string("$FOO", &variables, &dir_stack,
@@ -142,61 +141,61 @@ mod test {
//
// #[test]
// fn let_fails_if_no_value() {
- // let mut variables = Variables::default();
+ // let mut shell = Shell::default();
// let return_status = let_(&mut variables, vec!["let", "FOO"]);
// assert_eq!(FAILURE, return_status);
// }
//
// #[test]
// fn let_checks_variable_name() {
- // let mut variables = Variables::default();
+ // let mut shell = Shell::default();
// let return_status = let_(&mut variables, vec!["let", ",;!:", "=",
// "FOO"]); assert_eq!(FAILURE, return_status);
// }
#[test]
fn drop_deletes_variable() {
- let mut variables = Variables::default();
- variables.set("FOO", "BAR");
- let return_status = drop_variable(&mut variables, &["drop", "FOO"]);
+ let mut shell = Shell::default();
+ shell.variables_mut().set("FOO", "BAR");
+ let return_status = builtin_drop(&vec_string(&["drop", "FOO"]), &mut shell);
assert!(return_status.is_success());
- assert!(VariableExpander(variables).expand_string("$FOO").is_err());
+ assert!(shell.expand_string("$FOO").is_err());
}
#[test]
fn drop_fails_with_no_arguments() {
- let mut variables = Variables::default();
- let return_status = drop_variable(&mut variables, &["drop"]);
- assert!(!return_status.is_success());
+ let mut shell = Shell::default();
+ let return_status = builtin_drop(&vec_string(&["drop"]), &mut shell);
+ assert!(return_status.is_failure());
}
#[test]
fn drop_fails_with_undefined_variable() {
- let mut variables = Variables::default();
- let return_status = drop_variable(&mut variables, &["drop", "FOO"]);
- assert!(!return_status.is_success());
+ let mut shell = Shell::default();
+ let return_status = builtin_drop(&vec_string(&["drop", "FOO"]), &mut shell);
+ assert!(return_status.is_failure());
}
#[test]
fn drop_deletes_array() {
- let mut variables = Variables::default();
- variables.set("FOO", types_rs::array!["BAR"]);
- let return_status = drop_array(&mut variables, &["drop", "-a", "FOO"]);
+ let mut shell = Shell::default();
+ shell.variables_mut().set("FOO", types_rs::array!["BAR"]);
+ let return_status = builtin_drop(&vec_string(&["drop", "FOO"]), &mut shell);
assert_eq!(Status::SUCCESS, return_status);
- assert!(VariableExpander(variables).expand_string("@FOO").is_err());
+ assert!(shell.expand_string("@FOO").is_err());
}
#[test]
fn drop_array_fails_with_no_arguments() {
- let mut variables = Variables::default();
- let return_status = drop_array(&mut variables, &["drop", "-a"]);
- assert!(!return_status.is_success());
+ let mut shell = Shell::default();
+ let return_status = builtin_drop(&vec_string(&["drop"]), &mut shell);
+ assert!(return_status.is_failure());
}
#[test]
fn drop_array_fails_with_undefined_array() {
- let mut variables = Variables::default();
- let return_status = drop_array(&mut variables, &["drop", "FOO"]);
- assert!(!return_status.is_success());
+ let mut shell = Shell::default();
+ let return_status = builtin_drop(&vec_string(&["drop", "FOO"]), &mut shell);
+ assert!(return_status.is_failure());
}
}
diff --git a/src/lib/expansion/words/mod.rs b/src/lib/expansion/words/mod.rs
index a13e49e4..1253145c 100644
--- a/src/lib/expansion/words/mod.rs
+++ b/src/lib/expansion/words/mod.rs
@@ -496,7 +496,7 @@ impl<'a, E: Expander + 'a> WordIterator<'a, E> {
self.read += 1;
return value
.parse::<Select<types::Str>>()
- .map_err(|_| ExpansionError::IndexParsingError(value.into()));
+ .map_err(|_| ExpansionError::IndexParsingError(value));
}
self.read += 1;
}
diff --git a/src/lib/shell/mod.rs b/src/lib/shell/mod.rs
index 91f5179c..0f14dfc6 100644
--- a/src/lib/shell/mod.rs
+++ b/src/lib/shell/mod.rs
@@ -314,7 +314,7 @@ impl<'a> Shell<'a> {
if let Some(block) = self.flow_control.last().map(Statement::to_string) {
self.previous_status = Status::from_exit_code(1);
- Err(IonError::UnclosedBlock(block.into()))
+ Err(IonError::UnclosedBlock(block))
} else {
Ok(self.previous_status)
}
@@ -360,7 +360,7 @@ impl<'a> Shell<'a> {
self.command_not_found(&command);
Status::COULD_NOT_EXEC
}
- Err(err) => return Err(err.into()),
+ Err(err) => return Err(err),
};
if let Some(ref callback) = self.on_command {
diff --git a/src/lib/shell/pipe_exec/foreground.rs b/src/lib/shell/pipe_exec/foreground.rs
index 463eec47..fe9871c7 100644
--- a/src/lib/shell/pipe_exec/foreground.rs
+++ b/src/lib/shell/pipe_exec/foreground.rs
@@ -6,7 +6,7 @@ use std::sync::atomic::{AtomicUsize, Ordering};
#[derive(Debug)]
pub enum BackgroundResult {
Errored,
- Status(u8),
+ Status(i32),
}
const REPLIED: u8 = 1;
@@ -32,7 +32,7 @@ impl ForegroundSignals {
if reply == ERRORED {
Some(BackgroundResult::Errored)
} else if reply == REPLIED {
- Some(BackgroundResult::Status(self.status.load(Ordering::SeqCst) as u8))
+ Some(BackgroundResult::Status(self.status.load(Ordering::SeqCst) as i32))
} else {
None
}
diff --git a/src/lib/shell/pipe_exec/job_control.rs b/src/lib/shell/pipe_exec/job_control.rs
index dcc1f66b..924dd81c 100644
--- a/src/lib/shell/pipe_exec/job_control.rs
+++ b/src/lib/shell/pipe_exec/job_control.rs
@@ -305,9 +305,7 @@ impl<'a> Shell<'a> {
// signal, the status of that process will be communicated back. To
// avoid consuming CPU cycles, we wait 25 ms between polls.
match self.foreground_signals.was_processed() {
- Some(BackgroundResult::Status(stat)) => {
- break Status::from_exit_code(i32::from(stat))
- }
+ Some(BackgroundResult::Status(stat)) => break Status::from_exit_code(stat),
Some(BackgroundResult::Errored) => break Status::TERMINATED,
None => sleep(Duration::from_millis(25)),
}
diff --git a/src/lib/shell/shell_expand.rs b/src/lib/shell/shell_expand.rs
index 8c9f7d43..4402a4a3 100644
--- a/src/lib/shell/shell_expand.rs
+++ b/src/lib/shell/shell_expand.rs
@@ -167,11 +167,15 @@ impl<'a, 'b> Expander for Shell<'b> {
match self.variables.get(name) {
Some(&Value::HashMap(ref map)) => {
Self::select(map.keys().map(|x| format!("{}", x).into()), sel, map.len())
- .ok_or(ExpansionError::InvalidIndex(sel.clone(), "map-like", name.into()))
+ .ok_or_else(|| {
+ ExpansionError::InvalidIndex(sel.clone(), "map-like", name.into())
+ })
}
Some(&Value::BTreeMap(ref map)) => {
Self::select(map.keys().map(|x| format!("{}", x).into()), sel, map.len())
- .ok_or(ExpansionError::InvalidIndex(sel.clone(), "map-like", name.into()))
+ .ok_or_else(|| {
+ ExpansionError::InvalidIndex(sel.clone(), "map-like", name.into())
+ })
}
Some(_) => Err(ExpansionError::NotAMap(name.into())),
None => Err(ExpansionError::VarNotFound),
@@ -186,11 +190,15 @@ impl<'a, 'b> Expander for Shell<'b> {
match self.variables.get(name) {
Some(&Value::HashMap(ref map)) => {
Self::select(map.values().map(|x| format!("{}", x).into()), sel, map.len())
- .ok_or(ExpansionError::InvalidIndex(sel.clone(), "map-like", name.into()))
+ .ok_or_else(|| {
+ ExpansionError::InvalidIndex(sel.clone(), "map-like", name.into())
+ })
}
Some(&Value::BTreeMap(ref map)) => {
Self::select(map.values().map(|x| format!("{}", x).into()), sel, map.len())
- .ok_or(ExpansionError::InvalidIndex(sel.clone(), "map-like", name.into()))
+ .ok_or_else(|| {
+ ExpansionError::InvalidIndex(sel.clone(), "map-like", name.into())
+ })
}
Some(_) => Err(ExpansionError::NotAMap(name.into())),
None => Err(ExpansionError::VarNotFound),
--
GitLab