Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tracking PR for 3.0 #105

Merged
merged 30 commits into from
Aug 7, 2017
Merged

Tracking PR for 3.0 #105

merged 30 commits into from
Aug 7, 2017

Conversation

Marwes
Copy link
Owner

@Marwes Marwes commented Jul 24, 2017

This contains the current breaking changes I am planning to do for 3.0. More changes that are yet to be implemented are tracked in https://github.com/Marwes/combine/milestone/4.

Before this is merged I'd expect a few alpha/beta releases to be released on crates.io as well.

@coveralls
Copy link

Coverage Status

Coverage decreased (-1.4%) to 74.031% when pulling d8b439a on 3.0 into c7aef29 on master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.2%) to 75.294% when pulling 43bd600 on 3.0 into c7aef29 on master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.2%) to 75.294% when pulling 43bd600 on 3.0 into c7aef29 on master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.2%) to 75.294% when pulling 5afcf40 on 3.0 into c7aef29 on master.

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.2%) to 75.294% when pulling cb04063 on 3.0 into c7aef29 on master.

@coveralls
Copy link

Coverage Status

Coverage increased (+1.9%) to 77.371% when pulling 15909f7 on 3.0 into c7aef29 on master.

Marwes and others added 15 commits August 7, 2017 18:17
This will make mappings over parse errors be possible as desired in #86

BREAKING CHANGE

Changes the type parameters of `ParseError`
Previously these streams had a `usize` which was really a pointer offset
as their position which easily led to user errors. By wrapping the
offset in a newtype we get custom formatting which should make it
clearer that something strange is going on. As well as a simple way of
converting the offset into a position.

Fixes #104

BREAKING CHANGE

Change `usize` to `PointerOffset` where applicable.
Change `translate_position` calls into the same method on `PointerOffset`
The `range_of` parser makes it possible to "match" on the input parsed
by a non-range parser and extract the range of input which was parsed.
This makes range parsing much more expressive with only a small cost of
an additional method on `RangeStream`

Closes #83

BREAKING CHANGE

`distance` must be defined on `RangeStream`
The `StreamOnce` trait do not need the position to work even if many users of the trait needs it. So this way it becomes possible for wrappers such as `State` to carry types implementing `StreamOnce` but which are not `Positioned`

BREAKING CHANGE

Implementors of `StreamOnce` should implement `Positioned` as well.
`Positioned` must be imported if the `position` methoid is used
This makes it possible to use `choice` to create an alternation parser where the component parsers are of different types. This was still possible before by using `or` or the `choice!` macro but by overloading `choice` we get an easy to read solution without macros

BREAKING CHANGE

This changes the signature of the `Choice` type and the `choice` function
…ixes #79.

This approach separates Positioner from the underlying Stream and from
the underlying Item and Range types.  This allows to track positions
in streams where the Item type is third-party and the 1.0 Positioner
trait cannot be implemented (due to Rust's requirement that a defining
module originate one of the trait and the type).

Open questions:

1) Is this abstraction correct?
2) Is it worth separating Positioner and RangePositioner?
3) Should IndexPositioner be generic over (Item, Range) or over
Stream?
4) Should IndexPositioner use a single PhantomData<(Item, Range)>
rather than two phantoms?
5) Is there a Rust idiom that avoids needing explicitly defining
IndexPositioner::<...>::new() more frequently?

Finally, if some variation of this is suitable for merging, how should
we name the State type, and how should we arrange to transition the
existing State code to the new world order?  (I added a new module
specifically to separate addition from migration.)
This lets us remove the type parameters on `IndexPositioner`
Brings byte parsers closer to char parsers
These have been superseeded by the new type and trait in the `State` module which should prove more flexible (at a slight complexity cost)

BREAKING CHANGE

Import and use the types from the state module instead. Simple uses should look identical
…sing

This makes it so that `BufferedStream::new` actually returns a `BufferedStream` value.

BREAKING CHANGE

Renamed used of `SharedBufferedStream` to `BufferedStream` and `BufferedStream` to `BufferedStreamRef`. Calls to `BufferedStream::new` do not change as `new` is still on `BufferedStream`
Since it seems to be a bug preventing the no_std addition anyway it
is unlikely that older versions will end up working
BREAKING CHANGE

Use the recommended alternatives that has been suggested since they were
depreceated (2.4.0 have all the warnings with the alternatives)
…parsers

This is a tricky fix. Essentially, anytime errors are merged we lost the offset information which is used to add the correct errors after a sequencing parser has failed
…rding to or

Fixes an issue with deeply nested types
This collides with the equivalent function in `combinator` but it can
trivial be renamed using `recognize as recognize_range` if that is a
problem.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants