token and SliceStream do not play together well

[ncalexan]

First, thanks for the library. It's a fun way to learn Rust!

I'd like to understand some of the API decisions the library makes. First, my motivating use case is to parse a token stream (not a character stream), and my token's do not implement Copy because they include String data, like:

struct Token {
    Keyword(String),
    ...
}

If I use SliceStream, I can parse &[Token]. However, I can't really use token, because the Item type of SliceStream is &'a Token and that's not something I can create with the correct lifetime. It seems to me like token should have where Item : Clone and explicitly .clone() it's argument. I see at https://github.com/Marwes/combine/blob/master/src/combinator.rs#L200 that we can't actually parse without Item : Clone.

Is this deliberate? Oversight? Am I using SliceStream incorrectly?

[Marwes]

The reason for the Clone bound is that combine assumes that Stream is a type that can be cloned (which is necessary for the or parser which implements alternation). Since Stream can be cloned it follows that its tokens must be cloneable as well (any token that does not implement Clone could still be duplicated by cloning the stream stream.uncons().clone() vs stream.clone().uncons() so there is no reason that they would not be possible to clone). If we then also assume that streams and tokens are cheap to clone (if they were expensive to clone then parsing would be slow) then it follows that the predicates might as well take Item instead of &Item as it is much more convenient in common cases such as Item = u8 or Item = char.

That was my reasoning in any case :).

As for the lifetime problem, that is not something I had considered actually. I can't think of anyway to make token available for that case as the token requires exactly the Item type since that is used to construct the error message if the parser fails

combine/src/combinator.rs

Line 209 in 6a57d7f

error.errors.push(Error::Expected(Info::Token(self.c.clone())));

. Using the the satisfy parser instead should work though.

[Marwes]

Reasoning for Copy #44