parser-ts icon indicating copy to clipboard operation
parser-ts copied to clipboard

fix #47 P.either should fail with fatal error, if second parser faile…

Open regevbr opened this issue 4 years ago • 1 comments

This is a bit of a "big" PR, so I can split it if necessary.

After going through the entire library for educational purposes, I have came about many things I wanted to add/fix/test and I have came up with the following:

  • added charC - a case insensitive char parser.
  • added stringC - a case insensitive string parser.
  • added oneOfC - a case insensitive one of strings parser.
  • Added non breaking change support for the between parser to be able to provide 2 different left and right parsers
  • Fixed the semigroup to mark as fatal if any was fatal and not if just the first one
  • Obviously added tests to all additions/changes, maintaining 100% coverage
  • Added some missing tests here and there
  • Enhanced the command example
  • Enhanced small bits of code here and there

@gcanti please let me know how do you want to proceed here, I would appreciate any feedback :-)

regevbr avatar Jul 07 '21 16:07 regevbr

Hey @regevbr - thank you very much for your work on this PR!

However, in general, I would suggest that PRs be kept highly focused on whatever issue is being addressed. Otherwise it can be quite difficult to tease apart what the actual purpose of the PR was.

Could you please trim down the PR to address only your suggestion relating to the ParseError module?

In addition, I'm still not 100% sure if the semantics of using semigroupAny here is correct so I do think having @gcanti weigh in would be helpful.

IMax153 avatar Aug 18 '21 11:08 IMax153