Repository: Marwes/combine (language: rust)

Issue:
Question: Look ahead and PartialStream
Hello

I am trying to parse a Http-Header like structure (the language server protocol header  :) )

There are the examples regarding exactly that problem in the exampes folder, but I have problems trying to make a working parser that can parse the headers in any order where one header is optional. (I would really like to know a general solution for arbitrary many headers.) 


I have one parser for each of the two header types. Then I tried combining them with combinations of skip_many, optional, choice, arbitrary,... 
Sometimes they parse all my tests correctly, sometimes they give the correct errors, but most of the time I cannot get the error, but instead get Ok(None) indicating that more input is needed.

Now I have a suspicion that you can maybe confirm: is it generally impossible to make arbitrary lookup + choice/many when using a partial stream?

I am on mobile now and just hat this thought. I'll try to create a minimal example tomorrow. 


Discussion / hints:
I think you are looking fot the [attempt](https://docs.rs/combine/3.6.3/combine/fn.attempt.html) parser which lets you do arbitrary lookahead :).
Hey

I now know a solution to my problem, but I don't understand it.

I made a reduced version of the problem: https://github.com/Phaiax/parser-case1/blob/master/main.rs
Run with `cargo test -- --nocapture --test-threads 1`
The part enclosed with `<<<<<<<<<<` and `>>>>>>>>>>>` marks two cases which have different behaviour.

They differ in the use of `skip_count_min_max` vs `skip_many1`. (I initially used the former because at first I had lifetime problems with the latter. Turned out I needed to map-drop output even in intermediate steps because otherwise the PartialState would not be 'static ).

The relevant testcase has the following input which is delivered to the parser in two portions.
```
foobar12\r\nfoobaz\r\n\r\nabcdefg\r\n
      >< here inbetween is the split
```
The first call the parser gets to see only the left part of the splitted input. 

The digits are parsed with skip_many1(digit()).

The parser that uses skip_count_min_max fails to parse correctly.

I would consider this a bug, gut feeling.

Thanks for your time and for the whole project.
There is a small difference in behaviour with `skip_many1` expecting at least one element and the `skip_count...` being ok with no elements. Can't say more at a glance but I can look into it. If you are able to reduce the test case even further that would be really appreciated!

> (I initially used the former because at first I had lifetime problems with the latter. Turned out I needed to map-drop output even in intermediate steps because otherwise the PartialState would not be 'static ).

This seems like a bug maybe. I'd expect them to both have the same issue or not have it. May need to look into why :/
https://github.com/Phaiax/parser-case1/blob/minimal_example/main.rs

This is the minimum I could bring it down to :D

Fix patch:
diff --git a/src/lib.rs b/src/lib.rs
index 88bc6c71..3117f1ed 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -973,7 +973,7 @@ mod std_tests {
         Times(Box<Expr>, Box<Expr>),
     }
 
-    parser!{
+    parser! {
         fn expr[I]()(I) -> Expr
         where
             [I: Stream<Item = char>,]
diff --git a/src/parser/combinator.rs b/src/parser/combinator.rs
index da18f6c2..ab88e72e 100644
--- a/src/parser/combinator.rs
+++ b/src/parser/combinator.rs
@@ -16,7 +16,7 @@ use either::Either;
 
 use error::FastResult::*;
 
-parser!{
+parser! {
 #[derive(Copy, Clone)]
 pub struct NotFollowedBy;
 /// Succeeds only if `parser` fails.
@@ -409,7 +409,7 @@ where
                             return EmptyErr(
                                 <P::Input as StreamOnce>::Error::from_error(input.position(), err)
                                     .into(),
-                            )
+                            );
                         }
                     }
                 }
@@ -426,7 +426,7 @@ where
                             return ConsumedErr(<P::Input as StreamOnce>::Error::from_error(
                                 input.position(),
                                 err,
-                            ))
+                            ));
                         }
                     }
                 }
@@ -443,7 +443,7 @@ where
                             return ConsumedErr(<P::Input as StreamOnce>::Error::from_error(
                                 input.position(),
                                 err,
-                            ))
+                            ));
                         }
                     }
                 }
@@ -1059,7 +1059,7 @@ impl StrLike for [u8] {
     }
 }
 
-parser!{
+parser! {
 pub struct FromStr;
 type PartialState = P::PartialState;
 
diff --git a/src/parser/range.rs b/src/parser/range.rs
index 40466442..303ffeed 100644
--- a/src/parser/range.rs
+++ b/src/parser/range.rs
@@ -50,9 +50,10 @@ where
     }
 }
 
-parser!{
+parser! {
     #[derive(Clone)]
     pub struct Recognize;
+    type PartialState = <RecognizeWithValue<P> as Parser>::PartialState;
     /// Zero-copy parser which returns consumed input range.
     ///
     /// [`combinator::recognize`][] is a non-`RangeStream` alternative.
diff --git a/src/parser/repeat.rs b/src/parser/repeat.rs
index d985546d..7636e1a5 100644
--- a/src/parser/repeat.rs
+++ b/src/parser/repeat.rs
@@ -4,7 +4,7 @@ use lib::borrow::BorrowMut;
 use lib::marker::PhantomData;
 use lib::mem;
 
-use combinator::{ignore, optional, parser, value, FnParser, Ignore, Optional};
+use combinator::{ignore, optional, parser, value, FnParser, Ignore, Optional, Value};
 use error::{Consumed, ConsumedResult, ParseError, ParseResult, StreamError, Tracked};
 use parser::choice::Or;
 use parser::sequence::With;
@@ -44,9 +44,10 @@ where [
 
 }
 
-parser!{
+parser! {
     #[derive(Copy, Clone)]
     pub struct SkipCount;
+    type PartialState = <With<Count<Sink, P>, Value<P::Input, ()>> as Parser>::PartialState;
     /// Parses `parser` from zero up to `count` times skipping the output of `parser`.
     ///
     /// ```
@@ -65,7 +66,7 @@ parser!{
         P: Parser
     ]
     {
-        ::combinator::count::<Sink<()>, _>(*count, parser.map(|_| ())).with(value(()))
+        ::combinator::count::<Sink, _>(*count, parser.map(|_| ())).with(value(()))
     }
 }
 
@@ -159,9 +160,10 @@ where
     }
 }
 
-parser!{
+parser! {
     #[derive(Copy, Clone)]
     pub struct SkipCountMinMax;
+    type PartialState = <With<CountMinMax<Sink, P>, Value<P::Input, ()>> as Parser>::PartialState;
     /// Parses `parser` from `min` to `max` times (including `min` and `max`)
     /// skipping the output of `parser`.
     ///
@@ -186,7 +188,7 @@ parser!{
         P: Parser
     ]
     {
-        ::combinator::count_min_max::<Sink<()>, _>(*min, *max, parser.map(|_| ())).with(value(()))
+        ::combinator::count_min_max::<Sink, _>(*min, *max, parser.map(|_| ())).with(value(()))
     }
 }
 
@@ -488,15 +490,15 @@ where
 #[derive(Clone)]
 #[doc(hidden)]
 // FIXME Should not be public
-pub struct Sink<T>(PhantomData<T>);
+pub struct Sink;
 
-impl<T> Default for Sink<T> {
+impl Default for Sink {
     fn default() -> Self {
-        Sink(PhantomData)
+        Sink
     }
 }
 
-impl<A> Extend<A> for Sink<A> {
+impl<A> Extend<A> for Sink {
     fn extend<T>(&mut self, iter: T)
     where
         T: IntoIterator<Item = A>,
@@ -505,7 +507,7 @@ impl<A> Extend<A> for Sink<A> {
     }
 }
 
-impl_parser!{ SkipMany(P,), Ignore<Many<Sink<()>, Ignore<P>>> }
+impl_parser! { SkipMany(P,), Ignore<Many<Sink, Ignore<P>>> }
 
 /// Parses `p` zero or more times ignoring the result.
 ///
@@ -530,7 +532,7 @@ where
     SkipMany(ignore(many(ignore(p))))
 }
 
-impl_parser!{ SkipMany1(P,), Ignore<Many1<Sink<()>, Ignore<P>>> }
+impl_parser! { SkipMany1(P,), Ignore<Many1<Sink, Ignore<P>>> }
 
 /// Parses `p` one or more times ignoring the result.
 ///
@@ -1175,9 +1177,10 @@ where
     }
 }
 
-parser!{
+parser! {
     #[derive(Copy, Clone)]
     pub struct SkipUntil;
+    type PartialState = <With<TakeUntil<Sink, P>, Value<P::Input, ()>> as Parser>::PartialState;
     /// Skips input until `end` is encountered or `end` indicates that it has consumed input before
     /// failing (`attempt` can be used to make it look like it has not consumed any input)
     ///
@@ -1206,7 +1209,7 @@ parser!{
         P: Parser,
     ]
     {
-        take_until::<Sink<_>, _>(end).with(value(()))
+        take_until::<Sink, _>(end).with(value(()))
     }
 }
 


Test patch:
diff --git a/tests/async.rs b/tests/async.rs
index 989acf95..6daf4f44 100644
--- a/tests/async.rs
+++ b/tests/async.rs
@@ -143,7 +143,7 @@ where
         .wait()
 }
 
-parser!{
+parser! {
     type PartialState = AnyPartialState;
     fn basic_parser['a, I]()(I) -> String
         where [ I: RangeStream<Item = char, Range = &'a str> ]
@@ -154,7 +154,7 @@ parser!{
     }
 }
 
-impl_decoder!{ Basic, String, basic_parser() }
+impl_decoder! { Basic, String, basic_parser() }
 
 #[test]
 fn many1_skip_no_errors() {
@@ -167,7 +167,7 @@ fn many1_skip_no_errors() {
     assert_eq!(result.unwrap(), vec!["123".to_string(), "456".to_string()]);
 }
 
-parser!{
+parser! {
     type PartialState = AnyPartialState;
     fn prefix_many_then_parser['a, I]()(I) -> String
         where [ I: RangeStream<Item = char, Range = &'a str> ]
@@ -182,7 +182,7 @@ parser!{
     }
 }
 
-parser!{
+parser! {
     type PartialState = AnyPartialState;
     fn choice_parser['a, I]()(I) -> String
         where [ I: RangeStream<Item = char, Range = &'a str> ]
@@ -459,10 +459,11 @@ quickcheck! {
         assert_eq!(result.unwrap(), sizes);
     }
 }
+
 #[test]
 fn inner_no_partial_test() {
     let seq = vec![PartialOp::Limited(10)];
-    impl_decoder!{ TestParser, String,
+    impl_decoder! { TestParser, String,
         no_partial(many1(digit()))
             .or(many1(letter()))
             .skip(range(&"\r\n"[..]))
@@ -479,3 +480,18 @@ fn inner_no_partial_test() {
     assert!(result.as_ref().is_ok(), "{}", result.unwrap_err());
     assert_eq!(result.unwrap(), ["1", "abcd", "123", "abc", "1232751"]);
 }
+
+#[test]
+fn skip_count_min_max_test() {
+    let seq = vec![PartialOp::Limited(1)];
+    impl_decoder! { TestParser, String,
+        repeat::skip_count_min_max(1, 2, char('_')).skip(char('.')).map(|_| "".to_string())
+    }
+
+    let input = "_.";
+
+    let result = run_decoder(input, seq, TestParser::default());
+
+    assert!(result.as_ref().is_ok(), "{}", result.unwrap_err());
+    assert_eq!(result.unwrap(), [""]);
+}
diff --git a/tests/parser.rs b/tests/parser.rs
index a3337e7a..7c319741 100644
--- a/tests/parser.rs
+++ b/tests/parser.rs
@@ -46,7 +46,7 @@ mod tests_std {
     use combine::parser::byte::{alpha_num, bytes};
     use combine::parser::byte::num::be_u32;
     use combine::parser::char::{char, digit, letter};
-    use combine::stream::easy::{Error, Errors, ParseError};
+    use combine::stream::easy::{self, Error, Errors};
     use combine::stream::state::{SourcePosition, State};
     use combine::Parser;
 
@@ -76,7 +76,7 @@ mod tests_std {
     #[test]
     fn sep_by_consumed_error() {
         let mut parser2 = sep_by((letter(), letter()), token(','));
-        let result_err: Result<(Vec<(char, char)>, &str), ParseError<&str>> =
+        let result_err: Result<(Vec<(char, char)>, &str), easy::ParseError<&str>> =
             parser2.easy_parse("a,bc");
         assert!(result_err.is_err());
     }