From 3b36bfba68ea5a4ed123203a90c378114f7301d3 Mon Sep 17 00:00:00 2001
From: stratact <stratact1@gmail.com>
Date: Fri, 22 Mar 2019 16:23:25 +0000
Subject: [PATCH] Simplify the assignments code
---
Cargo.lock | 11 +-
Cargo.toml | 2 +-
src/lib/shell/assignments.rs | 244 +++++++++--------------------------
src/lib/shell/mod.rs | 124 +++++++++++++++++-
4 files changed, 186 insertions(+), 195 deletions(-)
diff --git a/Cargo.lock b/Cargo.lock
index eb0b4264..5fecace3 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -341,7 +341,7 @@ dependencies = [
"ion_builtins 0.1.0",
"ion_lexers 0.1.0",
"ion_sys 0.1.0",
- "itertools 0.7.11 (registry+https://github.com/rust-lang/crates.io-index)",
+ "itertools 0.8.0 (registry+https://github.com/rust-lang/crates.io-index)",
"itoa 0.4.3 (registry+https://github.com/rust-lang/crates.io-index)",
"lazy_static 1.3.0 (registry+https://github.com/rust-lang/crates.io-index)",
"liner 0.4.5 (git+https://gitlab.redox-os.org/redox-os/liner)",
@@ -391,14 +391,6 @@ dependencies = [
"users 0.8.1 (registry+https://github.com/rust-lang/crates.io-index)",
]
-[[package]]
-name = "itertools"
-version = "0.7.11"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-dependencies = [
- "either 1.5.1 (registry+https://github.com/rust-lang/crates.io-index)",
-]
-
[[package]]
name = "itertools"
version = "0.8.0"
@@ -931,7 +923,6 @@ dependencies = [
"checksum getopts 0.2.18 (registry+https://github.com/rust-lang/crates.io-index)" = "0a7292d30132fb5424b354f5dc02512a86e4c516fe544bb7a25e7f266951b797"
"checksum glob 0.2.11 (registry+https://github.com/rust-lang/crates.io-index)" = "8be18de09a56b60ed0edf84bc9df007e30040691af7acd1c41874faac5895bfb"
"checksum hashbrown 0.1.8 (registry+https://github.com/rust-lang/crates.io-index)" = "3bae29b6653b3412c2e71e9d486db9f9df5d701941d86683005efb9f2d28e3da"
-"checksum itertools 0.7.11 (registry+https://github.com/rust-lang/crates.io-index)" = "0d47946d458e94a1b7bcabbf6521ea7c037062c81f534615abcad76e84d4970d"
"checksum itertools 0.8.0 (registry+https://github.com/rust-lang/crates.io-index)" = "5b8467d9c1cebe26feb08c640139247fac215782d35371ade9a2136ed6085358"
"checksum itoa 0.4.3 (registry+https://github.com/rust-lang/crates.io-index)" = "1306f3464951f30e30d12373d31c79fbd52d236e5e896fd92f96ec7babbbe60b"
"checksum lazy_static 1.3.0 (registry+https://github.com/rust-lang/crates.io-index)" = "bc5729f27f159ddd61f4df6228e827e86643d4d3e7c32183cb30a1c08f604a14"
diff --git a/Cargo.toml b/Cargo.toml
index 693a1387..c520cddd 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -61,7 +61,7 @@ ion_lexers = { path = "members/lexers" }
ion_sys = { path = "members/sys" }
ion-ranges = { path = "members/ranges" }
hashbrown = "0.1.2"
-itertools = "0.7.9"
+itertools = "0.8"
getopts = "0.2"
[lib]
diff --git a/src/lib/shell/assignments.rs b/src/lib/shell/assignments.rs
index 048df1e8..3868ef98 100644
--- a/src/lib/shell/assignments.rs
+++ b/src/lib/shell/assignments.rs
@@ -9,11 +9,8 @@ use crate::{
shell::{history::ShellHistory, variables::Value},
types,
};
-use hashbrown::HashMap;
-
use std::{
env,
- ffi::OsString,
fmt::{self, Display},
io::{self, BufWriter, Write},
result::Result,
@@ -52,11 +49,10 @@ pub(crate) trait VariableStore {
/// Export a variable to the process environment given a binding
fn export(&mut self, action: ExportAction) -> i32;
/// Collect all updates to perform on variables for a given assignement action
- fn create_patch<'a>(
+ fn calculate<'a>(
&mut self,
actions: AssignmentActions<'a>,
- ) -> Result<HashMap<Key<'a>, Value>, String>;
- fn patch(&mut self, patch: HashMap<Key, Value>) -> Result<(), String>;
+ ) -> Result<Vec<(Key<'a>, Value)>, String>;
}
impl VariableStore for Shell {
@@ -117,185 +113,62 @@ impl VariableStore for Shell {
SUCCESS
}
- fn create_patch<'a>(
+ fn calculate<'a>(
&mut self,
actions: AssignmentActions<'a>,
- ) -> Result<HashMap<Key<'a>, Value>, String> {
- let mut patch = HashMap::new();
- actions
- .map(|act| act.map_err(|e| e.to_string()))
- .map(|action| {
- action
- .and_then(|act| {
- // sanitize variable names
- if ["HOME", "HOST", "PWD", "MWD", "SWD", "?"].contains(&act.0.name) {
- Err(format!("not allowed to set `{}`", act.0.name))
- } else {
- Ok(act)
- }
- })
- .and_then(|Action(key, operator, expression)| {
- value_check(self, &expression, &key.kind)
- .map_err(|why| format!("{}: {}", key.name, why))
- .map(|rhs| (rhs, key, operator))
- })
- .and_then(|(rhs, key, operator)| {
- if operator == Operator::OptionalEqual
- && self.variables.get_ref(key.name).is_some()
- {
- return Ok(());
- }
- if [Operator::Equal, Operator::OptionalEqual].contains(&operator) {
- // When we changed the HISTORY_IGNORE variable, update the
- // ignore patterns. This happens first because `set_array`
- // consumes 'values'
- if key.name == "HISTORY_IGNORE" {
- if let Value::Array(array) = &rhs {
- self.update_ignore_patterns(array);
- }
+ ) -> Result<Vec<(Key<'a>, Value)>, String> {
+ let mut backup: Vec<_> = Vec::new();
+ for action in actions.map(|act| act.map_err(|e| e.to_string())) {
+ action
+ .and_then(|Action(key, operator, expression)| {
+ // sanitize variable names
+ if ["HOME", "HOST", "PWD", "MWD", "SWD", "?"].contains(&key.name) {
+ Err(format!("not allowed to set `{}`", key.name))
+ } else {
+ Ok((key, operator, expression))
+ }
+ })
+ .and_then(|(key, operator, expression)| {
+ value_check(self, &expression, &key.kind)
+ .map_err(|why| format!("{}: {}", key.name, why))
+ .map(|rhs| (key, operator, rhs))
+ })
+ .and_then(|(key, operator, rhs)| {
+ if operator == Operator::OptionalEqual
+ && self.variables.get_ref(key.name).is_some()
+ {
+ Ok(())
+ } else if [Operator::Equal, Operator::OptionalEqual].contains(&operator) {
+ // When we changed the HISTORY_IGNORE variable, update the
+ // ignore patterns. This happens first because `set_array`
+ // consumes 'values'
+ if key.name == "HISTORY_IGNORE" {
+ if let Value::Array(array) = &rhs {
+ self.update_ignore_patterns(array);
}
-
- return match (&rhs, &key.kind) {
- (Value::HashMap(_), Primitive::Indexed(..)) => {
- Err("cannot insert hmap into index".to_string())
- }
- (Value::BTreeMap(_), Primitive::Indexed(..)) => {
- Err("cannot insert bmap into index".to_string())
- }
- (Value::Array(_), Primitive::Indexed(..)) => {
- Err("multi-dimensional arrays are not yet supported"
- .to_string())
- }
- _ => {
- patch.insert(key, rhs);
- Ok(())
- }
- };
}
- self.variables
- .get_mut(key.name)
- .ok_or_else(|| {
- format!("cannot update non existing variable `{}`", key.name)
- })
- .and_then(|lhs| match rhs {
- Value::Str(rhs) => match lhs {
- Value::Str(lhs) => math(&key.kind, operator, &rhs)
- .and_then(|action| parse(&lhs, &*action))
- .map(|value| {
- patch.insert(key, Value::Str(value.into()));
- })
- .map_err(|why| why.to_string()),
- Value::Array(ref mut array) => match operator {
- Operator::Concatenate => {
- array.push(rhs);
- Ok(())
- }
- Operator::ConcatenateHead => {
- array.insert(0, rhs);
- Ok(())
- }
- Operator::Filter => {
- array.retain(|item| item != &rhs);
- Ok(())
- }
- _ => math(&Primitive::Float, operator, &rhs)
- .and_then(|action| {
- array
- .iter_mut()
- .map(|el| {
- parse(el, &*action)
- .map(|result| *el = result.into())
- })
- .find(|e| e.is_err())
- .unwrap_or(Ok(()))
- })
- .map_err(|why| why.to_string()),
- },
- _ => Err("type does not support this operator".to_string()),
- },
- Value::Array(values) => {
- if let Value::Array(ref mut array) = lhs {
- match operator {
- Operator::Concatenate => array.extend(values),
- Operator::ConcatenateHead => values
- .into_iter()
- .rev()
- .for_each(|value| array.insert(0, value)),
- Operator::Filter => {
- array.retain(|item| !values.contains(item))
- }
- _ => {}
- }
- }
- Ok(())
- }
- _ => unreachable!(),
- })
- })
- })
- .find(|e| e.is_err())
- .unwrap_or_else(|| Ok(()))
- .map(|_| patch)
- }
-
- fn patch(&mut self, patch: HashMap<Key, Value>) -> Result<(), String> {
- patch
- .into_iter()
- .map(|(key, val)| match (&val, &key.kind) {
- (Value::Str(_), Primitive::Indexed(ref index_name, ref index_kind)) => {
- value_check(self, index_name, index_kind)
- .map_err(|why| format!("assignment error: {}: {}", key.name, why))
- .and_then(|index| match index {
- Value::Array(_) => Err("index variable cannot be an array".to_string()),
- Value::HashMap(_) => Err("index variable cannot be a hmap".to_string()),
- Value::BTreeMap(_) => {
- Err("index variable cannot be a bmap".to_string())
+ match (&rhs, &key.kind) {
+ (Value::HashMap(_), Primitive::Indexed(..)) => {
+ Err("cannot insert hmap into index".to_string())
}
- Value::Str(ref index) => self
- .variables
- .get_mut(key.name)
- .map(|lhs| match lhs {
- Value::HashMap(hmap) => {
- hmap.insert(index.clone(), val);
- Ok(())
- }
- Value::BTreeMap(bmap) => {
- bmap.insert(index.clone(), val);
- Ok(())
- }
- Value::Array(array) => index
- .parse::<usize>()
- .map_err(|_| {
- format!(
- "index variable is not a numeric value: `{}`",
- index
- )
- })
- .map(|index_num| {
- if let (Some(var), Value::Str(value)) =
- (array.get_mut(index_num), val)
- {
- *var = value;
- }
- }),
- _ => Ok(()),
- })
- .unwrap_or(Ok(())),
- _ => Ok(()),
- })
- }
- (Value::Str(_), _)
- | (Value::HashMap(_), Primitive::HashMap(_))
- | (Value::BTreeMap(_), Primitive::BTreeMap(_))
- | (Value::Array(_), _) => {
- self.variables.set(key.name, val);
- Ok(())
- }
- _ => Ok(()),
- })
- .find(|e| e.is_err())
- .unwrap_or_else(|| Ok(()))
+ (Value::BTreeMap(_), Primitive::Indexed(..)) => {
+ Err("cannot insert bmap into index".to_string())
+ }
+ (Value::Array(_), Primitive::Indexed(..)) => {
+ Err("multi-dimensional arrays are not yet supported".to_string())
+ }
+ _ => {
+ backup.push((key, rhs));
+ Ok(())
+ }
+ }
+ } else {
+ self.overwrite(key, operator, rhs)
+ }
+ })?;
+ }
+ Ok(backup)
}
fn local(&mut self, action: LocalAction) -> i32 {
@@ -306,7 +179,12 @@ impl VariableStore for Shell {
}
LocalAction::Assign(ref keys, op, ref vals) => {
let actions = AssignmentActions::new(keys, op, vals);
- if let Err(why) = self.create_patch(actions).and_then(|patch| self.patch(patch)) {
+ if let Err(why) = self.calculate(actions).and_then(|apply| {
+ for (key, value) in apply {
+ self.assign(key, value)?
+ }
+ Ok(())
+ }) {
eprintln!("ion: assignment error: {}", why);
FAILURE
} else {
@@ -318,7 +196,7 @@ impl VariableStore for Shell {
}
#[derive(Debug)]
-enum MathError {
+pub enum MathError {
RHS,
LHS,
Unsupported,
@@ -336,7 +214,7 @@ impl Display for MathError {
}
}
-fn parse<T: str::FromStr, F: Fn(T) -> Option<f64>>(
+pub fn parse<T: str::FromStr, F: Fn(T) -> Option<f64>>(
lhs: &str,
operation: F,
) -> Result<String, MathError> {
@@ -346,7 +224,7 @@ fn parse<T: str::FromStr, F: Fn(T) -> Option<f64>>(
.map(|x| x.to_string())
}
-fn math<'a>(
+pub fn math<'a>(
key: &Primitive,
operator: Operator,
value: &'a str,
diff --git a/src/lib/shell/mod.rs b/src/lib/shell/mod.rs
index 91c0b732..4de5cd4e 100644
--- a/src/lib/shell/mod.rs
+++ b/src/lib/shell/mod.rs
@@ -40,6 +40,7 @@ pub(crate) use self::{
};
use self::{
+ assignments::{math, parse},
directory_stack::DirectoryStack,
flags::*,
flow_control::{FlowControl, Function, FunctionError},
@@ -51,7 +52,11 @@ use self::{
};
use crate::{
builtins::{BuiltinMap, BUILTINS},
- parser::{pipelines::Pipeline, Expander, MapKeyIter, MapValueIter, Select, Terminator},
+ lexers::{Key, Operator, Primitive},
+ parser::{
+ assignments::value_check, pipelines::Pipeline, Expander, MapKeyIter, MapValueIter, Select,
+ Terminator,
+ },
sys,
types::{self, Array},
};
@@ -405,6 +410,123 @@ impl Shell {
shell.update_ignore_patterns(&ignore_patterns);
shell
}
+
+ pub fn assign(&mut self, key: Key, value: Value) -> Result<(), String> {
+ match (&key.kind, &value) {
+ (Primitive::Indexed(ref index_name, ref index_kind), Value::Str(_)) => {
+ let index = value_check(self, index_name, index_kind)
+ .map_err(|why| format!("{}: {}", key.name, why))?;
+
+ match index {
+ Value::Str(ref index) => {
+ let lhs = self
+ .variables
+ .get_mut(key.name)
+ .ok_or_else(|| "index value does not exist".to_string())?;
+
+ match lhs {
+ Value::HashMap(hmap) => {
+ let _ = hmap.insert(index.clone(), value);
+ Ok(())
+ }
+ Value::BTreeMap(bmap) => {
+ let _ = bmap.insert(index.clone(), value);
+ Ok(())
+ }
+ Value::Array(array) => {
+ let index_num = index.parse::<usize>().map_err(|_| {
+ format!("index variable is not a numeric value: `{}`", index)
+ })?;
+
+ if let (Some(var), Value::Str(val)) =
+ (array.get_mut(index_num), value)
+ {
+ *var = val;
+ }
+ Ok(())
+ }
+ _ => Ok(()),
+ }
+ }
+ Value::Array(_) => Err("index variable cannot be an array".into()),
+ Value::HashMap(_) => Err("index variable cannot be a hmap".into()),
+ Value::BTreeMap(_) => Err("index variable cannot be a bmap".into()),
+ _ => Ok(()),
+ }
+ }
+ (_, Value::Str(_))
+ | (_, Value::Array(_))
+ | (Primitive::HashMap(_), Value::HashMap(_))
+ | (Primitive::BTreeMap(_), Value::BTreeMap(_)) => {
+ self.variables.set(key.name, value);
+ Ok(())
+ }
+ _ => Ok(()),
+ }
+ }
+
+ pub fn overwrite(&mut self, key: Key, operator: Operator, rhs: Value) -> Result<(), String> {
+ let lhs = self
+ .variables
+ .get_mut(key.name)
+ .ok_or_else(|| format!("cannot update non existing variable `{}`", key.name))?;
+
+ match lhs {
+ Value::Str(lhs) => {
+ if let Value::Str(rhs) = rhs {
+ let action = math(&key.kind, operator, &rhs).map_err(|why| why.to_string())?;
+ let value = parse(&lhs, &*action).map_err(|why| why.to_string())?;
+ *lhs = value.into();
+ }
+ Ok(())
+ }
+ Value::Array(array) => match rhs {
+ Value::Str(rhs) => match operator {
+ Operator::Concatenate => {
+ array.push(rhs.clone());
+ Ok(())
+ }
+ Operator::ConcatenateHead => {
+ array.insert(0, rhs.clone());
+ Ok(())
+ }
+ Operator::Filter => {
+ array.retain(|item| item != &rhs);
+ Ok(())
+ }
+ _ => math(&Primitive::Float, operator, &rhs)
+ .and_then(|action| {
+ array
+ .iter_mut()
+ .map(|el| parse(el, &*action).map(|result| *el = result.into()))
+ .find(|e| e.is_err())
+ .unwrap_or(Ok(()))
+ })
+ .map_err(|why| why.to_string()),
+ },
+ Value::Array(values) => {
+ match operator {
+ Operator::Concatenate => array.extend(values.clone()),
+ Operator::ConcatenateHead => values
+ .into_iter()
+ .rev()
+ .for_each(|value| array.insert(0, value.clone())),
+ Operator::Filter => array.retain(|item| !values.contains(item)),
+ _ => {}
+ }
+ Ok(())
+ }
+ _ => Ok(()),
+ },
+ _ => {
+ if let Value::Str(_) = rhs {
+ Err("type does not support this operator".to_string())
+ } else {
+ Ok(())
+ }
+ }
+ }
+ }
}
impl<'a> Expander for Shell {
--
GitLab