holm/nushell
1
1
Fork 0
forked from extern/nushell
Code Issues Pull Requests Packages Projects Releases Wiki Activity

range operator accepts bot..=top as well as bot..top (#8382)

Browse Source 
# Description

A compromise fix for #8162. Nushell range operator now accepts `..=` to
mean the range includes the top value, so you can use your Rust habits.
But the unadorned `..` range operator also includes the value, so you
can also use your Nushell habits.

_(Description of your pull request goes here. **Provide examples and/or
screenshots** if your changes affect the user experience.)_

```nushell
〉1..5
╭───┬───╮
│ 0 │ 1 │
│ 1 │ 2 │
│ 2 │ 3 │
│ 3 │ 4 │
│ 4 │ 5 │
╰───┴───╯
-------------------------------------------- /home/bobhy/src/rust/nushell --------------------------------------------
〉1..=5
╭───┬───╮
│ 0 │ 1 │
│ 1 │ 2 │
│ 2 │ 3 │
│ 3 │ 4 │
│ 4 │ 5 │
╰───┴───╯
-------------------------------------------- /home/bobhy/src/rust/nushell --------------------------------------------
〉1..<5
╭───┬───╮
│ 0 │ 1 │
│ 1 │ 2 │
│ 2 │ 3 │
│ 3 │ 4 │
╰───┴───╯
```
# User-Facing Changes

Existing scripts with range operator will continue to operate as
heretofore.

_(List of all changes that impact the user experience here. This helps
us keep track of breaking changes.)_

# Tests + Formatting

Don't forget to add tests that cover your changes.

Make sure you've run and fixed any issues with these commands:

- [x] `cargo fmt --all -- --check` to check standard code formatting
(`cargo fmt --all` applies these changes)
- [x] `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used -A
clippy::needless_collect` to check that you're using the standard code
style
- [x] `cargo test --workspace` to check that all tests pass

# After Submitting

Will update the book to include new syntax.
This commit is contained in:
Bob Hyman 2023-04-07 07:40:05 -04:00 committed by GitHub
parent aded2c1937
commit 771e24913d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 229 additions and 262 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

33
Cargo.lock generated
View File

@ -2708,7 +2708,7 @@ dependencies = [
"pretty_assertions",
"rayon",
"reedline",
"rstest",
"rstest 0.17.0",
"serde_json",
"serial_test",
"signal-hook",
@ -2751,7 +2751,7 @@ dependencies = [
"once_cell",
"percent-encoding",
"reedline",
"rstest",
"rstest 0.17.0",
"sysinfo 0.28.2",
"thiserror",
]
@ -2863,7 +2863,7 @@ dependencies = [
"reedline",
"regex",
"roxmltree",
"rstest",
"rstest 0.17.0",
"rusqlite",
"rust-embed",
"same-file",
@ -2954,6 +2954,7 @@ dependencies = [
"nu-path",
"nu-plugin",
"nu-protocol",
"rstest 0.16.0",
"serde_json",
"thiserror",
]
@ -4348,16 +4349,40 @@ dependencies = [
"xmlparser",
]
[[package]]
name = "rstest"
version = "0.16.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "b07f2d176c472198ec1e6551dc7da28f1c089652f66a7b722676c2238ebc0edf"
dependencies = [
"rstest_macros 0.16.0",
"rustc_version",
]
[[package]]
name = "rstest"
version = "0.17.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "de1bb486a691878cd320c2f0d319ba91eeaa2e894066d8b5f8f117c000e9d962"
dependencies = [
"rstest_macros",
"rstest_macros 0.17.0",
"rustc_version",
]
[[package]]
name = "rstest_macros"
version = "0.16.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
checksum = "7229b505ae0706e64f37ffc54a9c163e11022a6636d58fe1f3f52018257ff9f7"
dependencies = [
"cfg-if 1.0.0",
"proc-macro2",
"quote",
"rustc_version",
"syn 1.0.109",
"unicode-ident",
]
[[package]]
name = "rstest_macros"
version = "0.17.0"

3
crates/nu-parser/Cargo.toml
View File

@ -23,5 +23,8 @@ nu-plugin = { path = "../nu-plugin", optional = true, version = "0.78.1" }
nu-engine = { path = "../nu-engine", version = "0.78.1" }
log = "0.4"
[dev-dependencies]
rstest = { version = "0.16.0", default-features = false }
[features]
plugin = ["nu-plugin"]

13
crates/nu-parser/src/parser.rs
View File

@ -1500,7 +1500,7 @@ pub fn parse_range(
// Range follows the following syntax: [<from>][<next_operator><next>]<range_operator>[<to>]
// where <next_operator> is ".."
// and <range_operator> is ".." or "..<"
// and <range_operator> is "..", "..=" or "..<"
// and one of the <from> or <to> bounds must be present (just '..' is not allowed since it
// looks like parent directory)
//bugbug range cannot be [..] because that looks like parent directory
@ -1553,12 +1553,12 @@ pub fn parse_range(
return garbage(span);
}
} else {
let op_str = "..";
let op_str = if token.contains("..=") { "..=" } else { ".." };
let op_span = Span::new(
span.start + range_op_pos,
span.start + range_op_pos + op_str.len(),
);
(RangeInclusion::Inclusive, "..", op_span)
(RangeInclusion::Inclusive, op_str, op_span)
};
// Now, based on the operator positions, figure out where the bounds & next are located and
@ -5178,7 +5178,12 @@ pub fn parse_expression(
let split = name.splitn(2, |x| *x == b'=');
let split: Vec<_> = split.collect();
if !name.starts_with(b"^") && split.len() == 2 && !split[0].is_empty() {
if !name.starts_with(b"^")
&& split.len() == 2
&& !split[0].is_empty()
&& !split[0].ends_with(b"..")
// was range op ..=
{
let point = split[0].len() + 1;
let starting_error_count = working_set.parse_errors.len();

442
crates/nu-parser/tests/test_parser.rs
View File

@ -6,6 +6,7 @@ use nu_protocol::{
engine::{Command, EngineState, Stack, StateWorkingSet},
ParseError, PipelineData, ShellError, Signature, SyntaxShape,
};
use rstest::rstest;
#[cfg(test)]
#[derive(Clone)]
@ -974,334 +975,267 @@ mod range {
use super::*;
use nu_protocol::ast::{RangeInclusion, RangeOperator};
#[test]
fn parse_inclusive_range() {
#[rstest]
#[case(b"0..10", RangeInclusion::Inclusive, "inclusive")]
#[case(b"0..=10", RangeInclusion::Inclusive, "=inclusive")]
#[case(b"0..<10", RangeInclusion::RightExclusive, "exclusive")]
#[case(b"10..0", RangeInclusion::Inclusive, "reverse inclusive")]
#[case(b"10..=0", RangeInclusion::Inclusive, "reverse =inclusive")]
#[case(
b"(3 - 3)..<(8 + 2)",
RangeInclusion::RightExclusive,
"subexpression exclusive"
)]
#[case(
b"(3 - 3)..(8 + 2)",
RangeInclusion::Inclusive,
"subexpression inclusive"
)]
#[case(
b"(3 - 3)..=(8 + 2)",
RangeInclusion::Inclusive,
"subexpression =inclusive"
)]
#[case(b"-10..-3", RangeInclusion::Inclusive, "negative inclusive")]
#[case(b"-10..=-3", RangeInclusion::Inclusive, "negative =inclusive")]
#[case(b"-10..<-3", RangeInclusion::RightExclusive, "negative exclusive")]
fn parse_bounded_range(
#[case] phrase: &[u8],
#[case] inclusion: RangeInclusion,
#[case] tag: &str,
) {
let engine_state = EngineState::new();
let mut working_set = StateWorkingSet::new(&engine_state);
let block = parse(&mut working_set, None, b"0..10", true, &[]);
let block = parse(&mut working_set, None, phrase, true, &[]);
assert!(working_set.parse_errors.is_empty());
assert_eq!(block.len(), 1);
assert_eq!(block.len(), 1, "{tag}: block length");
let expressions = &block[0];
assert_eq!(expressions.len(), 1);
assert!(matches!(
expressions[0],
PipelineElement::Expression(
_,
Expression {
expr: Expr::Range(
assert_eq!(expressions.len(), 1, "{tag}: expression length");
if let PipelineElement::Expression(
_,
Expression {
expr:
Expr::Range(
Some(_),
None,
Some(_),
RangeOperator {
inclusion: RangeInclusion::Inclusive,
inclusion: the_inclusion,
..
}
},
),
..
}
)
))
..
},
) = expressions[0]
{
assert_eq!(
the_inclusion, inclusion,
"{tag}: wrong RangeInclusion {the_inclusion:?}"
);
} else {
panic!("{tag}: expression mismatch.")
};
}
#[test]
fn parse_exclusive_range() {
let engine_state = EngineState::new();
let mut working_set = StateWorkingSet::new(&engine_state);
let block = parse(&mut working_set, None, b"0..<10", true, &[]);
assert!(working_set.parse_errors.is_empty());
assert_eq!(block.len(), 1);
let expressions = &block[0];
assert_eq!(expressions.len(), 1);
assert!(matches!(
expressions[0],
PipelineElement::Expression(
_,
Expression {
expr: Expr::Range(
Some(_),
None,
Some(_),
RangeOperator {
inclusion: RangeInclusion::RightExclusive,
..
}
),
..
}
)
))
}
#[test]
fn parse_reverse_range() {
let engine_state = EngineState::new();
let mut working_set = StateWorkingSet::new(&engine_state);
let block = parse(&mut working_set, None, b"10..0", true, &[]);
assert!(working_set.parse_errors.is_empty());
assert_eq!(block.len(), 1);
let expressions = &block[0];
assert_eq!(expressions.len(), 1);
assert!(matches!(
expressions[0],
PipelineElement::Expression(
_,
Expression {
expr: Expr::Range(
Some(_),
None,
Some(_),
RangeOperator {
inclusion: RangeInclusion::Inclusive,
..
}
),
..
}
)
))
}
#[test]
fn parse_subexpression_range() {
let engine_state = EngineState::new();
let mut working_set = StateWorkingSet::new(&engine_state);
let block = parse(&mut working_set, None, b"(3 - 3)..<(8 + 2)", true, &[]);
assert!(working_set.parse_errors.is_empty());
assert_eq!(block.len(), 1);
let expressions = &block[0];
assert_eq!(expressions.len(), 1);
assert!(matches!(
expressions[0],
PipelineElement::Expression(
_,
Expression {
expr: Expr::Range(
Some(_),
None,
Some(_),
RangeOperator {
inclusion: RangeInclusion::RightExclusive,
..
}
),
..
}
)
))
}
#[test]
fn parse_variable_range() {
#[rstest]
#[case(
b"let a = 2; $a..10",
RangeInclusion::Inclusive,
"variable start inclusive"
)]
#[case(
b"let a = 2; $a..=10",
RangeInclusion::Inclusive,
"variable start =inclusive"
)]
#[case(
b"let a = 2; $a..<($a + 10)",
RangeInclusion::RightExclusive,
"subexpression variable exclusive"
)]
fn parse_variable_range(
#[case] phrase: &[u8],
#[case] inclusion: RangeInclusion,
#[case] tag: &str,
) {
let engine_state = EngineState::new();
let mut working_set = StateWorkingSet::new(&engine_state);
working_set.add_decl(Box::new(Let));
let block = parse(&mut working_set, None, b"let a = 2; $a..10", true, &[]);
let block = parse(&mut working_set, None, phrase, true, &[]);
assert!(working_set.parse_errors.is_empty());
assert_eq!(block.len(), 2);
assert_eq!(block.len(), 2, "{tag} block len 2");
let expressions = &block[1];
assert_eq!(expressions.len(), 1);
assert!(matches!(
expressions[0],
PipelineElement::Expression(
_,
Expression {
expr: Expr::Range(
assert_eq!(expressions.len(), 1, "{tag}: expression length 1");
if let PipelineElement::Expression(
_,
Expression {
expr:
Expr::Range(
Some(_),
None,
Some(_),
RangeOperator {
inclusion: RangeInclusion::Inclusive,
inclusion: the_inclusion,
..
}
},
),
..
}
)
))
..
},
) = expressions[0]
{
assert_eq!(
the_inclusion, inclusion,
"{tag}: wrong RangeInclusion {the_inclusion:?}"
);
} else {
panic!("{tag}: expression mismatch.")
};
}
#[test]
fn parse_subexpression_variable_range() {
#[rstest]
#[case(b"0..", RangeInclusion::Inclusive, "right unbounded")]
#[case(b"0..=", RangeInclusion::Inclusive, "right unbounded =inclusive")]
#[case(b"0..<", RangeInclusion::RightExclusive, "right unbounded")]
fn parse_right_unbounded_range(
#[case] phrase: &[u8],
#[case] inclusion: RangeInclusion,
#[case] tag: &str,
) {
let engine_state = EngineState::new();
let mut working_set = StateWorkingSet::new(&engine_state);
working_set.add_decl(Box::new(Let));
let block = parse(
&mut working_set,
None,
b"let a = 2; $a..<($a + 10)",
true,
&[],
);
let block = parse(&mut working_set, None, phrase, true, &[]);
assert!(working_set.parse_errors.is_empty());
assert_eq!(block.len(), 2);
let expressions = &block[1];
assert_eq!(expressions.len(), 1);
assert!(matches!(
expressions[0],
PipelineElement::Expression(
_,
Expression {
expr: Expr::Range(
Some(_),
None,
Some(_),
RangeOperator {
inclusion: RangeInclusion::RightExclusive,
..
}
),
..
}
)
))
}
#[test]
fn parse_right_unbounded_range() {
let engine_state = EngineState::new();
let mut working_set = StateWorkingSet::new(&engine_state);
let block = parse(&mut working_set, None, b"0..", true, &[]);
assert!(working_set.parse_errors.is_empty());
assert_eq!(block.len(), 1);
assert_eq!(block.len(), 1, "{tag}: block len 1");
let expressions = &block[0];
assert_eq!(expressions.len(), 1);
assert!(matches!(
expressions[0],
PipelineElement::Expression(
_,
Expression {
expr: Expr::Range(
assert_eq!(expressions.len(), 1, "{tag}: expression length 1");
if let PipelineElement::Expression(
_,
Expression {
expr:
Expr::Range(
Some(_),
None,
None,
RangeOperator {
inclusion: RangeInclusion::Inclusive,
inclusion: the_inclusion,
..
}
},
),
..
}
)
))
..
},
) = expressions[0]
{
assert_eq!(
the_inclusion, inclusion,
"{tag}: wrong RangeInclusion {the_inclusion:?}"
);
} else {
panic!("{tag}: expression mismatch.")
};
}
#[test]
fn parse_left_unbounded_range() {
#[rstest]
#[case(b"..10", RangeInclusion::Inclusive, "left unbounded inclusive")]
#[case(b"..=10", RangeInclusion::Inclusive, "left unbounded =inclusive")]
#[case(b"..<10", RangeInclusion::RightExclusive, "left unbounded exclusive")]
fn parse_left_unbounded_range(
#[case] phrase: &[u8],
#[case] inclusion: RangeInclusion,
#[case] tag: &str,
) {
let engine_state = EngineState::new();
let mut working_set = StateWorkingSet::new(&engine_state);
let block = parse(&mut working_set, None, b"..10", true, &[]);
let block = parse(&mut working_set, None, phrase, true, &[]);
assert!(working_set.parse_errors.is_empty());
assert_eq!(block.len(), 1);
assert_eq!(block.len(), 1, "{tag}: block len 1");
let expressions = &block[0];
assert_eq!(expressions.len(), 1);
assert!(matches!(
expressions[0],
PipelineElement::Expression(
_,
Expression {
expr: Expr::Range(
assert_eq!(expressions.len(), 1, "{tag}: expression length 1");
if let PipelineElement::Expression(
_,
Expression {
expr:
Expr::Range(
None,
None,
Some(_),
RangeOperator {
inclusion: RangeInclusion::Inclusive,
inclusion: the_inclusion,
..
}
},
),
..
}
)
))
..
},
) = expressions[0]
{
assert_eq!(
the_inclusion, inclusion,
"{tag}: wrong RangeInclusion {the_inclusion:?}"
);
} else {
panic!("{tag}: expression mismatch.")
};
}
#[test]
fn parse_negative_range() {
#[rstest]
#[case(b"2.0..4.0..10.0", RangeInclusion::Inclusive, "float inclusive")]
#[case(b"2.0..4.0..=10.0", RangeInclusion::Inclusive, "float =inclusive")]
#[case(b"2.0..4.0..<10.0", RangeInclusion::RightExclusive, "float exclusive")]
fn parse_float_range(
#[case] phrase: &[u8],
#[case] inclusion: RangeInclusion,
#[case] tag: &str,
) {
let engine_state = EngineState::new();
let mut working_set = StateWorkingSet::new(&engine_state);
let block = parse(&mut working_set, None, b"-10..-3", true, &[]);
let block = parse(&mut working_set, None, phrase, true, &[]);
assert!(working_set.parse_errors.is_empty());
assert_eq!(block.len(), 1);
assert_eq!(block.len(), 1, "{tag}: block length 1");
let expressions = &block[0];
assert_eq!(expressions.len(), 1);
assert!(matches!(
expressions[0],
PipelineElement::Expression(
_,
Expression {
expr: Expr::Range(
Some(_),
None,
Some(_),
RangeOperator {
inclusion: RangeInclusion::Inclusive,
..
}
),
..
}
)
))
}
#[test]
fn parse_float_range() {
let engine_state = EngineState::new();
let mut working_set = StateWorkingSet::new(&engine_state);
let block = parse(&mut working_set, None, b"2.0..4.0..10.0", true, &[]);
assert!(working_set.parse_errors.is_empty());
assert_eq!(block.len(), 1);
let expressions = &block[0];
assert_eq!(expressions.len(), 1);
assert!(matches!(
expressions[0],
PipelineElement::Expression(
_,
Expression {
expr: Expr::Range(
assert_eq!(expressions.len(), 1, "{tag}: expression length 1");
if let PipelineElement::Expression(
_,
Expression {
expr:
Expr::Range(
Some(_),
Some(_),
Some(_),
RangeOperator {
inclusion: RangeInclusion::Inclusive,
inclusion: the_inclusion,
..
}
},
),
..
}
)
))
..
},
) = expressions[0]
{
assert_eq!(
the_inclusion, inclusion,
"{tag}: wrong RangeInclusion {the_inclusion:?}"
);
} else {
panic!("{tag}: expression mismatch.")
};
}
#[test]
@ -1309,7 +1243,7 @@ mod range {
let engine_state = EngineState::new();
let mut working_set = StateWorkingSet::new(&engine_state);
parse(&mut working_set, None, b"(0)..\"a\"", true, &[]);
let _ = parse(&mut working_set, None, b"(0)..\"a\"", true, &[]);
assert!(!working_set.parse_errors.is_empty());
}