From 599bb9797deed240de64db9aa6333c4f344ae98d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Radek=20V=C3=ADt?= Date: Sun, 13 Sep 2020 23:53:08 +0200 Subject: [PATCH] Implement exclusive and inclusive ranges with ..< and .. (#2541) * Implement exclusive and inclusive ranges with .. and ..= This commit adds right-exclusive ranges. The original a..b inclusive syntax was changed to reflect the Rust notation. New a..=b syntax was introduced to have the old behavior. Currently, both a.. and b..= is valid, and it is unclear whether it's valid to impose restrictions. The original issue suggests .. for inclusive and ..< for exclusive ranges, this can be implemented by making simple changes to this commit. * Fix collect tests by changing ranges to ..= * Fix clippy lints in exclusive range matching * Implement exclusive ranges using `..<` --- crates/nu-cli/src/evaluate/evaluator.rs | 7 +++- crates/nu-cli/src/shell/palette.rs | 6 +++ crates/nu-data/src/base/shape.rs | 6 ++- crates/nu-parser/src/parse.rs | 28 ++++++++----- crates/nu-parser/src/shapes.rs | 8 +++- crates/nu-protocol/src/hir.rs | 15 +++++-- crates/nu-protocol/src/value/primitive.rs | 9 ++++- tests/shell/pipeline/commands/internal.rs | 48 +++++++++++++++++++++++ 8 files changed, 108 insertions(+), 19 deletions(-) diff --git a/crates/nu-cli/src/evaluate/evaluator.rs b/crates/nu-cli/src/evaluate/evaluator.rs index 768fb21e3..9d117e7d9 100644 --- a/crates/nu-cli/src/evaluate/evaluator.rs +++ b/crates/nu-cli/src/evaluate/evaluator.rs @@ -5,7 +5,7 @@ use crate::prelude::*; use async_recursion::async_recursion; use log::trace; use nu_errors::{ArgumentError, ShellError}; -use nu_protocol::hir::{self, Expression, ExternalRedirection, SpannedExpression}; +use nu_protocol::hir::{self, Expression, ExternalRedirection, RangeOperator, SpannedExpression}; use nu_protocol::{ ColumnPath, Primitive, RangeInclusion, UnspannedPathMember, UntaggedValue, Value, }; @@ -82,7 +82,10 @@ pub(crate) async fn evaluate_baseline_expr( ); let right = ( right.as_primitive()?.spanned(right_span), - RangeInclusion::Inclusive, + match &range.operator.item { + RangeOperator::Inclusive => RangeInclusion::Inclusive, + RangeOperator::RightExclusive => RangeInclusion::Exclusive, + }, ); Ok(UntaggedValue::range(left, right).into_value(tag)) diff --git a/crates/nu-cli/src/shell/palette.rs b/crates/nu-cli/src/shell/palette.rs index 8fb996c05..9c3358346 100644 --- a/crates/nu-cli/src/shell/palette.rs +++ b/crates/nu-cli/src/shell/palette.rs @@ -26,6 +26,9 @@ impl Palette for DefaultPalette { } FlatShape::Type => single_style_span(Color::Blue.bold(), shape.span), FlatShape::Operator => single_style_span(Color::Yellow.normal(), shape.span), + FlatShape::DotDotLeftAngleBracket => { + single_style_span(Color::Yellow.bold(), shape.span) + } FlatShape::DotDot => single_style_span(Color::Yellow.bold(), shape.span), FlatShape::Dot => single_style_span(Style::new().fg(Color::White), shape.span), FlatShape::InternalCommand => single_style_span(Color::Cyan.bold(), shape.span), @@ -91,6 +94,9 @@ impl Palette for ThemedPalette { FlatShape::Identifier => single_style_span(self.theme.identifier.normal(), shape.span), FlatShape::Type => single_style_span(self.theme.r#type.bold(), shape.span), FlatShape::Operator => single_style_span(self.theme.operator.normal(), shape.span), + FlatShape::DotDotLeftAngleBracket => { + single_style_span(self.theme.dot_dot.bold(), shape.span) + } FlatShape::DotDot => single_style_span(self.theme.dot_dot.bold(), shape.span), FlatShape::Dot => single_style_span(Style::new().fg(*self.theme.dot), shape.span), FlatShape::InternalCommand => { diff --git a/crates/nu-data/src/base/shape.rs b/crates/nu-data/src/base/shape.rs index a8f70a86f..0616c4c7e 100644 --- a/crates/nu-data/src/base/shape.rs +++ b/crates/nu-data/src/base/shape.rs @@ -143,7 +143,11 @@ impl PrettyDebug for FormatInlineShape { let op = match (left_inclusion, right_inclusion) { (RangeInclusion::Inclusive, RangeInclusion::Inclusive) => "..", - _ => unimplemented!("No syntax for ranges that aren't inclusive on the left and inclusive on the right") + (RangeInclusion::Inclusive, RangeInclusion::Exclusive) => "..<", + _ => unimplemented!( + "No syntax for ranges that aren't inclusive on the left and exclusive \ + or inclusive on the right" + ), }; left.clone().format().pretty() + b::operator(op) + right.clone().format().pretty() diff --git a/crates/nu-parser/src/parse.rs b/crates/nu-parser/src/parse.rs index 336058a5b..e8f6eb707 100644 --- a/crates/nu-parser/src/parse.rs +++ b/crates/nu-parser/src/parse.rs @@ -5,7 +5,7 @@ use nu_errors::{ArgumentError, ParseError}; use nu_protocol::hir::{ self, Binary, Block, ClassifiedBlock, ClassifiedCommand, ClassifiedPipeline, Commands, Expression, ExternalRedirection, Flag, FlagKind, InternalCommand, Member, NamedArguments, - Operator, SpannedExpression, Unit, + Operator, RangeOperator, SpannedExpression, Unit, }; use nu_protocol::{NamedType, PositionalType, Signature, SyntaxShape, UnspannedPathMember}; use nu_source::{Span, Spanned, SpannedItem}; @@ -242,8 +242,18 @@ fn parse_range( ) -> (SpannedExpression, Option) { let lite_arg_span_start = lite_arg.span.start(); let lite_arg_len = lite_arg.item.len(); - let dotdot_pos = lite_arg.item.find(".."); - let numbers: Vec<_> = lite_arg.item.split("..").collect(); + let (dotdot_pos, operator_str, operator) = if let Some(pos) = lite_arg.item.find("..<") { + (pos, "..<", RangeOperator::RightExclusive) + } else if let Some(pos) = lite_arg.item.find("..") { + (pos, "..", RangeOperator::Inclusive) + } else { + return ( + garbage(lite_arg.span), + Some(ParseError::mismatch("range", lite_arg.clone())), + ); + }; + + let numbers: Vec<_> = lite_arg.item.split(operator_str).collect(); if numbers.len() != 2 { return ( @@ -252,19 +262,19 @@ fn parse_range( ); } - let dotdot_pos = dotdot_pos.expect("Internal error: range .. can't be found but should be"); + let right_number_offset = operator_str.len(); let lhs = numbers[0].to_string().spanned(Span::new( lite_arg_span_start, lite_arg_span_start + dotdot_pos, )); let rhs = numbers[1].to_string().spanned(Span::new( - lite_arg_span_start + dotdot_pos + 2, + lite_arg_span_start + dotdot_pos + right_number_offset, lite_arg_span_start + lite_arg_len, )); let left_hand_open = dotdot_pos == 0; - let right_hand_open = dotdot_pos == lite_arg_len - 2; + let right_hand_open = dotdot_pos == lite_arg_len - right_number_offset; let left = if left_hand_open { None @@ -292,10 +302,10 @@ fn parse_range( SpannedExpression::new( Expression::range( left, - Span::new( + operator.spanned(Span::new( lite_arg_span_start + dotdot_pos, - lite_arg_span_start + dotdot_pos + 2, - ), + lite_arg_span_start + dotdot_pos + right_number_offset, + )), right, ), lite_arg.span, diff --git a/crates/nu-parser/src/shapes.rs b/crates/nu-parser/src/shapes.rs index 78b3f91d6..e87c71ed7 100644 --- a/crates/nu-parser/src/shapes.rs +++ b/crates/nu-parser/src/shapes.rs @@ -68,7 +68,13 @@ pub fn expression_to_flat_shape(e: &SpannedExpression) -> Vec if let Some(left) = &range.left { output.append(&mut expression_to_flat_shape(left)); } - output.push(FlatShape::DotDot.spanned(range.dotdot)); + output.push( + match &range.operator.item { + RangeOperator::Inclusive => FlatShape::DotDot, + RangeOperator::RightExclusive => FlatShape::DotDotLeftAngleBracket, + } + .spanned(&range.operator.span), + ); if let Some(right) = &range.right { output.append(&mut expression_to_flat_shape(right)); } diff --git a/crates/nu-protocol/src/hir.rs b/crates/nu-protocol/src/hir.rs index 5b3f6cbae..0b5386dfb 100644 --- a/crates/nu-protocol/src/hir.rs +++ b/crates/nu-protocol/src/hir.rs @@ -906,7 +906,7 @@ impl ShellTypeName for Synthetic { #[derive(Debug, Ord, PartialOrd, Eq, PartialEq, Clone, Hash, Deserialize, Serialize)] pub struct Range { pub left: Option, - pub dotdot: Span, + pub operator: Spanned, pub right: Option, } @@ -919,7 +919,7 @@ impl PrettyDebugWithSource for Range { } else { DebugDocBuilder::blank() }) + b::space() - + b::keyword(self.dotdot.slice(source)) + + b::keyword(self.operator.span().slice(source)) + b::space() + (if let Some(right) = &self.right { right.pretty_debug(source) @@ -932,6 +932,12 @@ impl PrettyDebugWithSource for Range { } } +#[derive(Debug, Ord, PartialOrd, Eq, PartialEq, Clone, Hash, Deserialize, Serialize)] +pub enum RangeOperator { + Inclusive, + RightExclusive, +} + #[derive(Debug, Ord, PartialOrd, Eq, PartialEq, Clone, Hash, Deserialize, Serialize)] pub enum Literal { Number(Number), @@ -1107,12 +1113,12 @@ impl Expression { pub fn range( left: Option, - dotdot: Span, + operator: Spanned, right: Option, ) -> Expression { Expression::Range(Box::new(Range { left, - dotdot, + operator, right, })) } @@ -1291,6 +1297,7 @@ pub enum FlatShape { Operator, Dot, DotDot, + DotDotLeftAngleBracket, InternalCommand, ExternalCommand, ExternalWord, diff --git a/crates/nu-protocol/src/value/primitive.rs b/crates/nu-protocol/src/value/primitive.rs index 0e22d6f18..2aad1ec62 100644 --- a/crates/nu-protocol/src/value/primitive.rs +++ b/crates/nu-protocol/src/value/primitive.rs @@ -1,6 +1,6 @@ use crate::type_name::ShellTypeName; use crate::value::column_path::ColumnPath; -use crate::value::range::Range; +use crate::value::range::{Range, RangeInclusion}; use crate::value::{serde_bigdecimal, serde_bigint}; use bigdecimal::BigDecimal; use chrono::{DateTime, Utc}; @@ -251,8 +251,13 @@ pub fn format_primitive(primitive: &Primitive, field_name: Option<&String>) -> S Primitive::Int(i) => i.to_string(), Primitive::Decimal(decimal) => format!("{:.4}", decimal), Primitive::Range(range) => format!( - "{}..{}", + "{}..{}{}", format_primitive(&range.from.0.item, None), + if range.to.1 == RangeInclusion::Exclusive { + "<" + } else { + "" + }, format_primitive(&range.to.0.item, None) ), Primitive::Pattern(s) => s.to_string(), diff --git a/tests/shell/pipeline/commands/internal.rs b/tests/shell/pipeline/commands/internal.rs index 065f7d10b..eefb9dfb5 100644 --- a/tests/shell/pipeline/commands/internal.rs +++ b/tests/shell/pipeline/commands/internal.rs @@ -418,6 +418,18 @@ fn echoing_ranges() { assert_eq!(actual.out, "6"); } +#[test] +fn echoing_exclusive_ranges() { + let actual = nu!( + cwd: ".", + r#" + echo 1..<4 | math sum + "# + ); + + assert_eq!(actual.out, "6"); +} + #[test] fn table_literals1() { let actual = nu!( @@ -490,6 +502,18 @@ fn range_with_open_left() { assert_eq!(actual.out, "465"); } +#[test] +fn exclusive_range_with_open_left() { + let actual = nu!( + cwd: ".", + r#" + echo ..<31 | math sum + "# + ); + + assert_eq!(actual.out, "465"); +} + #[test] fn range_with_open_right() { let actual = nu!( @@ -502,6 +526,18 @@ fn range_with_open_right() { assert_eq!(actual.out, "95"); } +#[test] +fn exclusive_range_with_open_right() { + let actual = nu!( + cwd: ".", + r#" + echo 5..< | first 10 | math sum + "# + ); + + assert_eq!(actual.out, "95"); +} + #[test] fn range_with_mixed_types() { let actual = nu!( @@ -514,6 +550,18 @@ fn range_with_mixed_types() { assert_eq!(actual.out, "55"); } +#[test] +fn exclusive_range_with_mixed_types() { + let actual = nu!( + cwd: ".", + r#" + echo 1..<10.5 | math sum + "# + ); + + assert_eq!(actual.out, "55"); +} + #[test] fn it_expansion_of_tables() { let actual = nu!(