nom
nom copied to clipboard
Implement Parser for tuples
Hello ! My first PR to this project.
This PR does :
- Implement the issue #1417.
- Add some test.
- Modify the
fn tupleto use this new feature.
that breaking change at this point there is no point to keep tuple trait with this change. I agree Tuple is obsolete now. I would tag this for nom 8.
if you want this PR accepted for nom 7 I would reverse the change on Tuple, and we could depreciate it in nom 7 et remove it in nom 8.
I think the tuple function is still useful in case the tuple in question is not composed in another parser. Indeed, tuples implement Parser, not FnMut, so it is impossible to parse in the same way as other parsers. This code doesn't compile :
let (i, result) = (parser1, parser2)(i);
To maintain consistency in calls, I think it could be a good idea to keep it. The Tuple trait is now only used to be able to implement this function, I don't see any other way.
I don't follow you, you want use it as documentation ? Unless I miss something remove Tuple trait the code still compile.
The Tuple trait is still used to implement the tuple function.
We can still do :
let (i, result) = tuple((parser1, parser2))(i)?;
The
Tupletrait is still used to implement thetuplefunction.We can still do :
let (i, result) = tuple((parser1, parser2))(i)?;
Since you change to trait Tuple {} this is impossible, do pub fn tuple<I, O, E: ParseError<I>, List: Parser<I, O, E>> it will compile fine.
By doing this you allow any parser to be given as argument to tuple, and so you allow this code to compile :
let (i, result) = tuple(tag("hello"))(i)?;
The Tuple trait acts now like a marker, identifying parsers that really are tuples. Marker traits are not from my invention, some are defined in the std lib, like std::marker::Copy or std::cmp::Eq.
I am using this marker trait because for now, to my knowledge there is no other way to define a generic function on tuples of any size.
That a valid point, but this lead to a simple conclusion tuple is simply not needed anymore. Tuple trait was actually the Parser trait, tuple() is just a parser that apply a Parser now. It's general enough and you want to limit it. Unless there is a technical reason to limit it I don't see why we should.
I see two outcome:
- rename
tuple()toparse()orapply(), allow a shortcut to(foo, bar).parse(input) - remove it
I was considering the first solution too, I think it's a good idea! I will do that.
Done. It's a real breaking change, since it removes a function and change the signature of some others.
Done. It's a real breaking change, since it removes a function and change the signature of some others.
Well, Tuple was a pub trait since you change it, it was already a breaking change ^^. Big work indeed I will try to double check it.
You are right, I forgot them ! It's fixed
sorry for the delay, it is now merged! I removed the parse combinator as it is not really necessary, but the rest was great, thanks for your help!