search

mrkkrp / parser-combinators

Lightweight package providing commonly useful parser combinators
Other
52 stars 15 forks source link

Redefining permutation parser interface #34

Closed recursion-ninja closed 3 years ago

recursion-ninja commented 3 years ago

I updated the Control.Applicative.Permutations module to only require an Applicative instance. This required a complete rewrite of the underlying code and data structures. As a result the code in this module may be less performant than previously so I have marked this as a breaking change in the CHANGELOG.md and parser-combinators.cabal files with a version bump to 1.3.0.

Since the original code is potentially more efficient depending on the underlying parser, I took that permutation parser code which relied on Monad instances and added it to the new module Control.Monad.Permutations. This is in line with the design philosophy of the parser-combinators library, as I understand it.

Close #33.

mrkkrp commented 3 years ago

It looks like the new code does not behave in the same way:

https://github.com/mrkkrp/parser-combinators/pull/34/checks?check_run_id=1817381898#step:11:200

@recursion-ninja Is this expected?

recursion-ninja commented 3 years ago

@mrkkrp no, that's not expected. Let me look into this.

recursion-ninja commented 3 years ago

@mrkkrp It turns out that there was a subtle defect in the existing monadic permutation parser. If the permutation was searching for "a,b,c" with default values "x,y,z" and encountered "a,b,x", the permutation parser would consume the second ',' and succeed by returning the default value z in place of c. This behavior is incorrect.

The desired behavior is that if an effect/separator that occurs between permutation components is encountered, there must immediately be a permutation component following it or else the parser should fail.

For example: Input Output
"a,b,q" Failed! Expected 'c' after "a,b,"
"a,bq" Success! Result ('a','b',z')

The incorrect behavior was specified in the test suite. I corrected the functionality in the Applicative and Monad versions of the permutation parser and updated the test suite to reflect corrected behavior.