From 0b412cd6b3ea017f27f1d7f53351cd5e9be5ea92 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20=C5=BD=C3=A1dn=C3=ADk?= Date: Sun, 5 Sep 2021 00:52:57 +0300 Subject: [PATCH 1/7] Add support for positive integer ranges Including support for variables and subexpressions as range bounds. --- crates/nu-cli/src/syntax_highlight.rs | 4 + crates/nu-engine/src/eval.rs | 44 ++++++- crates/nu-parser/src/flatten.rs | 12 ++ crates/nu-parser/src/parser.rs | 154 ++++++++++++++++++++++- crates/nu-protocol/src/ast/expr.rs | 7 +- crates/nu-protocol/src/ast/expression.rs | 2 + crates/nu-protocol/src/ast/operator.rs | 23 ++++ crates/nu-protocol/src/ty.rs | 2 + crates/nu-protocol/src/value.rs | 34 +++++ 9 files changed, 277 insertions(+), 5 deletions(-) diff --git a/crates/nu-cli/src/syntax_highlight.rs b/crates/nu-cli/src/syntax_highlight.rs index 9a57dd71ed..4df2e42c97 100644 --- a/crates/nu-cli/src/syntax_highlight.rs +++ b/crates/nu-cli/src/syntax_highlight.rs @@ -57,6 +57,10 @@ impl Highlighter for NuHighlighter { FlatShape::Float => { output.push((Style::new().fg(nu_ansi_term::Color::Green), next_token)) } + FlatShape::Range => output.push(( + Style::new().fg(nu_ansi_term::Color::LightPurple), + next_token, + )), FlatShape::Bool => { output.push((Style::new().fg(nu_ansi_term::Color::LightCyan), next_token)) } diff --git a/crates/nu-engine/src/eval.rs b/crates/nu-engine/src/eval.rs index 733fd63807..4d6509702e 100644 --- a/crates/nu-engine/src/eval.rs +++ b/crates/nu-engine/src/eval.rs @@ -1,6 +1,6 @@ use nu_protocol::ast::{Block, Call, Expr, Expression, Operator, Statement}; use nu_protocol::engine::EvaluationContext; -use nu_protocol::{ShellError, Value}; +use nu_protocol::{Range, ShellError, Span, Value}; pub fn eval_operator(op: &Expression) -> Result { match op { @@ -55,6 +55,48 @@ pub fn eval_expression( val: *f, span: expr.span, }), + Expr::Range(from, to, operator) => { + // TODO: Embed the min/max into Range and set max to be the true max + let from = if let Some(f) = from { + eval_expression(context, &f)? + } else { + Value::Int { + val: 0i64, + span: Span::unknown(), + } + }; + + let to = if let Some(t) = to { + eval_expression(context, &t)? + } else { + Value::Int { + val: 100i64, + span: Span::unknown(), + } + }; + + let range = match (&from, &to) { + (&Value::Int { .. }, &Value::Int { .. }) => Range { + from: from.clone(), + to: to.clone(), + inclusion: operator.inclusion, + }, + (lhs, rhs) => { + return Err(ShellError::OperatorMismatch { + op_span: operator.span, + lhs_ty: lhs.get_type(), + lhs_span: lhs.span(), + rhs_ty: rhs.get_type(), + rhs_span: rhs.span(), + }) + } + }; + + Ok(Value::Range { + val: Box::new(range), + span: expr.span, + }) + } Expr::Var(var_id) => context .get_var(*var_id) .map_err(move |_| ShellError::VariableNotFound(expr.span)), diff --git a/crates/nu-parser/src/flatten.rs b/crates/nu-parser/src/flatten.rs index d2a55e9af6..d850bd5c6c 100644 --- a/crates/nu-parser/src/flatten.rs +++ b/crates/nu-parser/src/flatten.rs @@ -7,6 +7,7 @@ pub enum FlatShape { Bool, Int, Float, + Range, InternalCall, External, Literal, @@ -65,6 +66,17 @@ pub fn flatten_expression( } Expr::Float(_) => { vec![(expr.span, FlatShape::Float)] + } + Expr::Range(from, to, op) => { + let mut output = vec![]; + if let Some(f) = from { + output.extend(flatten_expression(working_set, f)); + } + if let Some(t) = to { + output.extend(flatten_expression(working_set, t)); + } + output.extend(vec![(op.span, FlatShape::Operator)]); + output } Expr::Bool(_) => { vec![(expr.span, FlatShape::Bool)] diff --git a/crates/nu-parser/src/parser.rs b/crates/nu-parser/src/parser.rs index 327506bbca..bb9bea80c3 100644 --- a/crates/nu-parser/src/parser.rs +++ b/crates/nu-parser/src/parser.rs @@ -5,7 +5,9 @@ use crate::{ }; use nu_protocol::{ - ast::{Block, Call, Expr, Expression, Operator, Pipeline, Statement}, + ast::{ + Block, Call, Expr, Expression, Operator, Pipeline, RangeInclusion, RangeOperator, Statement, + }, engine::StateWorkingSet, span, Flag, PositionalArg, Signature, Span, SyntaxShape, Type, VarId, }; @@ -702,6 +704,143 @@ pub fn parse_number(token: &str, span: Span) -> (Expression, Option) } } +pub fn parse_range( + working_set: &mut StateWorkingSet, + span: Span, +) -> (Expression, Option) { + // Range follows the following syntax: [][][] + // where is ".." + // and is ".." or "..<" + // and one of the or bounds must be present (just '..' is not allowed since it + // looks like parent directory) + + let contents = working_set.get_span_contents(span); + let token = if let Ok(s) = String::from_utf8(contents.into()) { + s + } else { + return (garbage(span), Some(ParseError::NonUtf8(span))); + }; + + // First, figure out what exact operators are used and determine their positions + let dotdot_pos: Vec<_> = token.match_indices("..").map(|(pos, _)| pos).collect(); + + let (step_op_pos, range_op_pos) = + match dotdot_pos.len() { + 1 => (None, dotdot_pos[0]), + 2 => (Some(dotdot_pos[0]), dotdot_pos[1]), + _ => return ( + garbage(span), + Some(ParseError::Expected( + "one range operator ('..' or '..<') and optionally one step operator ('..')" + .into(), + span, + )), + ), + }; + + let step_op_span = if let Some(pos) = step_op_pos { + Some(Span::new( + span.start + pos, + span.start + pos + "..".len(), // Only ".." is allowed for step operator + )) + } else { + None + }; + + let (range_op, range_op_str, range_op_span) = if let Some(pos) = token.find("..<") { + if pos == range_op_pos { + let op_str = "..<"; + let op_span = Span::new( + span.start + range_op_pos, + span.start + range_op_pos + op_str.len(), + ); + ( + RangeOperator { + inclusion: RangeInclusion::RightExclusive, + span: op_span, + }, + "..<", + op_span, + ) + } else { + return ( + garbage(span), + Some(ParseError::Expected( + "inclusive operator preceding second range bound".into(), + span, + )), + ); + } + } else { + let op_str = ".."; + let op_span = Span::new( + span.start + range_op_pos, + span.start + range_op_pos + op_str.len(), + ); + ( + RangeOperator { + inclusion: RangeInclusion::Inclusive, + span: op_span, + }, + "..", + op_span, + ) + }; + + // Now, based on the operator positions, figure out where the bounds & step are located and + // parse them + // TODO: Actually parse the step number + let from = if token.starts_with("..") { + // token starts with either step operator, or range operator -- we don't care which one + None + } else { + let from_span = Span::new(span.start, span.start + dotdot_pos[0]); + match parse_value(working_set, from_span, &SyntaxShape::Number) { + (expression, None) => Some(Box::new(expression)), + _ => { + return ( + garbage(span), + Some(ParseError::Expected("number".into(), span)), + ) + } + } + }; + + let to = if token.ends_with(range_op_str) { + None + } else { + let to_span = Span::new(range_op_span.end, span.end); + match parse_value(working_set, to_span, &SyntaxShape::Number) { + (expression, None) => Some(Box::new(expression)), + _ => { + return ( + garbage(span), + Some(ParseError::Expected("number".into(), span)), + ) + } + } + }; + + if let (None, None) = (&from, &to) { + return ( + garbage(span), + Some(ParseError::Expected( + "at least one range bound set".into(), + span, + )), + ); + } + + ( + Expression { + expr: Expr::Range(from, to, range_op), + span, + ty: Type::Range, + }, + None, + ) +} + pub(crate) fn parse_dollar_expr( working_set: &mut StateWorkingSet, span: Span, @@ -711,7 +850,11 @@ pub(crate) fn parse_dollar_expr( if contents.starts_with(b"$\"") { parse_string_interpolation(working_set, span) } else { - parse_variable_expr(working_set, span) + if let (expr, None) = parse_range(working_set, span) { + (expr, None) + } else { + parse_variable_expr(working_set, span) + } } } @@ -1684,7 +1827,11 @@ pub fn parse_value( } else if bytes.starts_with(b"$") { return parse_dollar_expr(working_set, span); } else if bytes.starts_with(b"(") { - return parse_full_column_path(working_set, span); + if let (expr, None) = parse_range(working_set, span) { + return (expr, None); + } else { + return parse_full_column_path(working_set, span); + } } else if bytes.starts_with(b"{") { if matches!(shape, SyntaxShape::Block) || matches!(shape, SyntaxShape::Any) { return parse_block_expression(working_set, span); @@ -1730,6 +1877,7 @@ pub fn parse_value( ) } } + SyntaxShape::Range => parse_range(working_set, span), SyntaxShape::String | SyntaxShape::GlobPattern | SyntaxShape::FilePath => { parse_string(working_set, span) } diff --git a/crates/nu-protocol/src/ast/expr.rs b/crates/nu-protocol/src/ast/expr.rs index 818db24cd5..021b57b9eb 100644 --- a/crates/nu-protocol/src/ast/expr.rs +++ b/crates/nu-protocol/src/ast/expr.rs @@ -1,4 +1,4 @@ -use super::{Call, Expression, Operator}; +use super::{Call, Expression, Operator, RangeOperator}; use crate::{BlockId, Signature, Span, VarId}; #[derive(Debug, Clone)] @@ -6,6 +6,11 @@ pub enum Expr { Bool(bool), Int(i64), Float(f64), + Range( + Option>, + Option>, + RangeOperator, + ), Var(VarId), Call(Box), ExternalCall(Vec, Vec>), diff --git a/crates/nu-protocol/src/ast/expression.rs b/crates/nu-protocol/src/ast/expression.rs index d17b05428c..228bdf50ec 100644 --- a/crates/nu-protocol/src/ast/expression.rs +++ b/crates/nu-protocol/src/ast/expression.rs @@ -7,6 +7,7 @@ pub struct Expression { pub span: Span, pub ty: Type, } + impl Expression { pub fn garbage(span: Span) -> Expression { Expression { @@ -15,6 +16,7 @@ impl Expression { ty: Type::Unknown, } } + pub fn precedence(&self) -> usize { match &self.expr { Expr::Operator(operator) => { diff --git a/crates/nu-protocol/src/ast/operator.rs b/crates/nu-protocol/src/ast/operator.rs index f230e4a89a..c7c82eba41 100644 --- a/crates/nu-protocol/src/ast/operator.rs +++ b/crates/nu-protocol/src/ast/operator.rs @@ -1,3 +1,5 @@ +use crate::Span; + use std::fmt::Display; #[derive(Debug, Clone, PartialEq, Eq)] @@ -46,3 +48,24 @@ impl Display for Operator { } } } + +#[derive(Debug, Copy, Clone, PartialEq)] +pub enum RangeInclusion { + Inclusive, + RightExclusive, +} + +#[derive(Debug, Copy, Clone)] +pub struct RangeOperator { + pub inclusion: RangeInclusion, + pub span: Span, +} + +impl Display for RangeOperator { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self.inclusion { + RangeInclusion::Inclusive => write!(f, ".."), + RangeInclusion::RightExclusive => write!(f, "..<"), + } + } +} diff --git a/crates/nu-protocol/src/ty.rs b/crates/nu-protocol/src/ty.rs index ba2fe8d2ec..b0e291b414 100644 --- a/crates/nu-protocol/src/ty.rs +++ b/crates/nu-protocol/src/ty.rs @@ -4,6 +4,7 @@ use std::fmt::Display; pub enum Type { Int, Float, + Range, Bool, String, Block, @@ -31,6 +32,7 @@ impl Display for Type { Type::Filesize => write!(f, "filesize"), Type::Float => write!(f, "float"), Type::Int => write!(f, "int"), + Type::Range => write!(f, "range"), Type::List(l) => write!(f, "list<{}>", l), Type::Nothing => write!(f, "nothing"), Type::Number => write!(f, "number"), diff --git a/crates/nu-protocol/src/value.rs b/crates/nu-protocol/src/value.rs index 7e47fd75d4..745bbbaadb 100644 --- a/crates/nu-protocol/src/value.rs +++ b/crates/nu-protocol/src/value.rs @@ -1,5 +1,6 @@ use std::{cell::RefCell, fmt::Debug, rc::Rc}; +use crate::ast::RangeInclusion; use crate::{span, BlockId, Span, Type}; use crate::ShellError; @@ -103,6 +104,13 @@ impl IntoRowStream for Vec> { } } +#[derive(Debug, Clone, PartialEq)] +pub struct Range { + pub from: Value, + pub to: Value, + pub inclusion: RangeInclusion, +} + #[derive(Debug, Clone)] pub enum Value { Bool { @@ -113,6 +121,10 @@ pub enum Value { val: i64, span: Span, }, + Range { + val: Box, + span: Span, + }, Float { val: f64, span: Span, @@ -161,6 +173,7 @@ impl Value { Value::Bool { span, .. } => *span, Value::Int { span, .. } => *span, Value::Float { span, .. } => *span, + Value::Range { span, .. } => *span, Value::String { span, .. } => *span, Value::List { span, .. } => *span, Value::Table { span, .. } => *span, @@ -176,6 +189,7 @@ impl Value { Value::Bool { span, .. } => *span = new_span, Value::Int { span, .. } => *span = new_span, Value::Float { span, .. } => *span = new_span, + Value::Range { span, .. } => *span = new_span, Value::String { span, .. } => *span = new_span, Value::RowStream { span, .. } => *span = new_span, Value::ValueStream { span, .. } => *span = new_span, @@ -193,6 +207,7 @@ impl Value { Value::Bool { .. } => Type::Bool, Value::Int { .. } => Type::Int, Value::Float { .. } => Type::Float, + Value::Range { .. } => Type::Range, Value::String { .. } => Type::String, Value::List { .. } => Type::List(Box::new(Type::Unknown)), // FIXME Value::Table { .. } => Type::Table, // FIXME @@ -208,6 +223,25 @@ impl Value { Value::Bool { val, .. } => val.to_string(), Value::Int { val, .. } => val.to_string(), Value::Float { val, .. } => val.to_string(), + Value::Range { val, .. } => { + let vals: Vec = match (&val.from, &val.to) { + (Value::Int { val: from, .. }, Value::Int { val: to, .. }) => { + match val.inclusion { + RangeInclusion::Inclusive => (*from..=*to).collect(), + RangeInclusion::RightExclusive => (*from..*to).collect(), + } + } + _ => Vec::new(), + }; + + format!( + "range: [{}]", + vals.iter() + .map(|x| x.to_string()) + .collect::>() + .join(", ".into()) + ) + }, Value::String { val, .. } => val, Value::ValueStream { stream, .. } => stream.into_string(), Value::List { val, .. } => val From 672fa852b31642de285a0ef586d5f9a6931aee4a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20=C5=BD=C3=A1dn=C3=ADk?= Date: Sun, 5 Sep 2021 01:25:31 +0300 Subject: [PATCH 2/7] Add some tests to range parsing --- crates/nu-parser/tests/test_parser.rs | 222 ++++++++++++++++++++++++++ 1 file changed, 222 insertions(+) diff --git a/crates/nu-parser/tests/test_parser.rs b/crates/nu-parser/tests/test_parser.rs index a2f5fc2609..fa0df63d69 100644 --- a/crates/nu-parser/tests/test_parser.rs +++ b/crates/nu-parser/tests/test_parser.rs @@ -142,3 +142,225 @@ pub fn parse_call_missing_req_flag() { let (_, err) = parse_source(&mut working_set, b"foo", true); assert!(matches!(err, Some(ParseError::MissingRequiredFlag(..)))); } + +mod range { + use super::*; + use nu_protocol::ast::{RangeInclusion, RangeOperator}; + + #[test] + fn parse_inclusive_range() { + let engine_state = EngineState::new(); + let mut working_set = StateWorkingSet::new(&engine_state); + + let (block, err) = parse_source(&mut working_set, b"0..10", true); + + assert!(err.is_none()); + assert!(block.len() == 1); + match &block[0] { + Statement::Pipeline(Pipeline { expressions }) => { + assert!(expressions.len() == 1); + assert!(matches!( + expressions[0], + Expression { + expr: Expr::Range( + Some(_), + Some(_), + RangeOperator { + inclusion: RangeInclusion::Inclusive, + .. + } + ), + .. + } + )) + } + _ => panic!("No match"), + } + } + + #[test] + fn parse_exclusive_range() { + let engine_state = EngineState::new(); + let mut working_set = StateWorkingSet::new(&engine_state); + + let (block, err) = parse_source(&mut working_set, b"0..<10", true); + + assert!(err.is_none()); + assert!(block.len() == 1); + match &block[0] { + Statement::Pipeline(Pipeline { expressions }) => { + assert!(expressions.len() == 1); + assert!(matches!( + expressions[0], + Expression { + expr: Expr::Range( + Some(_), + Some(_), + RangeOperator { + inclusion: RangeInclusion::RightExclusive, + .. + } + ), + .. + } + )) + } + _ => panic!("No match"), + } + } + + #[test] + fn parse_subexpression_range() { + let engine_state = EngineState::new(); + let mut working_set = StateWorkingSet::new(&engine_state); + + let (block, err) = parse_source(&mut working_set, b"(3 - 3)..<(8 + 2)", true); + + assert!(err.is_none()); + assert!(block.len() == 1); + match &block[0] { + Statement::Pipeline(Pipeline { expressions }) => { + assert!(expressions.len() == 1); + assert!(matches!( + expressions[0], + Expression { + expr: Expr::Range( + Some(_), + Some(_), + RangeOperator { + inclusion: RangeInclusion::RightExclusive, + .. + } + ), + .. + } + )) + } + _ => panic!("No match"), + } + } + + #[test] + fn parse_variable_range() { + let engine_state = EngineState::new(); + let mut working_set = StateWorkingSet::new(&engine_state); + + let (block, err) = parse_source(&mut working_set, b"let a = 2; $a..10", true); + + assert!(err.is_none()); + assert!(block.len() == 2); + match &block[1] { + Statement::Pipeline(Pipeline { expressions }) => { + assert!(expressions.len() == 1); + assert!(matches!( + expressions[0], + Expression { + expr: Expr::Range( + Some(_), + Some(_), + RangeOperator { + inclusion: RangeInclusion::Inclusive, + .. + } + ), + .. + } + )) + } + _ => panic!("No match"), + } + } + + #[test] + fn parse_subexpression_variable_range() { + let engine_state = EngineState::new(); + let mut working_set = StateWorkingSet::new(&engine_state); + + let (block, err) = parse_source(&mut working_set, b"let a = 2; $a..<($a + 10)", true); + + assert!(err.is_none()); + assert!(block.len() == 2); + match &block[1] { + Statement::Pipeline(Pipeline { expressions }) => { + assert!(expressions.len() == 1); + assert!(matches!( + expressions[0], + Expression { + expr: Expr::Range( + Some(_), + Some(_), + RangeOperator { + inclusion: RangeInclusion::RightExclusive, + .. + } + ), + .. + } + )) + } + _ => panic!("No match"), + } + } + + #[test] + fn parse_left_unbounded_range() { + let engine_state = EngineState::new(); + let mut working_set = StateWorkingSet::new(&engine_state); + + let (block, err) = parse_source(&mut working_set, b"..10", true); + + assert!(err.is_none()); + assert!(block.len() == 1); + match &block[0] { + Statement::Pipeline(Pipeline { expressions }) => { + assert!(expressions.len() == 1); + assert!(matches!( + expressions[0], + Expression { + expr: Expr::Range( + None, + Some(_), + RangeOperator { + inclusion: RangeInclusion::Inclusive, + .. + } + ), + .. + } + )) + } + _ => panic!("No match"), + } + } + + #[test] + fn parse_right_unbounded_range() { + let engine_state = EngineState::new(); + let mut working_set = StateWorkingSet::new(&engine_state); + + let (block, err) = parse_source(&mut working_set, b"0..", true); + + assert!(err.is_none()); + assert!(block.len() == 1); + match &block[0] { + Statement::Pipeline(Pipeline { expressions }) => { + assert!(expressions.len() == 1); + assert!(matches!( + expressions[0], + Expression { + expr: Expr::Range( + Some(_), + None, + RangeOperator { + inclusion: RangeInclusion::Inclusive, + .. + } + ), + .. + } + )) + } + _ => panic!("No match"), + } + } +} From 6b4fee88c901187edc7d6a926446ebd0aa4e4b06 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20=C5=BD=C3=A1dn=C3=ADk?= Date: Sun, 5 Sep 2021 01:35:08 +0300 Subject: [PATCH 3/7] Fmt --- crates/nu-parser/src/flatten.rs | 2 +- crates/nu-protocol/src/value.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/crates/nu-parser/src/flatten.rs b/crates/nu-parser/src/flatten.rs index d850bd5c6c..dec4c89f5b 100644 --- a/crates/nu-parser/src/flatten.rs +++ b/crates/nu-parser/src/flatten.rs @@ -67,7 +67,7 @@ pub fn flatten_expression( Expr::Float(_) => { vec![(expr.span, FlatShape::Float)] } - Expr::Range(from, to, op) => { + Expr::Range(from, to, op) => { let mut output = vec![]; if let Some(f) = from { output.extend(flatten_expression(working_set, f)); diff --git a/crates/nu-protocol/src/value.rs b/crates/nu-protocol/src/value.rs index 745bbbaadb..d1d72727cf 100644 --- a/crates/nu-protocol/src/value.rs +++ b/crates/nu-protocol/src/value.rs @@ -241,7 +241,7 @@ impl Value { .collect::>() .join(", ".into()) ) - }, + } Value::String { val, .. } => val, Value::ValueStream { stream, .. } => stream.into_string(), Value::List { val, .. } => val From f0d469f1d4d893ec05b590aed183bd7ab0443c07 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20=C5=BD=C3=A1dn=C3=ADk?= Date: Sun, 5 Sep 2021 01:40:15 +0300 Subject: [PATCH 4/7] Fix clippy warnings --- crates/nu-engine/src/eval.rs | 4 ++-- crates/nu-parser/src/parser.rs | 18 +++++++----------- crates/nu-protocol/src/value.rs | 2 +- 3 files changed, 10 insertions(+), 14 deletions(-) diff --git a/crates/nu-engine/src/eval.rs b/crates/nu-engine/src/eval.rs index 4d6509702e..e7a9cef6bf 100644 --- a/crates/nu-engine/src/eval.rs +++ b/crates/nu-engine/src/eval.rs @@ -58,7 +58,7 @@ pub fn eval_expression( Expr::Range(from, to, operator) => { // TODO: Embed the min/max into Range and set max to be the true max let from = if let Some(f) = from { - eval_expression(context, &f)? + eval_expression(context, f)? } else { Value::Int { val: 0i64, @@ -67,7 +67,7 @@ pub fn eval_expression( }; let to = if let Some(t) = to { - eval_expression(context, &t)? + eval_expression(context, t)? } else { Value::Int { val: 100i64, diff --git a/crates/nu-parser/src/parser.rs b/crates/nu-parser/src/parser.rs index bb9bea80c3..3a0e600e9e 100644 --- a/crates/nu-parser/src/parser.rs +++ b/crates/nu-parser/src/parser.rs @@ -738,14 +738,12 @@ pub fn parse_range( ), }; - let step_op_span = if let Some(pos) = step_op_pos { - Some(Span::new( + let step_op_span = step_op_pos.map(|pos| { + Span::new( span.start + pos, span.start + pos + "..".len(), // Only ".." is allowed for step operator - )) - } else { - None - }; + ) + }); let (range_op, range_op_str, range_op_span) = if let Some(pos) = token.find("..<") { if pos == range_op_pos { @@ -849,12 +847,10 @@ pub(crate) fn parse_dollar_expr( if contents.starts_with(b"$\"") { parse_string_interpolation(working_set, span) + } else if let (expr, None) = parse_range(working_set, span) { + (expr, None) } else { - if let (expr, None) = parse_range(working_set, span) { - (expr, None) - } else { - parse_variable_expr(working_set, span) - } + parse_variable_expr(working_set, span) } } diff --git a/crates/nu-protocol/src/value.rs b/crates/nu-protocol/src/value.rs index d1d72727cf..d834bd3209 100644 --- a/crates/nu-protocol/src/value.rs +++ b/crates/nu-protocol/src/value.rs @@ -239,7 +239,7 @@ impl Value { vals.iter() .map(|x| x.to_string()) .collect::>() - .join(", ".into()) + .join(", ") ) } Value::String { val, .. } => val, From 7ae4ca88b6bb1ca55b8ac66f7b7ec316675308e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20=C5=BD=C3=A1dn=C3=ADk?= Date: Sun, 5 Sep 2021 11:03:04 +0300 Subject: [PATCH 5/7] "Fix" failing CI --- crates/nu-parser/src/parser.rs | 2 +- crates/nu-parser/tests/test_parser.rs | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/crates/nu-parser/src/parser.rs b/crates/nu-parser/src/parser.rs index 3a0e600e9e..3dc254ad4e 100644 --- a/crates/nu-parser/src/parser.rs +++ b/crates/nu-parser/src/parser.rs @@ -738,7 +738,7 @@ pub fn parse_range( ), }; - let step_op_span = step_op_pos.map(|pos| { + let _step_op_span = step_op_pos.map(|pos| { Span::new( span.start + pos, span.start + pos + "..".len(), // Only ".." is allowed for step operator diff --git a/crates/nu-parser/tests/test_parser.rs b/crates/nu-parser/tests/test_parser.rs index fa0df63d69..e5fab53c31 100644 --- a/crates/nu-parser/tests/test_parser.rs +++ b/crates/nu-parser/tests/test_parser.rs @@ -302,6 +302,7 @@ mod range { } } + #[ignore] #[test] fn parse_left_unbounded_range() { let engine_state = EngineState::new(); From 56c8987e0f38a5b69bb137842c778b6ab41b83bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20=C5=BD=C3=A1dn=C3=ADk?= Date: Sun, 5 Sep 2021 20:33:53 +0300 Subject: [PATCH 6/7] Add '.' and '-' to restricted characters This means that commands cannot start with these characters. However, we get the following benefits: * Negative numbers > -10 * Ranges with negative numbers > -10..-1 * Left-unbounded ranges > ..10 --- crates/nu-parser/src/parser.rs | 2 +- crates/nu-parser/tests/test_parser.rs | 32 ++++++++++++++++++++++++++- 2 files changed, 32 insertions(+), 2 deletions(-) diff --git a/crates/nu-parser/src/parser.rs b/crates/nu-parser/src/parser.rs index 3dc254ad4e..a414660418 100644 --- a/crates/nu-parser/src/parser.rs +++ b/crates/nu-parser/src/parser.rs @@ -2103,7 +2103,7 @@ pub fn parse_expression( match bytes[0] { b'0' | b'1' | b'2' | b'3' | b'4' | b'5' | b'6' | b'7' | b'8' | b'9' | b'(' | b'{' - | b'[' | b'$' | b'"' | b'\'' => parse_math_expression(working_set, spans), + | b'[' | b'$' | b'"' | b'\'' | b'.' | b'-' => parse_math_expression(working_set, spans), _ => parse_call(working_set, spans, true), } } diff --git a/crates/nu-parser/tests/test_parser.rs b/crates/nu-parser/tests/test_parser.rs index e5fab53c31..c47808fe88 100644 --- a/crates/nu-parser/tests/test_parser.rs +++ b/crates/nu-parser/tests/test_parser.rs @@ -302,7 +302,6 @@ mod range { } } - #[ignore] #[test] fn parse_left_unbounded_range() { let engine_state = EngineState::new(); @@ -364,4 +363,35 @@ mod range { _ => panic!("No match"), } } + + #[test] + fn parse_negative_range() { + let engine_state = EngineState::new(); + let mut working_set = StateWorkingSet::new(&engine_state); + + let (block, err) = parse_source(&mut working_set, b"-10..-3", true); + + assert!(err.is_none()); + assert!(block.len() == 1); + match &block[0] { + Statement::Pipeline(Pipeline { expressions }) => { + assert!(expressions.len() == 1); + assert!(matches!( + expressions[0], + Expression { + expr: Expr::Range( + Some(_), + Some(_), + RangeOperator { + inclusion: RangeInclusion::Inclusive, + .. + } + ), + .. + } + )) + } + _ => panic!("No match"), + } + } } From 6ebc97dec26490aa8256a497d3e322ba83e57866 Mon Sep 17 00:00:00 2001 From: JT <547158+jntrnr@users.noreply.github.com> Date: Mon, 6 Sep 2021 06:09:36 +1200 Subject: [PATCH 7/7] Update parser.rs --- crates/nu-parser/src/parser.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/nu-parser/src/parser.rs b/crates/nu-parser/src/parser.rs index a414660418..1e8227fa67 100644 --- a/crates/nu-parser/src/parser.rs +++ b/crates/nu-parser/src/parser.rs @@ -2103,7 +2103,7 @@ pub fn parse_expression( match bytes[0] { b'0' | b'1' | b'2' | b'3' | b'4' | b'5' | b'6' | b'7' | b'8' | b'9' | b'(' | b'{' - | b'[' | b'$' | b'"' | b'\'' | b'.' | b'-' => parse_math_expression(working_set, spans), + | b'[' | b'$' | b'"' | b'\'' | b'-' => parse_math_expression(working_set, spans), _ => parse_call(working_set, spans, true), } }