Implement take_until_parser_matches

[tomalexander]

Implement a macro that will consume input until the child parser matches

[tomalexander]

Hey, sorry that took so long. Ran into an old friend I hadn't talked to in over half a decade and I've been recovering from that hangover.

[coveralls]

Coverage increased (+0.1%) to 82.845% when pulling 803e17c on tomalexander:take_until_parser_matches into c5090ef on Geal:master.

[coveralls]

Coverage increased (+0.02%) to 87.96% when pulling f387919 on tomalexander:take_until_parser_matches into db94701 on Geal:master.

[chengsun]

This looks very useful, but a bit broken as implemented. Specifically, if the child parser never matches, take_until_parser_matches will not return an Error or Incomplete. For example:

named!(end <&[u8],&[u8]>, tag!("bbb"));
named!(before_bbb, take_until_parser_matches!(end));

assert_eq!(before_bbb(&b"ab"[..]), Incomplete(Needed::Size(3)));
// assert fails: actual value is Done(&b"b"[..], &b"a"[..])

Also, maybe we could also have a take_until_parser_matches_and_consume variant?

[tomalexander]

Thanks for taking a look! That behavior was my intent, but I just checked and you're right. The other take_until* macros throw an error if theres no match. I'll revise (and add more tests for that)

[Geal]

any news on this?

[tomalexander]

Ah sorry. Since posting this merge request I haven't found time to update it (changed jobs, sister's wedding, etc). FWIW this is a feature I still want but I can't commit to any particular timeframe so would you prefer I close the merge request and open a new one later or keep this one lingering?

…

On Jul 17, 2017 3:26 AM, "Geoffroy Couprie" ***@***.***> wrote: any news on this? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#469 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAb-IGx8m4wxG6uHTE4dDmMe44MDP0z9ks5sOzbYgaJpZM4Mh9wo> .

[tomalexander]

Hey, back from the dead. I'm loving the switch to functions in nom version 5! I've updated the PR the latest master, rewrote it as a function instead of a macro, and fixed the style issue chengsun raised about it not returning an error if the parser does not find anything to match.

I haven't implemented take_until_parser_matches_and_consume because I think that would be the same as just a simple terminated(take_until_parser_matches(foo), foo) but if you'd still like it in, I can add it.

[tomalexander]

I thought about it some more and I decided to add in a take_until_parser_matches_and_consume because I realized its a performance thing. As in, terminated(take_until_parser_matches(foo), foo) still has to run the parser foo one additional time, even though we just successfully ran it in take_until_parser_matches. I'm still using the same ErrorKind though, should I be making a distinct one for take_until_parser_matches_and_consume ?

[tomalexander]

Hey, I just made a small design change, so I want to call it out here to make sure that the project is aware of it and agree with it. Namely, I changed it to attempt to use its inner parser on the empty string after the full input has been captured.

Previously, take_until_parser_matches would apply its inner parser on all substrings until the end of the input, but not including the end of the input. So for example, if the input was "foo" it would test f("foo"), f("oo") and f("o") but not f(""). This meant that it could never capture the full input.

An example use-case where this change is helpful is trying to read until the end of the input, excluding any trailing whitespace. You could write this parser as:

assert_eq!(
    Ok(("\n", "foo bar baz")),
    take_until_parser_matches::<_, _, _, (_, ErrorKind)>(all_consuming(multispace0))(
        "foo bar baz\n"
    )
);

but if there was no trailing whitespace, even though multispace0 accepts an empty input, this would fail:

assert_eq!(
    Ok(("", "foo bar baz")),
    take_until_parser_matches::<_, _, _, (_, ErrorKind)>(all_consuming(multispace0))(
        "foo bar baz"
    )
);

After this patch, both code blocks succeed.

Also I see I need to rebase my branch on the latest changes. I'll get to that now.

[tomalexander]

I think its important to point out the difference between this and many_till since the two are very similar parsers.

many_till works by running the first parser and then checking the second parser. Then it runs the first parser again, and then checks the second parser. This repeats until the second parser has a result.

take_until_parser_matches instead iterates forward checking the second parser at each step, returning everything from before the match.

The difference is useful when the first parser is aggressive/insatiable (I want to say 'greedy' but I know in parsing thats the opposite). So, for example, lets say you had a string:
"ababababefg" and you wanted to match as many "ab"s as possible, followed by as many "a"s as possible, as long as its all before a "befg". With many_till you might try to write this as:

let x: IResult<&str, (Vec<&str>, &str)> =
            many_till(alt((tag("ab"), tag("a"))), tag("befg"))("ababababefg");
        assert_eq!(x, Ok(("", (vec!["ab", "ab", "ab", "a"], "befg"))));

But that won't work because the first parser will eat the "b" in "befg", causing the second parser to never match.

Whereas take_until_parser_matches is only attempting to match the end/2nd parser so you could write:

        let x: IResult<&str, (Vec<&str>, &str)> = tuple((
            map_parser(
                take_until_parser_matches(tag("befg")),
                many1(alt((tag("ab"), tag("a")))),
            ),
            tag("befg"),
        ))("ababababefg");
        assert_eq!(x, Ok(("", (vec!["ab", "ab", "ab", "a"], "befg"))));

and it would work.

These are obviously contrived examples, but I think it would be particularly useful for parsing org-mode headlines. The headlines support "tags" at the end of the line, so you could have a headline like:

*** Handling segfaults :thisIsATag:andSoIsThis:

but if the ending isn't a valid tag (for example, having spaces or not having a closing colon) then it gets treated as part of the headline

*** Handling segfaults :this isnt a tag:
*** Handling segfaults :thisAlsoIsntATag

so with take_until_parser_matches you could do:

let (i, (headline, tags)) = tuple((take_until_parser_matches(alt((tag_parser, line_ending))), opt(tag_parser)))(i)?;

but with many_till I don't really see a solution other than:

many_till(anychar, alt((tag_parser, line_ending)))

leaving you with an unfortunate vector of single characters instead of a nice slice.

[NickNick]

Hi! I was taking this for a spin, but got a panic when the 'take_split' tried to index a string between code points:

thread 'test_nom_token_recipe' panicked at 'byte index 10 is not a char boundary; it is inside 'ö' (bytes 9..11) of ` frozen rösti rounds to oven`', /home/nick/.cargo/registry/src/github.com-1ecc6299db9ec823/nom-6.0.1/src/traits.rs:504:7

This happens here:

        for ind in 0..=input_length {
            let (remaining, taken) = i.take_split(ind);
            match f.parse(remaining) {
                Err(_) => (),

I changed it into this (added InputIter, use iter_indices):

    Input: InputTake + InputIter + InputLength + Clone,
    F: Parser<Input, O, Error>,
    Error: ParseError<Input>,
{
    move |input: Input| {
        let i = input;
        for (ind, _) in i.iter_indices() {
            let (remaining, taken) = i.take_split(ind);

Which seems to work for me, but please do check it. :)

Of course, thanks for writing it in the first place! Couldn't have done it myself. 👍

[tomalexander]

Hey thanks for the report and the suggested change! I'll try to look at it this weekend (I'm taking vacation days this week to work on a side project so I don't want get sidetracked during the week)

[tomalexander]

Thanks! I like the change. The only change I needed to add was for checking the parser after the input has been exhausted.

[meritozh]

this is very useful. any process on this PR?

[tomalexander]

Thanks! Last I heard from @Geal he was very busy so he didn't have a lot of time to spend on PRs. AFAIK all thats remaining is code review and merging.

[Geal]

Yup, still pretty busy, but I intend to do a pass on PRs soon

[tomalexander]

Thats wonderful news, thanks!

[meritozh]

@Geal hi，this PR not merged into 6.2？Or we delay it to 7.0?

[homersimpsons]

@Geal I guess this PR could be closed as it seems to be a duplicate of take_till

[meritozh]

@homersimpsons take_till only accept InputTakeAtPosition which mostly is &[u8] or &str, take_until_parser_matches accept Parser.

[chotchki]

I just wanted to add that this pull request works great for my use case.

I'm trying to parse Postgres compatible sql constant strings. There is a variant that auto joins separated strings together ONLY if they are separated by a newline. The take_until_parser_matches_and_consume function is perfect for this.

[Stargateur]

See my comment I don't think this is a good addition to nom

[tomalexander]

Good enough for me, closing