From 6034de641a15dc6b2ca219199e045e51a3836179 Mon Sep 17 00:00:00 2001 From: George Pollard Date: Thu, 5 Sep 2019 03:30:51 +1200 Subject: [PATCH 1/2] Improve parsing of pipelines, require pipes At the moment the pipeline parser does not enforce that there must be a pipe between each part of the pipeline, which can lead to confusing behaviour or misleading errors. --- src/parser/parse/parser.rs | 18 +++++++++--------- src/parser/parse/pipeline.rs | 2 +- src/parser/parse/token_tree_builder.rs | 13 ++++++------- src/shell/helper.rs | 8 ++++---- 4 files changed, 20 insertions(+), 21 deletions(-) diff --git a/src/parser/parse/parser.rs b/src/parser/parse/parser.rs index 238a144d5a..8e4d995a2d 100644 --- a/src/parser/parse/parser.rs +++ b/src/parser/parse/parser.rs @@ -552,11 +552,11 @@ pub fn node(input: NomSpan) -> IResult { pub fn pipeline(input: NomSpan) -> IResult { trace_step(input, "pipeline", |input| { let start = input.offset; - let (input, head) = opt(tuple((raw_call, opt(space1), opt(tag("|")))))(input)?; + let (input, head) = opt(tuple((raw_call, opt(space1))))(input)?; let (input, items) = trace_step( input, "many0", - many0(tuple((opt(space1), raw_call, opt(space1), opt(tag("|"))))), + many0(tuple((tag("|"), opt(space1), raw_call, opt(space1)))), )?; let (input, tail) = opt(space1)(input)?; @@ -582,28 +582,28 @@ pub fn pipeline(input: NomSpan) -> IResult { } fn make_call_list( - head: Option<(Tagged, Option, Option)>, + head: Option<(Tagged, Option)>, items: Vec<( + NomSpan, Option, Tagged, Option, - Option, )>, ) -> Vec { let mut out = vec![]; if let Some(head) = head { - let el = PipelineElement::new(None, head.0, head.1.map(Span::from), head.2.map(Span::from)); + let el = PipelineElement::new(None, None, head.0, head.1.map(Span::from)); out.push(el); } - for (ws1, call, ws2, pipe) in items { + for (pipe, ws1, call, ws2) in items { let el = PipelineElement::new( + Some(pipe).map(Span::from), ws1.map(Span::from), call, - ws2.map(Span::from), - pipe.map(Span::from), - ); + ws2.map(Span::from)); + out.push(el); } diff --git a/src/parser/parse/pipeline.rs b/src/parser/parse/pipeline.rs index 20365bfbc7..75155d143a 100644 --- a/src/parser/parse/pipeline.rs +++ b/src/parser/parse/pipeline.rs @@ -11,9 +11,9 @@ pub struct Pipeline { #[derive(Debug, Clone, Eq, PartialEq, Ord, PartialOrd, Getters, new)] pub struct PipelineElement { + pub pipe: Option, pub pre_ws: Option, #[get = "pub(crate)"] call: Tagged, pub post_ws: Option, - pub post_pipe: Option, } diff --git a/src/parser/parse/token_tree_builder.rs b/src/parser/parse/token_tree_builder.rs index ad4eebdc8a..9dd1ebc16c 100644 --- a/src/parser/parse/token_tree_builder.rs +++ b/src/parser/parse/token_tree_builder.rs @@ -47,32 +47,31 @@ impl TokenTreeBuilder { .next() .expect("A pipeline must contain at least one element"); + let pipe = None; let pre_span = pre.map(|pre| b.consume(&pre)); let call = call(b); let post_span = post.map(|post| b.consume(&post)); - let pipe = input.peek().map(|_| Span::from(b.consume("|"))); + out.push(PipelineElement::new( + pipe, pre_span.map(Span::from), call, - post_span.map(Span::from), - pipe, - )); + post_span.map(Span::from))); loop { match input.next() { None => break, Some((pre, call, post)) => { + let pipe = Some(Span::from(b.consume("|"))); let pre_span = pre.map(|pre| b.consume(&pre)); let call = call(b); let post_span = post.map(|post| b.consume(&post)); - let pipe = input.peek().map(|_| Span::from(b.consume("|"))); - out.push(PipelineElement::new( + pipe, pre_span.map(Span::from), call, post_span.map(Span::from), - pipe, )); } } diff --git a/src/shell/helper.rs b/src/shell/helper.rs index 9feffcb4ce..8e21c50ec8 100644 --- a/src/shell/helper.rs +++ b/src/shell/helper.rs @@ -147,6 +147,10 @@ fn paint_token_node(token_node: &TokenNode, line: &str) -> String { fn paint_pipeline_element(pipeline_element: &PipelineElement, line: &str) -> String { let mut styled = String::new(); + if let Some(_) = pipeline_element.pipe { + styled.push_str(&Color::Purple.paint("|")); + } + if let Some(ws) = pipeline_element.pre_ws { styled.push_str(&Color::White.normal().paint(ws.slice(line))); } @@ -168,10 +172,6 @@ fn paint_pipeline_element(pipeline_element: &PipelineElement, line: &str) -> Str styled.push_str(&Color::White.normal().paint(ws.slice(line))); } - if let Some(_) = pipeline_element.post_pipe { - styled.push_str(&Color::Purple.paint("|")); - } - styled.to_string() } From 60212611e504178dd5ca8eee34f60c89166902f3 Mon Sep 17 00:00:00 2001 From: George Pollard Date: Thu, 5 Sep 2019 04:13:07 +1200 Subject: [PATCH 2/2] Allow leading space before head of pipeline --- src/parser/parse/parser.rs | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/parser/parse/parser.rs b/src/parser/parse/parser.rs index 8e4d995a2d..f230c36c22 100644 --- a/src/parser/parse/parser.rs +++ b/src/parser/parse/parser.rs @@ -552,7 +552,7 @@ pub fn node(input: NomSpan) -> IResult { pub fn pipeline(input: NomSpan) -> IResult { trace_step(input, "pipeline", |input| { let start = input.offset; - let (input, head) = opt(tuple((raw_call, opt(space1))))(input)?; + let (input, head) = opt(tuple((opt(space1), raw_call, opt(space1))))(input)?; let (input, items) = trace_step( input, "many0", @@ -582,7 +582,11 @@ pub fn pipeline(input: NomSpan) -> IResult { } fn make_call_list( - head: Option<(Tagged, Option)>, + head: Option<( + Option, + Tagged, + Option + )>, items: Vec<( NomSpan, Option, @@ -593,7 +597,12 @@ fn make_call_list( let mut out = vec![]; if let Some(head) = head { - let el = PipelineElement::new(None, None, head.0, head.1.map(Span::from)); + let el = PipelineElement::new( + None, + head.0.map(Span::from), + head.1, + head.2.map(Span::from)); + out.push(el); }