diff --git a/crates/nu-cmd-base/src/util.rs b/crates/nu-cmd-base/src/util.rs index 44f782492b..7a790725dd 100644 --- a/crates/nu-cmd-base/src/util.rs +++ b/crates/nu-cmd-base/src/util.rs @@ -1,9 +1,8 @@ use nu_protocol::{ - ast::RangeInclusion, engine::{EngineState, Stack, StateWorkingSet}, report_error, Range, ShellError, Span, Value, }; -use std::path::PathBuf; +use std::{ops::Bound, path::PathBuf}; pub fn get_init_cwd() -> PathBuf { std::env::current_dir().unwrap_or_else(|_| { @@ -24,35 +23,21 @@ pub fn get_guaranteed_cwd(engine_state: &EngineState, stack: &Stack) -> PathBuf type MakeRangeError = fn(&str, Span) -> ShellError; pub fn process_range(range: &Range) -> Result<(isize, isize), MakeRangeError> { - let start = match &range.from { - Value::Int { val, .. } => isize::try_from(*val).unwrap_or_default(), - Value::Nothing { .. } => 0, - _ => { - return Err(|msg, span| ShellError::TypeMismatch { - err_message: msg.to_string(), - span, - }) + match range { + Range::IntRange(range) => { + let start = range.start().try_into().unwrap_or(0); + let end = match range.end() { + Bound::Included(v) => v as isize, + Bound::Excluded(v) => (v - 1) as isize, + Bound::Unbounded => isize::MAX, + }; + Ok((start, end)) } - }; - - let end = match &range.to { - Value::Int { val, .. } => { - if matches!(range.inclusion, RangeInclusion::Inclusive) { - isize::try_from(*val).unwrap_or(isize::max_value()) - } else { - isize::try_from(*val).unwrap_or(isize::max_value()) - 1 - } - } - Value::Nothing { .. } => isize::max_value(), - _ => { - return Err(|msg, span| ShellError::TypeMismatch { - err_message: msg.to_string(), - span, - }) - } - }; - - Ok((start, end)) + Range::FloatRange(_) => Err(|msg, span| ShellError::TypeMismatch { + err_message: msg.to_string(), + span, + }), + } } const HELP_MSG: &str = "Nushell's config file can be found with the command: $nu.config-path. \ diff --git a/crates/nu-cmd-lang/src/core_commands/for_.rs b/crates/nu-cmd-lang/src/core_commands/for_.rs index 8d1fa97cb0..cb6de1fda4 100644 --- a/crates/nu-cmd-lang/src/core_commands/for_.rs +++ b/crates/nu-cmd-lang/src/core_commands/for_.rs @@ -69,7 +69,7 @@ impl Command for For { let eval_expression = get_eval_expression(engine_state); let eval_block = get_eval_block(engine_state); - let values = eval_expression(engine_state, stack, keyword_expr)?; + let value = eval_expression(engine_state, stack, keyword_expr)?; let block: Block = call.req(engine_state, stack, 2)?; @@ -81,7 +81,8 @@ impl Command for For { let stack = &mut stack.push_redirection(None, None); - match values { + let span = value.span(); + match value { Value::List { vals, .. } => { for (idx, x) in ListStream::from_stream(vals.into_iter(), ctrlc).enumerate() { // with_env() is used here to ensure that each iteration uses @@ -125,7 +126,7 @@ impl Command for For { } } Value::Range { val, .. } => { - for (idx, x) in val.into_range_iter(ctrlc)?.enumerate() { + for (idx, x) in val.into_range_iter(span, ctrlc).enumerate() { stack.add_var( var_id, if numbered { diff --git a/crates/nu-cmd-lang/src/example_support.rs b/crates/nu-cmd-lang/src/example_support.rs index aa2cbb10ce..ed2778314e 100644 --- a/crates/nu-cmd-lang/src/example_support.rs +++ b/crates/nu-cmd-lang/src/example_support.rs @@ -1,11 +1,15 @@ use itertools::Itertools; use nu_engine::command_prelude::*; use nu_protocol::{ - ast::{Block, RangeInclusion}, + ast::Block, debugger::WithoutDebug, engine::{StateDelta, StateWorkingSet}, + Range, +}; +use std::{ + sync::Arc, + {collections::HashSet, ops::Bound}, }; -use std::{collections::HashSet, sync::Arc}; pub fn check_example_input_and_output_types_match_command_signature( example: &Example, @@ -219,17 +223,45 @@ impl<'a> std::fmt::Debug for DebuggableValue<'a> { Value::Date { val, .. } => { write!(f, "Date({:?})", val) } - Value::Range { val, .. } => match val.inclusion { - RangeInclusion::Inclusive => write!( - f, - "Range({:?}..{:?}, step: {:?})", - val.from, val.to, val.incr - ), - RangeInclusion::RightExclusive => write!( - f, - "Range({:?}..<{:?}, step: {:?})", - val.from, val.to, val.incr - ), + Value::Range { val, .. } => match val { + Range::IntRange(range) => match range.end() { + Bound::Included(end) => write!( + f, + "Range({:?}..{:?}, step: {:?})", + range.start(), + end, + range.step(), + ), + Bound::Excluded(end) => write!( + f, + "Range({:?}..<{:?}, step: {:?})", + range.start(), + end, + range.step(), + ), + Bound::Unbounded => { + write!(f, "Range({:?}.., step: {:?})", range.start(), range.step()) + } + }, + Range::FloatRange(range) => match range.end() { + Bound::Included(end) => write!( + f, + "Range({:?}..{:?}, step: {:?})", + range.start(), + end, + range.step(), + ), + Bound::Excluded(end) => write!( + f, + "Range({:?}..<{:?}, step: {:?})", + range.start(), + end, + range.step(), + ), + Bound::Unbounded => { + write!(f, "Range({:?}.., step: {:?})", range.start(), range.step()) + } + }, }, Value::String { val, .. } | Value::Glob { val, .. } => { write!(f, "{:?}", val) diff --git a/crates/nu-command/src/conversions/into/record.rs b/crates/nu-command/src/conversions/into/record.rs index e0d3516654..3df3aaf1b1 100644 --- a/crates/nu-command/src/conversions/into/record.rs +++ b/crates/nu-command/src/conversions/into/record.rs @@ -125,7 +125,7 @@ fn into_record( ), }, Value::Range { val, .. } => Value::record( - val.into_range_iter(engine_state.ctrlc.clone())? + val.into_range_iter(span, engine_state.ctrlc.clone()) .enumerate() .map(|(idx, val)| (format!("{idx}"), val)) .collect(), diff --git a/crates/nu-command/src/debug/explain.rs b/crates/nu-command/src/debug/explain.rs index 99fb26c9cd..60e917783b 100644 --- a/crates/nu-command/src/debug/explain.rs +++ b/crates/nu-command/src/debug/explain.rs @@ -257,13 +257,7 @@ pub fn debug_string_without_formatting(value: &Value) -> String { Value::Filesize { val, .. } => val.to_string(), Value::Duration { val, .. } => val.to_string(), Value::Date { val, .. } => format!("{val:?}"), - Value::Range { val, .. } => { - format!( - "{}..{}", - debug_string_without_formatting(&val.from), - debug_string_without_formatting(&val.to) - ) - } + Value::Range { val, .. } => val.to_string(), Value::String { val, .. } => val.clone(), Value::Glob { val, .. } => val.clone(), Value::List { vals: val, .. } => format!( diff --git a/crates/nu-command/src/filters/drop/nth.rs b/crates/nu-command/src/filters/drop/nth.rs index bd43f2f6ec..b0858b19cc 100644 --- a/crates/nu-command/src/filters/drop/nth.rs +++ b/crates/nu-command/src/filters/drop/nth.rs @@ -1,6 +1,7 @@ use itertools::Either; use nu_engine::command_prelude::*; -use nu_protocol::{ast::RangeInclusion, PipelineIterator, Range}; +use nu_protocol::{PipelineIterator, Range}; +use std::ops::Bound; #[derive(Clone)] pub struct DropNth; @@ -101,8 +102,8 @@ impl Command for DropNth { ) -> Result { let metadata = input.metadata(); let number_or_range = extract_int_or_range(engine_state, stack, call)?; - let mut lower_bound = None; - let rows = match number_or_range { + + let rows = match number_or_range.item { Either::Left(row_number) => { let and_rows: Vec> = call.rest(engine_state, stack, 1)?; let mut rows: Vec<_> = and_rows.into_iter().map(|x| x.item as usize).collect(); @@ -110,66 +111,71 @@ impl Command for DropNth { rows.sort_unstable(); rows } - Either::Right(row_range) => { - let from = row_range.from.as_int()?; // as usize; - let to = row_range.to.as_int()?; // as usize; - + Either::Right(Range::FloatRange(_)) => { + return Err(ShellError::UnsupportedInput { + msg: "float range".into(), + input: "value originates from here".into(), + msg_span: call.head, + input_span: number_or_range.span, + }); + } + Either::Right(Range::IntRange(range)) => { // check for negative range inputs, e.g., (2..-5) - if from.is_negative() || to.is_negative() { - let span: Spanned = call.req(engine_state, stack, 0)?; - return Err(ShellError::TypeMismatch { - err_message: "drop nth accepts only positive ints".to_string(), - span: span.span, + let end_negative = match range.end() { + Bound::Included(end) | Bound::Excluded(end) => end < 0, + Bound::Unbounded => false, + }; + if range.start().is_negative() || end_negative { + return Err(ShellError::UnsupportedInput { + msg: "drop nth accepts only positive ints".into(), + input: "value originates from here".into(), + msg_span: call.head, + input_span: number_or_range.span, }); } // check if the upper bound is smaller than the lower bound, e.g., do not accept 4..2 - if to < from { - let span: Spanned = call.req(engine_state, stack, 0)?; - return Err(ShellError::TypeMismatch { - err_message: - "The upper bound needs to be equal or larger to the lower bound" - .to_string(), - span: span.span, + if range.step() < 0 { + return Err(ShellError::UnsupportedInput { + msg: "The upper bound needs to be equal or larger to the lower bound" + .into(), + input: "value originates from here".into(), + msg_span: call.head, + input_span: number_or_range.span, }); } - // check for equality to isize::MAX because for some reason, - // the parser returns isize::MAX when we provide a range without upper bound (e.g., 5.. ) - let mut to = to as usize; - let from = from as usize; + let start = range.start() as usize; - if let PipelineData::Value(Value::List { ref vals, .. }, _) = input { - let max = from + vals.len() - 1; - if to > max { - to = max; + let end = match range.end() { + Bound::Included(end) => end as usize, + Bound::Excluded(end) => (end - 1) as usize, + Bound::Unbounded => { + return Ok(input + .into_iter() + .take(start) + .into_pipeline_data_with_metadata( + metadata, + engine_state.ctrlc.clone(), + )) } }; - if to > 0 && to as isize == isize::MAX { - lower_bound = Some(from); - vec![from] - } else if matches!(row_range.inclusion, RangeInclusion::Inclusive) { - (from..=to).collect() + let end = if let PipelineData::Value(Value::List { vals, .. }, _) = &input { + end.min(vals.len() - 1) } else { - (from..to).collect() - } + end + }; + + (start..=end).collect() } }; - if let Some(lower_bound) = lower_bound { - Ok(input - .into_iter() - .take(lower_bound) - .collect::>() - .into_pipeline_data_with_metadata(metadata, engine_state.ctrlc.clone())) - } else { - Ok(DropNthIterator { - input: input.into_iter(), - rows, - current: 0, - } - .into_pipeline_data_with_metadata(metadata, engine_state.ctrlc.clone())) + Ok(DropNthIterator { + input: input.into_iter(), + rows, + current: 0, } + .into_pipeline_data_with_metadata(metadata, engine_state.ctrlc.clone())) } } @@ -177,11 +183,11 @@ fn extract_int_or_range( engine_state: &EngineState, stack: &mut Stack, call: &Call, -) -> Result, ShellError> { - let value = call.req::(engine_state, stack, 0)?; +) -> Result>, ShellError> { + let value: Value = call.req(engine_state, stack, 0)?; let int_opt = value.as_int().map(Either::Left).ok(); - let range_opt = value.as_range().map(|r| Either::Right(r.clone())).ok(); + let range_opt = value.as_range().map(Either::Right).ok(); int_opt .or(range_opt) @@ -189,6 +195,10 @@ fn extract_int_or_range( err_message: "int or range".into(), span: value.span(), }) + .map(|either| Spanned { + item: either, + span: value.span(), + }) } struct DropNthIterator { diff --git a/crates/nu-command/src/filters/first.rs b/crates/nu-command/src/filters/first.rs index ca575ebab1..13408c00bb 100644 --- a/crates/nu-command/src/filters/first.rs +++ b/crates/nu-command/src/filters/first.rs @@ -133,11 +133,15 @@ fn first_helper( } } Value::Range { val, .. } => { + let mut iter = val.into_range_iter(span, ctrlc.clone()); if return_single_element { - Ok(val.from.into_pipeline_data()) + if let Some(v) = iter.next() { + Ok(v.into_pipeline_data()) + } else { + Err(ShellError::AccessEmptyContent { span: head }) + } } else { - Ok(val - .into_range_iter(ctrlc.clone())? + Ok(iter .take(rows_desired) .into_pipeline_data_with_metadata(metadata, ctrlc)) } diff --git a/crates/nu-command/src/filters/par_each.rs b/crates/nu-command/src/filters/par_each.rs index 096ab9655f..9d98787c77 100644 --- a/crates/nu-command/src/filters/par_each.rs +++ b/crates/nu-command/src/filters/par_each.rs @@ -139,84 +139,104 @@ impl Command for ParEach { match input { PipelineData::Empty => Ok(PipelineData::Empty), - PipelineData::Value(Value::Range { val, .. }, ..) => Ok(create_pool(max_threads)? - .install(|| { - let vec = val - .into_range_iter(ctrlc.clone()) - .expect("unable to create a range iterator") - .enumerate() - .par_bridge() - .map(move |(index, x)| { - let block = engine_state.get_block(block_id); + PipelineData::Value(value, ..) => { + let span = value.span(); + match value { + Value::List { vals, .. } => Ok(create_pool(max_threads)?.install(|| { + let vec = vals + .par_iter() + .enumerate() + .map(move |(index, x)| { + let block = engine_state.get_block(block_id); - let mut stack = stack.clone(); + let mut stack = stack.clone(); - if let Some(var) = block.signature.get_positional(0) { - if let Some(var_id) = &var.var_id { - stack.add_var(*var_id, x.clone()); + if let Some(var) = block.signature.get_positional(0) { + if let Some(var_id) = &var.var_id { + stack.add_var(*var_id, x.clone()); + } } - } - let val_span = x.span(); - let x_is_error = x.is_error(); + let val_span = x.span(); + let x_is_error = x.is_error(); - let val = match eval_block_with_early_return( - engine_state, - &mut stack, - block, - x.into_pipeline_data(), - ) { - Ok(v) => v.into_value(span), - Err(error) => Value::error( - chain_error_with_input(error, x_is_error, val_span), - val_span, - ), - }; + let val = match eval_block_with_early_return( + engine_state, + &mut stack, + block, + x.clone().into_pipeline_data(), + ) { + Ok(v) => v.into_value(span), + Err(error) => Value::error( + chain_error_with_input(error, x_is_error, val_span), + val_span, + ), + }; - (index, val) - }) - .collect::>(); + (index, val) + }) + .collect::>(); - apply_order(vec).into_pipeline_data(ctrlc) - })), - PipelineData::Value(Value::List { vals: val, .. }, ..) => Ok(create_pool(max_threads)? - .install(|| { - let vec = val - .par_iter() - .enumerate() - .map(move |(index, x)| { - let block = engine_state.get_block(block_id); + apply_order(vec).into_pipeline_data(ctrlc) + })), + Value::Range { val, .. } => Ok(create_pool(max_threads)?.install(|| { + let vec = val + .into_range_iter(span, ctrlc.clone()) + .enumerate() + .par_bridge() + .map(move |(index, x)| { + let block = engine_state.get_block(block_id); - let mut stack = stack.clone(); + let mut stack = stack.clone(); - if let Some(var) = block.signature.get_positional(0) { - if let Some(var_id) = &var.var_id { - stack.add_var(*var_id, x.clone()); + if let Some(var) = block.signature.get_positional(0) { + if let Some(var_id) = &var.var_id { + stack.add_var(*var_id, x.clone()); + } } + + let val_span = x.span(); + let x_is_error = x.is_error(); + + let val = match eval_block_with_early_return( + engine_state, + &mut stack, + block, + x.into_pipeline_data(), + ) { + Ok(v) => v.into_value(span), + Err(error) => Value::error( + chain_error_with_input(error, x_is_error, val_span), + val_span, + ), + }; + + (index, val) + }) + .collect::>(); + + apply_order(vec).into_pipeline_data(ctrlc) + })), + // This match allows non-iterables to be accepted, + // which is currently considered undesirable (Nov 2022). + value => { + let block = engine_state.get_block(block_id); + + if let Some(var) = block.signature.get_positional(0) { + if let Some(var_id) = &var.var_id { + stack.add_var(*var_id, value.clone()); } + } - let val_span = x.span(); - let x_is_error = x.is_error(); - - let val = match eval_block_with_early_return( - engine_state, - &mut stack, - block, - x.clone().into_pipeline_data(), - ) { - Ok(v) => v.into_value(span), - Err(error) => Value::error( - chain_error_with_input(error, x_is_error, val_span), - val_span, - ), - }; - - (index, val) - }) - .collect::>(); - - apply_order(vec).into_pipeline_data(ctrlc) - })), + eval_block_with_early_return( + engine_state, + &mut stack, + block, + value.into_pipeline_data(), + ) + } + } + } PipelineData::ListStream(stream, ..) => Ok(create_pool(max_threads)?.install(|| { let vec = stream .enumerate() @@ -294,24 +314,6 @@ impl Command for ParEach { apply_order(vec).into_pipeline_data(ctrlc) })), - // This match allows non-iterables to be accepted, - // which is currently considered undesirable (Nov 2022). - PipelineData::Value(x, ..) => { - let block = engine_state.get_block(block_id); - - if let Some(var) = block.signature.get_positional(0) { - if let Some(var_id) = &var.var_id { - stack.add_var(*var_id, x.clone()); - } - } - - eval_block_with_early_return( - engine_state, - &mut stack, - block, - x.into_pipeline_data(), - ) - } } .and_then(|x| x.filter(|v| !v.is_nothing(), outer_ctrlc)) .map(|res| res.set_metadata(metadata)) diff --git a/crates/nu-command/src/filters/range.rs b/crates/nu-command/src/filters/range.rs index db245b4d2c..08d1c0fe42 100644 --- a/crates/nu-command/src/filters/range.rs +++ b/crates/nu-command/src/filters/range.rs @@ -1,5 +1,6 @@ use nu_engine::command_prelude::*; -use nu_protocol::ast::RangeInclusion; +use nu_protocol::Range as NumRange; +use std::ops::Bound; #[derive(Clone)] pub struct Range; @@ -64,59 +65,68 @@ impl Command for Range { input: PipelineData, ) -> Result { let metadata = input.metadata(); - let rows: nu_protocol::Range = call.req(engine_state, stack, 0)?; + let rows: Spanned = call.req(engine_state, stack, 0)?; - let rows_from = get_range_val(rows.from); - let rows_to = if rows.inclusion == RangeInclusion::RightExclusive { - get_range_val(rows.to) - 1 - } else { - get_range_val(rows.to) - }; + match rows.item { + NumRange::IntRange(range) => { + let start = range.start(); + let end = match range.end() { + Bound::Included(end) => end, + Bound::Excluded(end) => end - 1, + Bound::Unbounded => { + if range.step() < 0 { + i64::MIN + } else { + i64::MAX + } + } + }; - // only collect the input if we have any negative indices - if rows_from < 0 || rows_to < 0 { - let v: Vec<_> = input.into_iter().collect(); - let vlen: i64 = v.len() as i64; + // only collect the input if we have any negative indices + if start < 0 || end < 0 { + let v: Vec<_> = input.into_iter().collect(); + let vlen: i64 = v.len() as i64; - let from = if rows_from < 0 { - (vlen + rows_from) as usize - } else { - rows_from as usize - }; + let from = if start < 0 { + (vlen + start) as usize + } else { + start as usize + }; - let to = if rows_to < 0 { - (vlen + rows_to) as usize - } else if rows_to > v.len() as i64 { - v.len() - } else { - rows_to as usize - }; + let to = if end < 0 { + (vlen + end) as usize + } else if end > v.len() as i64 { + v.len() + } else { + end as usize + }; - if from > to { - Ok(PipelineData::Value(Value::nothing(call.head), None)) - } else { - let iter = v.into_iter().skip(from).take(to - from + 1); - Ok(iter.into_pipeline_data(engine_state.ctrlc.clone())) - } - } else { - let from = rows_from as usize; - let to = rows_to as usize; - - if from > to { - Ok(PipelineData::Value(Value::nothing(call.head), None)) - } else { - let iter = input.into_iter().skip(from).take(to - from + 1); - Ok(iter.into_pipeline_data(engine_state.ctrlc.clone())) + if from > to { + Ok(PipelineData::Value(Value::nothing(call.head), None)) + } else { + let iter = v.into_iter().skip(from).take(to - from + 1); + Ok(iter.into_pipeline_data(engine_state.ctrlc.clone())) + } + } else { + let from = start as usize; + let to = end as usize; + + if from > to { + Ok(PipelineData::Value(Value::nothing(call.head), None)) + } else { + let iter = input.into_iter().skip(from).take(to - from + 1); + Ok(iter.into_pipeline_data(engine_state.ctrlc.clone())) + } + } + .map(|x| x.set_metadata(metadata)) } + NumRange::FloatRange(_) => Err(ShellError::UnsupportedInput { + msg: "float range".into(), + input: "value originates from here".into(), + msg_span: call.head, + input_span: rows.span, + }), } - .map(|x| x.set_metadata(metadata)) - } -} - -fn get_range_val(rows_val: Value) -> i64 { - match rows_val { - Value::Int { val: x, .. } => x, - _ => 0, } } diff --git a/crates/nu-command/src/filters/take/take_.rs b/crates/nu-command/src/filters/take/take_.rs index 2586942e4c..dc092c9779 100644 --- a/crates/nu-command/src/filters/take/take_.rs +++ b/crates/nu-command/src/filters/take/take_.rs @@ -60,7 +60,7 @@ impl Command for Take { Ok(PipelineData::Value(Value::binary(slice, span), metadata)) } Value::Range { val, .. } => Ok(val - .into_range_iter(ctrlc.clone())? + .into_range_iter(span, ctrlc.clone()) .take(rows_desired) .into_pipeline_data_with_metadata(metadata, ctrlc)), // Propagate errors by explicitly matching them before the final case. diff --git a/crates/nu-command/src/formats/from/nuon.rs b/crates/nu-command/src/formats/from/nuon.rs index e74c1fe5eb..52ef62bb48 100644 --- a/crates/nu-command/src/formats/from/nuon.rs +++ b/crates/nu-command/src/formats/from/nuon.rs @@ -292,7 +292,7 @@ fn convert_to_value( }; Ok(Value::range( - Range::new(expr.span, from, next, to, &operator)?, + Range::new(from, next, to, operator.inclusion, expr.span)?, expr.span, )) } diff --git a/crates/nu-command/src/formats/to/nuon.rs b/crates/nu-command/src/formats/to/nuon.rs index 84b237e52e..f1b0c5ccf5 100644 --- a/crates/nu-command/src/formats/to/nuon.rs +++ b/crates/nu-command/src/formats/to/nuon.rs @@ -2,8 +2,9 @@ use core::fmt::Write; use fancy_regex::Regex; use nu_engine::{command_prelude::*, get_columns}; use nu_parser::escape_quote_string; -use nu_protocol::ast::RangeInclusion; +use nu_protocol::Range; use once_cell::sync::Lazy; +use std::ops::Bound; #[derive(Clone)] pub struct ToNuon; @@ -234,16 +235,26 @@ pub fn value_to_string( } } Value::Nothing { .. } => Ok("null".to_string()), - Value::Range { val, .. } => Ok(format!( - "{}..{}{}", - value_to_string(&val.from, span, depth + 1, indent)?, - if val.inclusion == RangeInclusion::RightExclusive { - "<" - } else { - "" - }, - value_to_string(&val.to, span, depth + 1, indent)? - )), + Value::Range { val, .. } => match val { + Range::IntRange(range) => Ok(range.to_string()), + Range::FloatRange(range) => { + let start = + value_to_string(&Value::float(range.start(), span), span, depth + 1, indent)?; + match range.end() { + Bound::Included(end) => Ok(format!( + "{}..{}", + start, + value_to_string(&Value::float(end, span), span, depth + 1, indent)? + )), + Bound::Excluded(end) => Ok(format!( + "{}..<{}", + start, + value_to_string(&Value::float(end, span), span, depth + 1, indent)? + )), + Bound::Unbounded => Ok(format!("{start}..",)), + } + } + }, Value::Record { val, .. } => { let mut collection = vec![]; for (col, val) in &**val { diff --git a/crates/nu-command/src/formats/to/text.rs b/crates/nu-command/src/formats/to/text.rs index 873b2c8a08..07f91bb258 100644 --- a/crates/nu-command/src/formats/to/text.rs +++ b/crates/nu-command/src/formats/to/text.rs @@ -116,13 +116,7 @@ fn local_into_string(value: Value, separator: &str, config: &Config) -> String { Value::Date { val, .. } => { format!("{} ({})", val.to_rfc2822(), HumanTime::from(val)) } - Value::Range { val, .. } => { - format!( - "{}..{}", - local_into_string(val.from, ", ", config), - local_into_string(val.to, ", ", config) - ) - } + Value::Range { val, .. } => val.to_string(), Value::String { val, .. } => val, Value::Glob { val, .. } => val, Value::List { vals: val, .. } => val diff --git a/crates/nu-command/src/math/utils.rs b/crates/nu-command/src/math/utils.rs index 37c4bd1c0e..82ecf2f50b 100644 --- a/crates/nu-command/src/math/utils.rs +++ b/crates/nu-command/src/math/utils.rs @@ -92,7 +92,7 @@ pub fn calculate( } PipelineData::Value(Value::Range { val, .. }, ..) => { let new_vals: Result, ShellError> = val - .into_range_iter(None)? + .into_range_iter(span, None) .map(|val| mf(&[val], span, name)) .collect(); diff --git a/crates/nu-command/src/random/float.rs b/crates/nu-command/src/random/float.rs index 197c85b533..c34e468600 100644 --- a/crates/nu-command/src/random/float.rs +++ b/crates/nu-command/src/random/float.rs @@ -1,7 +1,7 @@ use nu_engine::command_prelude::*; -use nu_protocol::Range; +use nu_protocol::{FloatRange, Range}; use rand::prelude::{thread_rng, Rng}; -use std::cmp::Ordering; +use std::ops::Bound; #[derive(Clone)] pub struct SubCommand; @@ -68,43 +68,39 @@ fn float( stack: &mut Stack, call: &Call, ) -> Result { - let mut range_span = call.head; + let span = call.head; let range: Option> = call.opt(engine_state, stack, 0)?; - let (min, max) = if let Some(spanned_range) = range { - let r = spanned_range.item; - range_span = spanned_range.span; + let mut thread_rng = thread_rng(); - if r.is_end_inclusive() { - (r.from.coerce_float()?, r.to.coerce_float()?) - } else if r.to.coerce_float()? >= 1.0 { - (r.from.coerce_float()?, r.to.coerce_float()? - 1.0) - } else { - (0.0, 0.0) + match range { + Some(range) => { + let range_span = range.span; + let range = FloatRange::from(range.item); + + if range.step() < 0.0 { + return Err(ShellError::InvalidRange { + left_flank: range.start().to_string(), + right_flank: match range.end() { + Bound::Included(end) | Bound::Excluded(end) => end.to_string(), + Bound::Unbounded => "".into(), + }, + span: range_span, + }); + } + + let value = match range.end() { + Bound::Included(end) => thread_rng.gen_range(range.start()..=end), + Bound::Excluded(end) => thread_rng.gen_range(range.start()..end), + Bound::Unbounded => thread_rng.gen_range(range.start()..f64::INFINITY), + }; + + Ok(PipelineData::Value(Value::float(value, span), None)) } - } else { - (0.0, 1.0) - }; - - match min.partial_cmp(&max) { - Some(Ordering::Greater) => Err(ShellError::InvalidRange { - left_flank: min.to_string(), - right_flank: max.to_string(), - span: range_span, - }), - Some(Ordering::Equal) => Ok(PipelineData::Value( - Value::float(min, Span::new(64, 64)), + None => Ok(PipelineData::Value( + Value::float(thread_rng.gen_range(0.0..1.0), span), None, )), - _ => { - let mut thread_rng = thread_rng(); - let result: f64 = thread_rng.gen_range(min..max); - - Ok(PipelineData::Value( - Value::float(result, Span::new(64, 64)), - None, - )) - } } } diff --git a/crates/nu-command/src/random/int.rs b/crates/nu-command/src/random/int.rs index f2a1f22a62..6a5467b010 100644 --- a/crates/nu-command/src/random/int.rs +++ b/crates/nu-command/src/random/int.rs @@ -1,7 +1,7 @@ use nu_engine::command_prelude::*; use nu_protocol::Range; use rand::prelude::{thread_rng, Rng}; -use std::cmp::Ordering; +use std::ops::Bound; #[derive(Clone)] pub struct SubCommand; @@ -71,34 +71,44 @@ fn integer( let span = call.head; let range: Option> = call.opt(engine_state, stack, 0)?; - let mut range_span = call.head; - let (min, max) = if let Some(spanned_range) = range { - let r = spanned_range.item; - range_span = spanned_range.span; - if r.is_end_inclusive() { - (r.from.as_int()?, r.to.as_int()?) - } else if r.to.as_int()? > 0 { - (r.from.as_int()?, r.to.as_int()? - 1) - } else { - (0, 0) - } - } else { - (0, i64::MAX) - }; + let mut thread_rng = thread_rng(); - match min.partial_cmp(&max) { - Some(Ordering::Greater) => Err(ShellError::InvalidRange { - left_flank: min.to_string(), - right_flank: max.to_string(), - span: range_span, - }), - Some(Ordering::Equal) => Ok(PipelineData::Value(Value::int(min, span), None)), - _ => { - let mut thread_rng = thread_rng(); - let result: i64 = thread_rng.gen_range(min..=max); + match range { + Some(range) => { + let range_span = range.span; + match range.item { + Range::IntRange(range) => { + if range.step() < 0 { + return Err(ShellError::InvalidRange { + left_flank: range.start().to_string(), + right_flank: match range.end() { + Bound::Included(end) | Bound::Excluded(end) => end.to_string(), + Bound::Unbounded => "".into(), + }, + span: range_span, + }); + } - Ok(PipelineData::Value(Value::int(result, span), None)) + let value = match range.end() { + Bound::Included(end) => thread_rng.gen_range(range.start()..=end), + Bound::Excluded(end) => thread_rng.gen_range(range.start()..end), + Bound::Unbounded => thread_rng.gen_range(range.start()..=i64::MAX), + }; + + Ok(PipelineData::Value(Value::int(value, span), None)) + } + Range::FloatRange(_) => Err(ShellError::UnsupportedInput { + msg: "float range".into(), + input: "value originates from here".into(), + msg_span: call.head, + input_span: range.span, + }), + } } + None => Ok(PipelineData::Value( + Value::int(thread_rng.gen_range(0..=i64::MAX), span), + None, + )), } } diff --git a/crates/nu-command/src/strings/str_/index_of.rs b/crates/nu-command/src/strings/str_/index_of.rs index 66b378116a..6f6807ef23 100644 --- a/crates/nu-command/src/strings/str_/index_of.rs +++ b/crates/nu-command/src/strings/str_/index_of.rs @@ -251,7 +251,6 @@ mod tests { let options = Arguments { substring: String::from("Lm"), - range: None, cell_paths: None, end: false, @@ -266,12 +265,14 @@ mod tests { #[test] fn returns_index_of_next_substring() { let word = Value::test_string("Cargo.Cargo"); - let range = Range { - from: Value::int(1, Span::test_data()), - incr: Value::int(1, Span::test_data()), - to: Value::nothing(Span::test_data()), - inclusion: RangeInclusion::Inclusive, - }; + let range = Range::new( + Value::int(1, Span::test_data()), + Value::nothing(Span::test_data()), + Value::nothing(Span::test_data()), + RangeInclusion::Inclusive, + Span::test_data(), + ) + .expect("valid range"); let spanned_range = Spanned { item: range, @@ -294,12 +295,14 @@ mod tests { #[test] fn index_does_not_exist_due_to_end_index() { let word = Value::test_string("Cargo.Banana"); - let range = Range { - from: Value::nothing(Span::test_data()), - inclusion: RangeInclusion::Inclusive, - incr: Value::int(1, Span::test_data()), - to: Value::int(5, Span::test_data()), - }; + let range = Range::new( + Value::nothing(Span::test_data()), + Value::nothing(Span::test_data()), + Value::int(5, Span::test_data()), + RangeInclusion::Inclusive, + Span::test_data(), + ) + .expect("valid range"); let spanned_range = Spanned { item: range, @@ -322,12 +325,14 @@ mod tests { #[test] fn returns_index_of_nums_in_middle_due_to_index_limit_from_both_ends() { let word = Value::test_string("123123123"); - let range = Range { - from: Value::int(2, Span::test_data()), - incr: Value::int(1, Span::test_data()), - to: Value::int(6, Span::test_data()), - inclusion: RangeInclusion::Inclusive, - }; + let range = Range::new( + Value::int(2, Span::test_data()), + Value::nothing(Span::test_data()), + Value::int(6, Span::test_data()), + RangeInclusion::Inclusive, + Span::test_data(), + ) + .expect("valid range"); let spanned_range = Spanned { item: range, @@ -350,12 +355,14 @@ mod tests { #[test] fn index_does_not_exists_due_to_strict_bounds() { let word = Value::test_string("123456"); - let range = Range { - from: Value::int(2, Span::test_data()), - incr: Value::int(1, Span::test_data()), - to: Value::int(5, Span::test_data()), - inclusion: RangeInclusion::RightExclusive, - }; + let range = Range::new( + Value::int(2, Span::test_data()), + Value::nothing(Span::test_data()), + Value::int(5, Span::test_data()), + RangeInclusion::RightExclusive, + Span::test_data(), + ) + .expect("valid range"); let spanned_range = Spanned { item: range, @@ -381,7 +388,6 @@ mod tests { let options = Arguments { substring: String::from("ふが"), - range: None, cell_paths: None, end: false, @@ -396,12 +402,14 @@ mod tests { fn index_is_not_a_char_boundary() { let word = Value::string(String::from("💛"), Span::test_data()); - let range = Range { - from: Value::int(0, Span::test_data()), - incr: Value::int(1, Span::test_data()), - to: Value::int(3, Span::test_data()), - inclusion: RangeInclusion::Inclusive, - }; + let range = Range::new( + Value::int(0, Span::test_data()), + Value::int(1, Span::test_data()), + Value::int(3, Span::test_data()), + RangeInclusion::Inclusive, + Span::test_data(), + ) + .expect("valid range"); let spanned_range = Spanned { item: range, @@ -425,12 +433,14 @@ mod tests { fn index_is_out_of_bounds() { let word = Value::string(String::from("hello"), Span::test_data()); - let range = Range { - from: Value::int(-1, Span::test_data()), - incr: Value::int(1, Span::test_data()), - to: Value::int(3, Span::test_data()), - inclusion: RangeInclusion::Inclusive, - }; + let range = Range::new( + Value::int(-1, Span::test_data()), + Value::int(1, Span::test_data()), + Value::int(3, Span::test_data()), + RangeInclusion::Inclusive, + Span::test_data(), + ) + .expect("valid range"); let spanned_range = Spanned { item: range, diff --git a/crates/nu-command/src/viewers/table.rs b/crates/nu-command/src/viewers/table.rs index 4aab650dd7..e831b151f1 100644 --- a/crates/nu-command/src/viewers/table.rs +++ b/crates/nu-command/src/viewers/table.rs @@ -409,7 +409,7 @@ fn handle_table_command( } PipelineData::Value(Value::Range { val, .. }, metadata) => { let ctrlc = input.engine_state.ctrlc.clone(); - let stream = ListStream::from_stream(val.into_range_iter(ctrlc.clone())?, ctrlc); + let stream = ListStream::from_stream(val.into_range_iter(span, ctrlc.clone()), ctrlc); input.data = PipelineData::Empty; handle_row_stream(input, cfg, stream, metadata) } diff --git a/crates/nu-command/tests/format_conversions/json.rs b/crates/nu-command/tests/format_conversions/json.rs index f06736fca0..be3476e222 100644 --- a/crates/nu-command/tests/format_conversions/json.rs +++ b/crates/nu-command/tests/format_conversions/json.rs @@ -229,11 +229,13 @@ fn unbounded_from_in_range_fails() { #[test] fn inf_in_range_fails() { let actual = nu!(r#"inf..5 | to json"#); - assert!(actual.err.contains("Cannot create range")); + assert!(actual.err.contains("can't convert to countable values")); let actual = nu!(r#"5..inf | to json"#); - assert!(actual.err.contains("Cannot create range")); + assert!(actual + .err + .contains("Unbounded ranges are not allowed when converting to this format")); let actual = nu!(r#"-inf..inf | to json"#); - assert!(actual.err.contains("Cannot create range")); + assert!(actual.err.contains("can't convert to countable values")); } #[test] diff --git a/crates/nu-plugin/src/protocol/plugin_custom_value/tests.rs b/crates/nu-plugin/src/protocol/plugin_custom_value/tests.rs index 5136dc665e..ca5368e4f5 100644 --- a/crates/nu-plugin/src/protocol/plugin_custom_value/tests.rs +++ b/crates/nu-plugin/src/protocol/plugin_custom_value/tests.rs @@ -6,10 +6,7 @@ use crate::{ TestCustomValue, }, }; -use nu_protocol::{ - ast::RangeInclusion, engine::Closure, record, CustomValue, IntoSpanned, Range, ShellError, - Span, Value, -}; +use nu_protocol::{engine::Closure, record, CustomValue, IntoSpanned, ShellError, Span, Value}; use std::sync::Arc; #[test] @@ -60,50 +57,6 @@ fn add_source_in_at_root() -> Result<(), ShellError> { Ok(()) } -fn check_range_custom_values( - val: &Value, - mut f: impl FnMut(&str, &dyn CustomValue) -> Result<(), ShellError>, -) -> Result<(), ShellError> { - let range = val.as_range()?; - for (name, val) in [ - ("from", &range.from), - ("incr", &range.incr), - ("to", &range.to), - ] { - let custom_value = val - .as_custom_value() - .unwrap_or_else(|_| panic!("{name} not custom value")); - f(name, custom_value)?; - } - Ok(()) -} - -#[test] -fn add_source_in_nested_range() -> Result<(), ShellError> { - let orig_custom_val = Value::test_custom_value(Box::new(test_plugin_custom_value())); - let mut val = Value::test_range(Range { - from: orig_custom_val.clone(), - incr: orig_custom_val.clone(), - to: orig_custom_val.clone(), - inclusion: RangeInclusion::Inclusive, - }); - let source = Arc::new(PluginSource::new_fake("foo")); - PluginCustomValue::add_source_in(&mut val, &source)?; - - check_range_custom_values(&val, |name, custom_value| { - let plugin_custom_value: &PluginCustomValue = custom_value - .as_any() - .downcast_ref() - .unwrap_or_else(|| panic!("{name} not PluginCustomValue")); - assert_eq!( - Some(Arc::as_ptr(&source)), - plugin_custom_value.source.as_ref().map(Arc::as_ptr), - "{name} source not set correctly" - ); - Ok(()) - }) -} - fn check_record_custom_values( val: &Value, keys: &[&str], @@ -292,31 +245,6 @@ fn serialize_in_root() -> Result<(), ShellError> { Ok(()) } -#[test] -fn serialize_in_range() -> Result<(), ShellError> { - let orig_custom_val = Value::test_custom_value(Box::new(TestCustomValue(-1))); - let mut val = Value::test_range(Range { - from: orig_custom_val.clone(), - incr: orig_custom_val.clone(), - to: orig_custom_val.clone(), - inclusion: RangeInclusion::Inclusive, - }); - PluginCustomValue::serialize_custom_values_in(&mut val)?; - - check_range_custom_values(&val, |name, custom_value| { - let plugin_custom_value: &PluginCustomValue = custom_value - .as_any() - .downcast_ref() - .unwrap_or_else(|| panic!("{name} not PluginCustomValue")); - assert_eq!( - "TestCustomValue", - plugin_custom_value.name(), - "{name} name not set correctly" - ); - Ok(()) - }) -} - #[test] fn serialize_in_record() -> Result<(), ShellError> { let orig_custom_val = Value::test_custom_value(Box::new(TestCustomValue(32))); @@ -400,31 +328,6 @@ fn deserialize_in_root() -> Result<(), ShellError> { Ok(()) } -#[test] -fn deserialize_in_range() -> Result<(), ShellError> { - let orig_custom_val = Value::test_custom_value(Box::new(test_plugin_custom_value())); - let mut val = Value::test_range(Range { - from: orig_custom_val.clone(), - incr: orig_custom_val.clone(), - to: orig_custom_val.clone(), - inclusion: RangeInclusion::Inclusive, - }); - PluginCustomValue::deserialize_custom_values_in(&mut val)?; - - check_range_custom_values(&val, |name, custom_value| { - let test_custom_value: &TestCustomValue = custom_value - .as_any() - .downcast_ref() - .unwrap_or_else(|| panic!("{name} not TestCustomValue")); - assert_eq!( - expected_test_custom_value(), - *test_custom_value, - "{name} not deserialized correctly" - ); - Ok(()) - }) -} - #[test] fn deserialize_in_record() -> Result<(), ShellError> { let orig_custom_val = Value::test_custom_value(Box::new(test_plugin_custom_value())); diff --git a/crates/nu-plugin/src/util/with_custom_values_in.rs b/crates/nu-plugin/src/util/with_custom_values_in.rs index bffb6b3737..83813e04dd 100644 --- a/crates/nu-plugin/src/util/with_custom_values_in.rs +++ b/crates/nu-plugin/src/util/with_custom_values_in.rs @@ -33,7 +33,7 @@ where #[test] fn find_custom_values() { use crate::protocol::test_util::test_plugin_custom_value; - use nu_protocol::{ast::RangeInclusion, engine::Closure, record, LazyRecord, Range, Span}; + use nu_protocol::{engine::Closure, record, LazyRecord, Span}; #[derive(Debug, Clone)] struct Lazy; @@ -73,12 +73,6 @@ fn find_custom_values() { captures: vec![(0, cv.clone()), (1, Value::test_string("foo"))] } ), - "range" => Value::test_range(Range { - from: cv.clone(), - incr: cv.clone(), - to: cv.clone(), - inclusion: RangeInclusion::Inclusive - }), "lazy" => Value::test_lazy_record(Box::new(Lazy)), }); @@ -89,7 +83,7 @@ fn find_custom_values() { Ok(()) }) .expect("error"); - assert_eq!(7, found, "found in value"); + assert_eq!(4, found, "found in value"); // Try it on bare custom value too found = 0; diff --git a/crates/nu-protocol/src/ast/operator.rs b/crates/nu-protocol/src/ast/operator.rs index fa3a528fd8..46484761bc 100644 --- a/crates/nu-protocol/src/ast/operator.rs +++ b/crates/nu-protocol/src/ast/operator.rs @@ -109,7 +109,7 @@ impl Display for Operator { } } -#[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Serialize, Deserialize)] +#[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord, Serialize, Deserialize)] pub enum RangeInclusion { Inclusive, RightExclusive, diff --git a/crates/nu-protocol/src/eval_base.rs b/crates/nu-protocol/src/eval_base.rs index 01955217f1..5b8dc51312 100644 --- a/crates/nu-protocol/src/eval_base.rs +++ b/crates/nu-protocol/src/eval_base.rs @@ -168,8 +168,9 @@ pub trait Eval { } else { Value::nothing(expr.span) }; + Ok(Value::range( - Range::new(expr.span, from, next, to, operator)?, + Range::new(from, next, to, operator.inclusion, expr.span)?, expr.span, )) } diff --git a/crates/nu-protocol/src/pipeline_data/mod.rs b/crates/nu-protocol/src/pipeline_data/mod.rs index e180262037..d9aad30ce4 100644 --- a/crates/nu-protocol/src/pipeline_data/mod.rs +++ b/crates/nu-protocol/src/pipeline_data/mod.rs @@ -9,7 +9,7 @@ pub use stream::*; use crate::{ ast::{Call, PathMember}, engine::{EngineState, Stack, StateWorkingSet}, - format_error, Config, ShellError, Span, Value, + format_error, Config, Range, ShellError, Span, Value, }; use nu_utils::{stderr_write_all_and_flush, stdout_write_all_and_flush}; use std::{ @@ -404,7 +404,7 @@ impl PipelineData { /// It returns Err if the `self` cannot be converted to an iterator. pub fn into_iter_strict(self, span: Span) -> Result { match self { - PipelineData::Value(val, metadata) => match val { + PipelineData::Value(value, metadata) => match value { Value::List { vals, .. } => Ok(PipelineIterator(PipelineData::ListStream( ListStream::from_stream(vals.into_iter(), None), metadata, @@ -416,13 +416,11 @@ impl PipelineData { ), metadata, ))), - Value::Range { val, .. } => match val.into_range_iter(None) { - Ok(iter) => Ok(PipelineIterator(PipelineData::ListStream( - ListStream::from_stream(iter, None), + Value::Range { val, .. } => Ok(PipelineIterator(PipelineData::ListStream( + ListStream::from_stream(val.into_range_iter(value.span(), None), None), metadata, - ))), - Err(error) => Err(error), - }, + ))) + , // Propagate errors by explicitly matching them before the final case. Value::Error { error, .. } => Err(*error), other => Err(ShellError::OnlySupportsThisInputType { @@ -560,8 +558,21 @@ impl PipelineData { F: FnMut(Value) -> Value + 'static + Send, { match self { - PipelineData::Value(Value::List { vals, .. }, ..) => { - Ok(vals.into_iter().map(f).into_pipeline_data(ctrlc)) + PipelineData::Value(value, ..) => { + let span = value.span(); + match value { + Value::List { vals, .. } => { + Ok(vals.into_iter().map(f).into_pipeline_data(ctrlc)) + } + Value::Range { val, .. } => Ok(val + .into_range_iter(span, ctrlc.clone()) + .map(f) + .into_pipeline_data(ctrlc)), + value => match f(value) { + Value::Error { error, .. } => Err(*error), + v => Ok(v.into_pipeline_data()), + }, + } } PipelineData::Empty => Ok(PipelineData::Empty), PipelineData::ListStream(stream, ..) => Ok(stream.map(f).into_pipeline_data(ctrlc)), @@ -582,15 +593,6 @@ impl PipelineData { Ok(f(Value::binary(collected.item, collected.span)).into_pipeline_data()) } } - - PipelineData::Value(Value::Range { val, .. }, ..) => Ok(val - .into_range_iter(ctrlc.clone())? - .map(f) - .into_pipeline_data(ctrlc)), - PipelineData::Value(v, ..) => match f(v) { - Value::Error { error, .. } => Err(*error), - v => Ok(v.into_pipeline_data()), - }, } } @@ -608,8 +610,18 @@ impl PipelineData { { match self { PipelineData::Empty => Ok(PipelineData::Empty), - PipelineData::Value(Value::List { vals, .. }, ..) => { - Ok(vals.into_iter().flat_map(f).into_pipeline_data(ctrlc)) + PipelineData::Value(value, ..) => { + let span = value.span(); + match value { + Value::List { vals, .. } => { + Ok(vals.into_iter().flat_map(f).into_pipeline_data(ctrlc)) + } + Value::Range { val, .. } => Ok(val + .into_range_iter(span, ctrlc.clone()) + .flat_map(f) + .into_pipeline_data(ctrlc)), + value => Ok(f(value).into_iter().into_pipeline_data(ctrlc)), + } } PipelineData::ListStream(stream, ..) => { Ok(stream.flat_map(f).into_pipeline_data(ctrlc)) @@ -635,11 +647,6 @@ impl PipelineData { .into_pipeline_data(ctrlc)) } } - PipelineData::Value(Value::Range { val, .. }, ..) => Ok(val - .into_range_iter(ctrlc.clone())? - .flat_map(f) - .into_pipeline_data(ctrlc)), - PipelineData::Value(v, ..) => Ok(f(v).into_iter().into_pipeline_data(ctrlc)), } } @@ -654,8 +661,24 @@ impl PipelineData { { match self { PipelineData::Empty => Ok(PipelineData::Empty), - PipelineData::Value(Value::List { vals, .. }, ..) => { - Ok(vals.into_iter().filter(f).into_pipeline_data(ctrlc)) + PipelineData::Value(value, ..) => { + let span = value.span(); + match value { + Value::List { vals, .. } => { + Ok(vals.into_iter().filter(f).into_pipeline_data(ctrlc)) + } + Value::Range { val, .. } => Ok(val + .into_range_iter(span, ctrlc.clone()) + .filter(f) + .into_pipeline_data(ctrlc)), + value => { + if f(&value) { + Ok(value.into_pipeline_data()) + } else { + Ok(Value::nothing(span).into_pipeline_data()) + } + } + } } PipelineData::ListStream(stream, ..) => Ok(stream.filter(f).into_pipeline_data(ctrlc)), PipelineData::ExternalStream { stdout: None, .. } => Ok(PipelineData::Empty), @@ -687,17 +710,6 @@ impl PipelineData { } } } - PipelineData::Value(Value::Range { val, .. }, ..) => Ok(val - .into_range_iter(ctrlc.clone())? - .filter(f) - .into_pipeline_data(ctrlc)), - PipelineData::Value(v, ..) => { - if f(&v) { - Ok(v.into_pipeline_data()) - } else { - Ok(Value::nothing(v.span()).into_pipeline_data()) - } - } } } @@ -793,47 +805,43 @@ impl PipelineData { /// converting `to json` or `to nuon`. /// `1..3 | to XX -> [1,2,3]` pub fn try_expand_range(self) -> Result { - let input = match self { - PipelineData::Value(v, metadata) => match v { - Value::Range { val, .. } => { - let span = val.to.span(); - match (&val.to, &val.from) { - (Value::Float { val, .. }, _) | (_, Value::Float { val, .. }) => { - if *val == f64::INFINITY || *val == f64::NEG_INFINITY { - return Err(ShellError::GenericError { - error: "Cannot create range".into(), - msg: "Infinity is not allowed when converting to json".into(), - span: Some(span), - help: Some("Consider removing infinity".into()), - inner: vec![], - }); + match self { + PipelineData::Value(v, metadata) => { + let span = v.span(); + match v { + Value::Range { val, .. } => { + match val { + Range::IntRange(range) => { + if range.is_unbounded() { + return Err(ShellError::GenericError { + error: "Cannot create range".into(), + msg: "Unbounded ranges are not allowed when converting to this format".into(), + span: Some(span), + help: Some("Consider using ranges with valid start and end point.".into()), + inner: vec![], + }); + } + } + Range::FloatRange(range) => { + if range.is_unbounded() { + return Err(ShellError::GenericError { + error: "Cannot create range".into(), + msg: "Unbounded ranges are not allowed when converting to this format".into(), + span: Some(span), + help: Some("Consider using ranges with valid start and end point.".into()), + inner: vec![], + }); + } } } - (Value::Int { val, .. }, _) => { - if *val == i64::MAX || *val == i64::MIN { - return Err(ShellError::GenericError { - error: "Cannot create range".into(), - msg: "Unbounded ranges are not allowed when converting to json" - .into(), - span: Some(span), - help: Some( - "Consider using ranges with valid start and end point." - .into(), - ), - inner: vec![], - }); - } - } - _ => (), + let range_values: Vec = val.into_range_iter(span, None).collect(); + Ok(PipelineData::Value(Value::list(range_values, span), None)) } - let range_values: Vec = val.into_range_iter(None)?.collect(); - PipelineData::Value(Value::list(range_values, span), None) + x => Ok(PipelineData::Value(x, metadata)), } - x => PipelineData::Value(x, metadata), - }, - _ => self, - }; - Ok(input) + } + _ => Ok(self), + } } /// Consume and print self data immediately. @@ -948,26 +956,17 @@ impl IntoIterator for PipelineData { fn into_iter(self) -> Self::IntoIter { match self { - PipelineData::Value(v, metadata) => { - let span = v.span(); - match v { + PipelineData::Value(value, metadata) => { + let span = value.span(); + match value { Value::List { vals, .. } => PipelineIterator(PipelineData::ListStream( ListStream::from_stream(vals.into_iter(), None), metadata, )), - Value::Range { val, .. } => match val.into_range_iter(None) { - Ok(iter) => PipelineIterator(PipelineData::ListStream( - ListStream::from_stream(iter, None), - metadata, - )), - Err(error) => PipelineIterator(PipelineData::ListStream( - ListStream::from_stream( - std::iter::once(Value::error(error, span)), - None, - ), - metadata, - )), - }, + Value::Range { val, .. } => PipelineIterator(PipelineData::ListStream( + ListStream::from_stream(val.into_range_iter(span, None), None), + metadata, + )), x => PipelineIterator(PipelineData::Value(x, metadata)), } } diff --git a/crates/nu-protocol/src/value/from_value.rs b/crates/nu-protocol/src/value/from_value.rs index baa77ad7ea..4e72814f86 100644 --- a/crates/nu-protocol/src/value/from_value.rs +++ b/crates/nu-protocol/src/value/from_value.rs @@ -442,7 +442,7 @@ impl FromValue for Spanned> { impl FromValue for Range { fn from_value(v: Value) -> Result { match v { - Value::Range { val, .. } => Ok(*val), + Value::Range { val, .. } => Ok(val), v => Err(ShellError::CantConvert { to_type: "range".into(), from_type: v.get_type().to_string(), @@ -457,7 +457,7 @@ impl FromValue for Spanned { fn from_value(v: Value) -> Result { let span = v.span(); match v { - Value::Range { val, .. } => Ok(Spanned { item: *val, span }), + Value::Range { val, .. } => Ok(Spanned { item: val, span }), v => Err(ShellError::CantConvert { to_type: "range".into(), from_type: v.get_type().to_string(), diff --git a/crates/nu-protocol/src/value/mod.rs b/crates/nu-protocol/src/value/mod.rs index 1136c59d25..d747b88400 100644 --- a/crates/nu-protocol/src/value/mod.rs +++ b/crates/nu-protocol/src/value/mod.rs @@ -14,11 +14,11 @@ pub use filesize::*; pub use from_value::FromValue; pub use glob::*; pub use lazy_record::LazyRecord; -pub use range::*; +pub use range::{FloatRange, IntRange, Range}; pub use record::Record; use crate::{ - ast::{Bits, Boolean, CellPath, Comparison, Math, Operator, PathMember, RangeInclusion}, + ast::{Bits, Boolean, CellPath, Comparison, Math, Operator, PathMember}, did_you_mean, engine::{Closure, EngineState}, BlockId, Config, ShellError, Span, Type, @@ -36,6 +36,7 @@ use std::{ borrow::Cow, cmp::Ordering, fmt::{Debug, Display, Write}, + ops::Bound, path::PathBuf, }; @@ -87,7 +88,7 @@ pub enum Value { internal_span: Span, }, Range { - val: Box, + val: Range, // note: spans are being refactored out of Value // please use .span() instead of matching this span value #[serde(rename = "span")] @@ -197,7 +198,7 @@ impl Clone for Value { internal_span: *internal_span, }, Value::Range { val, internal_span } => Value::Range { - val: val.clone(), + val: *val, internal_span: *internal_span, }, Value::Float { val, internal_span } => Value::float(*val, *internal_span), @@ -345,9 +346,9 @@ impl Value { } /// Returns a reference to the inner [`Range`] value or an error if this `Value` is not a range - pub fn as_range(&self) -> Result<&Range, ShellError> { + pub fn as_range(&self) -> Result { if let Value::Range { val, .. } = self { - Ok(val.as_ref()) + Ok(*val) } else { self.cant_convert_to("range") } @@ -356,7 +357,7 @@ impl Value { /// Unwraps the inner [`Range`] value or returns an error if this `Value` is not a range pub fn into_range(self) -> Result { if let Value::Range { val, .. } = self { - Ok(*val) + Ok(val) } else { self.cant_convert_to("range") } @@ -921,13 +922,7 @@ impl Value { ) } }, - Value::Range { val, .. } => { - format!( - "{}..{}", - val.from.to_expanded_string(", ", config), - val.to.to_expanded_string(", ", config) - ) - } + Value::Range { val, .. } => val.to_string(), Value::String { val, .. } => val.clone(), Value::Glob { val, .. } => val.clone(), Value::List { vals: val, .. } => format!( @@ -1108,7 +1103,9 @@ impl Value { } } Value::Range { val, .. } => { - if let Some(item) = val.into_range_iter(None)?.nth(*count) { + if let Some(item) = + val.into_range_iter(current.span(), None).nth(*count) + { current = item; } else if *optional { return Ok(Value::nothing(*origin_span)); // short-circuit @@ -1856,12 +1853,6 @@ impl Value { f(self)?; // Check for contained values match self { - // Any values that can contain other values need to be handled recursively - Value::Range { ref mut val, .. } => { - val.from.recurse_mut(f)?; - val.to.recurse_mut(f)?; - val.incr.recurse_mut(f) - } Value::Record { ref mut val, .. } => val .iter_mut() .try_for_each(|(_, rec_value)| rec_value.recurse_mut(f)), @@ -1882,6 +1873,7 @@ impl Value { | Value::Filesize { .. } | Value::Duration { .. } | Value::Date { .. } + | Value::Range { .. } | Value::String { .. } | Value::Glob { .. } | Value::Block { .. } @@ -1975,7 +1967,7 @@ impl Value { pub fn range(val: Range, span: Span) -> Value { Value::Range { - val: Box::new(val), + val, internal_span: span, } } @@ -2185,12 +2177,11 @@ impl Value { Value::test_filesize(0), Value::test_duration(0), Value::test_date(DateTime::UNIX_EPOCH.into()), - Value::test_range(Range { - from: Value::test_nothing(), - incr: Value::test_nothing(), - to: Value::test_nothing(), - inclusion: RangeInclusion::Inclusive, - }), + Value::test_range(Range::IntRange(IntRange { + start: 0, + step: 1, + end: Bound::Excluded(0), + })), Value::test_float(0.0), Value::test_string(String::new()), Value::test_record(Record::new()), diff --git a/crates/nu-protocol/src/value/range.rs b/crates/nu-protocol/src/value/range.rs index dd16910d5e..5b1fa32ec7 100644 --- a/crates/nu-protocol/src/value/range.rs +++ b/crates/nu-protocol/src/value/range.rs @@ -1,246 +1,625 @@ +//! A Range is an iterator over integers or floats. + use serde::{Deserialize, Serialize}; use std::{ cmp::Ordering, + fmt::Display, sync::{atomic::AtomicBool, Arc}, }; -/// A Range is an iterator over integers. -use crate::{ - ast::{RangeInclusion, RangeOperator}, - *, -}; +use crate::{ast::RangeInclusion, ShellError, Span, Value}; -#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] -pub struct Range { - pub from: Value, - pub incr: Value, - pub to: Value, - pub inclusion: RangeInclusion, +mod int_range { + use std::{ + cmp::Ordering, + fmt::Display, + ops::Bound, + sync::{atomic::AtomicBool, Arc}, + }; + + use serde::{Deserialize, Serialize}; + + use crate::{ast::RangeInclusion, ShellError, Span, Value}; + + #[derive(Debug, Clone, Copy, Serialize, Deserialize)] + pub struct IntRange { + pub(crate) start: i64, + pub(crate) step: i64, + pub(crate) end: Bound, + } + + impl IntRange { + pub fn new( + start: Value, + next: Value, + end: Value, + inclusion: RangeInclusion, + span: Span, + ) -> Result { + fn to_int(value: Value) -> Result, ShellError> { + match value { + Value::Int { val, .. } => Ok(Some(val)), + Value::Nothing { .. } => Ok(None), + val => Err(ShellError::CantConvert { + to_type: "int".into(), + from_type: val.get_type().to_string(), + span: val.span(), + help: None, + }), + } + } + + let start = to_int(start)?.unwrap_or(0); + + let next_span = next.span(); + let next = to_int(next)?; + if next.is_some_and(|next| next == start) { + return Err(ShellError::CannotCreateRange { span: next_span }); + } + + let end = to_int(end)?; + + let step = match (next, end) { + (Some(next), Some(end)) => { + if (next < start) != (end < start) { + return Err(ShellError::CannotCreateRange { span }); + } + next - start + } + (Some(next), None) => next - start, + (None, Some(end)) => { + if end < start { + -1 + } else { + 1 + } + } + (None, None) => 1, + }; + + let end = if let Some(end) = end { + match inclusion { + RangeInclusion::Inclusive => Bound::Included(end), + RangeInclusion::RightExclusive => Bound::Excluded(end), + } + } else { + Bound::Unbounded + }; + + Ok(Self { start, step, end }) + } + + pub fn start(&self) -> i64 { + self.start + } + + pub fn end(&self) -> Bound { + self.end + } + + pub fn step(&self) -> i64 { + self.step + } + + pub fn is_unbounded(&self) -> bool { + self.end == Bound::Unbounded + } + + pub fn contains(&self, value: i64) -> bool { + if self.step < 0 { + value <= self.start + && match self.end { + Bound::Included(end) => value >= end, + Bound::Excluded(end) => value > end, + Bound::Unbounded => true, + } + } else { + self.start <= value + && match self.end { + Bound::Included(end) => value <= end, + Bound::Excluded(end) => value < end, + Bound::Unbounded => true, + } + } + } + + pub fn into_range_iter(self, ctrlc: Option>) -> Iter { + Iter { + current: Some(self.start), + step: self.step, + end: self.end, + ctrlc, + } + } + } + + impl Ord for IntRange { + fn cmp(&self, other: &Self) -> Ordering { + // Ranges are compared roughly according to their list representation. + // Compare in order: + // - the head element (start) + // - the tail elements (step) + // - the length (end) + self.start + .cmp(&other.start) + .then(self.step.cmp(&other.step)) + .then_with(|| match (self.end, other.end) { + (Bound::Included(l), Bound::Included(r)) + | (Bound::Excluded(l), Bound::Excluded(r)) => { + let ord = l.cmp(&r); + if self.step < 0 { + ord.reverse() + } else { + ord + } + } + (Bound::Included(l), Bound::Excluded(r)) => match l.cmp(&r) { + Ordering::Equal => Ordering::Greater, + ord if self.step < 0 => ord.reverse(), + ord => ord, + }, + (Bound::Excluded(l), Bound::Included(r)) => match l.cmp(&r) { + Ordering::Equal => Ordering::Less, + ord if self.step < 0 => ord.reverse(), + ord => ord, + }, + (Bound::Included(_), Bound::Unbounded) => Ordering::Less, + (Bound::Excluded(_), Bound::Unbounded) => Ordering::Less, + (Bound::Unbounded, Bound::Included(_)) => Ordering::Greater, + (Bound::Unbounded, Bound::Excluded(_)) => Ordering::Greater, + (Bound::Unbounded, Bound::Unbounded) => Ordering::Equal, + }) + } + } + + impl PartialOrd for IntRange { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.cmp(other)) + } + } + + impl PartialEq for IntRange { + fn eq(&self, other: &Self) -> bool { + self.start == other.start && self.step == other.step && self.end == other.end + } + } + + impl Eq for IntRange {} + + impl Display for IntRange { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + // what about self.step? + let start = self.start; + match self.end { + Bound::Included(end) => write!(f, "{start}..{end}"), + Bound::Excluded(end) => write!(f, "{start}..<{end}"), + Bound::Unbounded => write!(f, "{start}.."), + } + } + } + + pub struct Iter { + current: Option, + step: i64, + end: Bound, + ctrlc: Option>, + } + + impl Iterator for Iter { + type Item = i64; + + fn next(&mut self) -> Option { + if let Some(current) = self.current { + let not_end = match (self.step < 0, self.end) { + (true, Bound::Included(end)) => current >= end, + (true, Bound::Excluded(end)) => current > end, + (false, Bound::Included(end)) => current <= end, + (false, Bound::Excluded(end)) => current < end, + (_, Bound::Unbounded) => true, // will stop once integer overflows + }; + + if not_end && !nu_utils::ctrl_c::was_pressed(&self.ctrlc) { + self.current = current.checked_add(self.step); + Some(current) + } else { + self.current = None; + None + } + } else { + None + } + } + } +} + +mod float_range { + use std::{ + cmp::Ordering, + fmt::Display, + ops::Bound, + sync::{atomic::AtomicBool, Arc}, + }; + + use serde::{Deserialize, Serialize}; + + use crate::{ast::RangeInclusion, IntRange, Range, ShellError, Span, Value}; + + #[derive(Debug, Clone, Copy, Serialize, Deserialize)] + pub struct FloatRange { + pub(crate) start: f64, + pub(crate) step: f64, + pub(crate) end: Bound, + } + + impl FloatRange { + pub fn new( + start: Value, + next: Value, + end: Value, + inclusion: RangeInclusion, + span: Span, + ) -> Result { + fn to_float(value: Value) -> Result, ShellError> { + match value { + Value::Float { val, .. } => Ok(Some(val)), + Value::Int { val, .. } => Ok(Some(val as f64)), + Value::Nothing { .. } => Ok(None), + val => Err(ShellError::CantConvert { + to_type: "float".into(), + from_type: val.get_type().to_string(), + span: val.span(), + help: None, + }), + } + } + + // `start` must be finite (not NaN or infinity). + // `next` must be finite and not equal to `start`. + // `end` must not be NaN (but can be infinite). + // + // TODO: better error messages for the restrictions above + + let start_span = start.span(); + let start = to_float(start)?.unwrap_or(0.0); + if !start.is_finite() { + return Err(ShellError::CannotCreateRange { span: start_span }); + } + + let end_span = end.span(); + let end = to_float(end)?; + if end.is_some_and(f64::is_nan) { + return Err(ShellError::CannotCreateRange { span: end_span }); + } + + let next_span = next.span(); + let next = to_float(next)?; + if next.is_some_and(|next| next == start || !next.is_finite()) { + return Err(ShellError::CannotCreateRange { span: next_span }); + } + + let step = match (next, end) { + (Some(next), Some(end)) => { + if (next < start) != (end < start) { + return Err(ShellError::CannotCreateRange { span }); + } + next - start + } + (Some(next), None) => next - start, + (None, Some(end)) => { + if end < start { + -1.0 + } else { + 1.0 + } + } + (None, None) => 1.0, + }; + + let end = if let Some(end) = end { + if end.is_infinite() { + Bound::Unbounded + } else { + match inclusion { + RangeInclusion::Inclusive => Bound::Included(end), + RangeInclusion::RightExclusive => Bound::Excluded(end), + } + } + } else { + Bound::Unbounded + }; + + Ok(Self { start, step, end }) + } + + pub fn start(&self) -> f64 { + self.start + } + + pub fn end(&self) -> Bound { + self.end + } + + pub fn step(&self) -> f64 { + self.step + } + + pub fn is_unbounded(&self) -> bool { + self.end == Bound::Unbounded + } + + pub fn contains(&self, value: f64) -> bool { + if self.step < 0.0 { + value <= self.start + && match self.end { + Bound::Included(end) => value >= end, + Bound::Excluded(end) => value > end, + Bound::Unbounded => true, + } + } else { + self.start <= value + && match self.end { + Bound::Included(end) => value <= end, + Bound::Excluded(end) => value < end, + Bound::Unbounded => true, + } + } + } + + pub fn into_range_iter(self, ctrlc: Option>) -> Iter { + Iter { + start: self.start, + step: self.step, + end: self.end, + iter: Some(0), + ctrlc, + } + } + } + + impl Ord for FloatRange { + fn cmp(&self, other: &Self) -> Ordering { + fn float_cmp(a: f64, b: f64) -> Ordering { + // There is no way a `FloatRange` can have NaN values: + // - `FloatRange::new` ensures no values are NaN. + // - `From for FloatRange` cannot give NaNs either. + // - There are no other ways to create a `FloatRange`. + // - There is no way to modify values of a `FloatRange`. + a.partial_cmp(&b).expect("not NaN") + } + + // Ranges are compared roughly according to their list representation. + // Compare in order: + // - the head element (start) + // - the tail elements (step) + // - the length (end) + float_cmp(self.start, other.start) + .then(float_cmp(self.step, other.step)) + .then_with(|| match (self.end, other.end) { + (Bound::Included(l), Bound::Included(r)) + | (Bound::Excluded(l), Bound::Excluded(r)) => { + let ord = float_cmp(l, r); + if self.step < 0.0 { + ord.reverse() + } else { + ord + } + } + (Bound::Included(l), Bound::Excluded(r)) => match float_cmp(l, r) { + Ordering::Equal => Ordering::Greater, + ord if self.step < 0.0 => ord.reverse(), + ord => ord, + }, + (Bound::Excluded(l), Bound::Included(r)) => match float_cmp(l, r) { + Ordering::Equal => Ordering::Less, + ord if self.step < 0.0 => ord.reverse(), + ord => ord, + }, + (Bound::Included(_), Bound::Unbounded) => Ordering::Less, + (Bound::Excluded(_), Bound::Unbounded) => Ordering::Less, + (Bound::Unbounded, Bound::Included(_)) => Ordering::Greater, + (Bound::Unbounded, Bound::Excluded(_)) => Ordering::Greater, + (Bound::Unbounded, Bound::Unbounded) => Ordering::Equal, + }) + } + } + + impl PartialOrd for FloatRange { + fn partial_cmp(&self, other: &Self) -> Option { + Some(self.cmp(other)) + } + } + + impl PartialEq for FloatRange { + fn eq(&self, other: &Self) -> bool { + self.start == other.start && self.step == other.step && self.end == other.end + } + } + + impl Eq for FloatRange {} + + impl Display for FloatRange { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + // what about self.step? + let start = self.start; + match self.end { + Bound::Included(end) => write!(f, "{start}..{end}"), + Bound::Excluded(end) => write!(f, "{start}..<{end}"), + Bound::Unbounded => write!(f, "{start}.."), + } + } + } + + impl From for FloatRange { + fn from(range: IntRange) -> Self { + Self { + start: range.start as f64, + step: range.step as f64, + end: match range.end { + Bound::Included(b) => Bound::Included(b as f64), + Bound::Excluded(b) => Bound::Excluded(b as f64), + Bound::Unbounded => Bound::Unbounded, + }, + } + } + } + + impl From for FloatRange { + fn from(range: Range) -> Self { + match range { + Range::IntRange(range) => range.into(), + Range::FloatRange(range) => range, + } + } + } + + pub struct Iter { + start: f64, + step: f64, + end: Bound, + iter: Option, + ctrlc: Option>, + } + + impl Iterator for Iter { + type Item = f64; + + fn next(&mut self) -> Option { + if let Some(iter) = self.iter { + let current = self.start + self.step * iter as f64; + + let not_end = match (self.step < 0.0, self.end) { + (true, Bound::Included(end)) => current >= end, + (true, Bound::Excluded(end)) => current > end, + (false, Bound::Included(end)) => current <= end, + (false, Bound::Excluded(end)) => current < end, + (_, Bound::Unbounded) => current.is_finite(), + }; + + if not_end && !nu_utils::ctrl_c::was_pressed(&self.ctrlc) { + self.iter = iter.checked_add(1); + Some(current) + } else { + self.iter = None; + None + } + } else { + None + } + } + } +} + +pub use float_range::FloatRange; +pub use int_range::IntRange; + +#[derive(Debug, Clone, Copy, Serialize, Deserialize)] +pub enum Range { + IntRange(IntRange), + FloatRange(FloatRange), } impl Range { pub fn new( - expr_span: Span, - from: Value, + start: Value, next: Value, - to: Value, - operator: &RangeOperator, - ) -> Result { - // Select from & to values if they're not specified - // TODO: Replace the placeholder values with proper min/max for range based on data type - let from = if let Value::Nothing { .. } = from { - Value::int(0i64, expr_span) + end: Value, + inclusion: RangeInclusion, + span: Span, + ) -> Result { + // promote to float range if any Value is float + if matches!(start, Value::Float { .. }) + || matches!(next, Value::Float { .. }) + || matches!(end, Value::Float { .. }) + { + FloatRange::new(start, next, end, inclusion, span).map(Self::FloatRange) } else { - from - }; + IntRange::new(start, next, end, inclusion, span).map(Self::IntRange) + } + } - let to = if let Value::Nothing { .. } = to { - if let Ok(Value::Bool { val: true, .. }) = next.lt(expr_span, &from, expr_span) { - Value::int(i64::MIN, expr_span) - } else { - Value::int(i64::MAX, expr_span) + pub fn contains(&self, value: &Value) -> bool { + match (self, value) { + (Self::IntRange(range), Value::Int { val, .. }) => range.contains(*val), + (Self::IntRange(range), Value::Float { val, .. }) => { + FloatRange::from(*range).contains(*val) } - } else { - to - }; - - // Check if the range counts up or down - let moves_up = matches!( - from.lte(expr_span, &to, expr_span), - Ok(Value::Bool { val: true, .. }) - ); - - // Convert the next value into the increment - let incr = if let Value::Nothing { .. } = next { - if moves_up { - Value::int(1i64, expr_span) - } else { - Value::int(-1i64, expr_span) - } - } else { - next.sub(operator.next_op_span, &from, expr_span)? - }; - - let zero = Value::int(0i64, expr_span); - - // Increment must be non-zero, otherwise we iterate forever - if matches!( - incr.eq(expr_span, &zero, expr_span), - Ok(Value::Bool { val: true, .. }) - ) { - return Err(ShellError::CannotCreateRange { span: expr_span }); - } - - // If to > from, then incr > 0, otherwise we iterate forever - if let (Value::Bool { val: true, .. }, Value::Bool { val: false, .. }) = ( - to.gt(operator.span, &from, expr_span)?, - incr.gt(operator.next_op_span, &zero, expr_span)?, - ) { - return Err(ShellError::CannotCreateRange { span: expr_span }); - } - - // If to < from, then incr < 0, otherwise we iterate forever - if let (Value::Bool { val: true, .. }, Value::Bool { val: false, .. }) = ( - to.lt(operator.span, &from, expr_span)?, - incr.lt(operator.next_op_span, &zero, expr_span)?, - ) { - return Err(ShellError::CannotCreateRange { span: expr_span }); - } - - Ok(Range { - from, - incr, - to, - inclusion: operator.inclusion, - }) - } - - #[inline] - fn moves_up(&self) -> bool { - self.from <= self.to - } - - pub fn is_end_inclusive(&self) -> bool { - matches!(self.inclusion, RangeInclusion::Inclusive) - } - - pub fn from(&self) -> Result { - self.from.as_int() - } - - pub fn to(&self) -> Result { - let to = self.to.as_int()?; - if self.is_end_inclusive() { - Ok(to) - } else { - Ok(to - 1) + (Self::FloatRange(range), Value::Int { val, .. }) => range.contains(*val as f64), + (Self::FloatRange(range), Value::Float { val, .. }) => range.contains(*val), + _ => false, } } - pub fn contains(&self, item: &Value) -> bool { - match (item.partial_cmp(&self.from), item.partial_cmp(&self.to)) { - (Some(Ordering::Greater | Ordering::Equal), Some(Ordering::Less)) => self.moves_up(), - (Some(Ordering::Less | Ordering::Equal), Some(Ordering::Greater)) => !self.moves_up(), - (Some(_), Some(Ordering::Equal)) => self.is_end_inclusive(), - (_, _) => false, + pub fn into_range_iter(self, span: Span, ctrlc: Option>) -> Iter { + match self { + Range::IntRange(range) => Iter::IntIter(range.into_range_iter(ctrlc), span), + Range::FloatRange(range) => Iter::FloatIter(range.into_range_iter(ctrlc), span), } } +} - pub fn into_range_iter( - self, - ctrlc: Option>, - ) -> Result { - let span = self.from.span(); - - Ok(RangeIterator::new(self, ctrlc, span)) +impl Ord for Range { + fn cmp(&self, other: &Self) -> Ordering { + match (self, other) { + (Range::IntRange(l), Range::IntRange(r)) => l.cmp(r), + (Range::FloatRange(l), Range::FloatRange(r)) => l.cmp(r), + (Range::IntRange(int), Range::FloatRange(float)) => FloatRange::from(*int).cmp(float), + (Range::FloatRange(float), Range::IntRange(int)) => float.cmp(&FloatRange::from(*int)), + } } } impl PartialOrd for Range { fn partial_cmp(&self, other: &Self) -> Option { - match self.from.partial_cmp(&other.from) { - Some(core::cmp::Ordering::Equal) => {} - ord => return ord, - } - match self.incr.partial_cmp(&other.incr) { - Some(core::cmp::Ordering::Equal) => {} - ord => return ord, - } - match self.to.partial_cmp(&other.to) { - Some(core::cmp::Ordering::Equal) => {} - ord => return ord, - } - self.inclusion.partial_cmp(&other.inclusion) + Some(self.cmp(other)) } } -pub struct RangeIterator { - curr: Value, - end: Value, - span: Span, - is_end_inclusive: bool, - moves_up: bool, - incr: Value, - done: bool, - ctrlc: Option>, -} - -impl RangeIterator { - pub fn new(range: Range, ctrlc: Option>, span: Span) -> RangeIterator { - let moves_up = range.moves_up(); - let is_end_inclusive = range.is_end_inclusive(); - - let start = match range.from { - Value::Nothing { .. } => Value::int(0, span), - x => x, - }; - - let end = match range.to { - Value::Nothing { .. } => Value::int(i64::MAX, span), - x => x, - }; - - RangeIterator { - moves_up, - curr: start, - end, - span, - is_end_inclusive, - done: false, - incr: range.incr, - ctrlc, +impl PartialEq for Range { + fn eq(&self, other: &Self) -> bool { + match (self, other) { + (Range::IntRange(l), Range::IntRange(r)) => l == r, + (Range::FloatRange(l), Range::FloatRange(r)) => l == r, + (Range::IntRange(int), Range::FloatRange(float)) => FloatRange::from(*int) == *float, + (Range::FloatRange(float), Range::IntRange(int)) => *float == FloatRange::from(*int), } } } -impl Iterator for RangeIterator { +impl Eq for Range {} + +impl Display for Range { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Range::IntRange(range) => write!(f, "{range}"), + Range::FloatRange(range) => write!(f, "{range}"), + } + } +} + +impl From for Range { + fn from(range: IntRange) -> Self { + Self::IntRange(range) + } +} + +impl From for Range { + fn from(range: FloatRange) -> Self { + Self::FloatRange(range) + } +} + +pub enum Iter { + IntIter(int_range::Iter, Span), + FloatIter(float_range::Iter, Span), +} + +impl Iterator for Iter { type Item = Value; + fn next(&mut self) -> Option { - if self.done { - return None; - } - - if nu_utils::ctrl_c::was_pressed(&self.ctrlc) { - return None; - } - - let ordering = if matches!(self.end, Value::Nothing { .. }) { - Some(Ordering::Less) - } else { - self.curr.partial_cmp(&self.end) - }; - - let Some(ordering) = ordering else { - self.done = true; - return Some(Value::error( - ShellError::CannotCreateRange { span: self.span }, - self.span, - )); - }; - - let desired_ordering = if self.moves_up { - Ordering::Less - } else { - Ordering::Greater - }; - - if (ordering == desired_ordering) || (self.is_end_inclusive && ordering == Ordering::Equal) - { - let next_value = self.curr.add(self.span, &self.incr, self.span); - - let mut next = match next_value { - Ok(result) => result, - - Err(error) => { - self.done = true; - return Some(Value::error(error, self.span)); - } - }; - std::mem::swap(&mut self.curr, &mut next); - - Some(next) - } else { - None + match self { + Iter::IntIter(iter, span) => iter.next().map(|val| Value::int(val, *span)), + Iter::FloatIter(iter, span) => iter.next().map(|val| Value::float(val, *span)), } } }