New regex parser
#170
New regex parser
#170
Conversation
src/Parsing/String.purs
Outdated
| -- | | ||
| -- | [*MDN Advanced searching with flags*](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Regular_Expressions#advanced_searching_with_flags) | ||
| mkRegex :: forall m. String -> RegexFlags -> Either String (ParserT String m String) |
There was a problem hiding this comment.
Is there a reason for choosing mkRegex? I don't think we use mk as a parser prefix anywhere else. I personally suggest just sticking to regex, as it also mirrors Data.String.Regex.
There was a problem hiding this comment.
Is it worth exposing an unsafeRegex as well?
There was a problem hiding this comment.
Ok I renamed it to regex.
It's pretty easy to write unsafeRegex. I think I'd rather leave that to the users for now.
bench/Json/Parsing.purs
Outdated
| Just n' -> pure n' | ||
| Nothing -> fail "Expected number" | ||
| jsonNumber = case mkRegex """(\+|-)?(\d+(\.\d*)?|\d*\.\d+)([eE](\+|-)?\d+)?""" noFlags of | ||
| Left err -> unsafePerformEffect $ throw err |
There was a problem hiding this comment.
This can just be unsafeCrashWith.
test/Main.purs
Outdated
| ( let | ||
| prependContext m' (ParseError m pos) = ParseError (m' <> m) pos | ||
| p = region (prependContext "context1 ") $ do | ||
| _ <- string "a" | ||
| region (prependContext "context2 ") $ do | ||
| string "b" | ||
| in | ||
| case runParser "aa" p of | ||
| Right _ -> assert' "error: ParseError expected!" false | ||
| Left (ParseError message _) -> do | ||
| let messageExpected = "context1 context2 Expected \"b\"" | ||
| assert' ("expected message: " <> messageExpected <> ", message: " <> message) (message == messageExpected) | ||
| logShow messageExpected | ||
| ) |
There was a problem hiding this comment.
I think that you can remove the parens and in.
There was a problem hiding this comment.
The p was shadowing something else, so now those bindings are locally scoped.
There was a problem hiding this comment.
Sorry, I'm saying that, because you are in a do block, ending with a let/in like this is exactly the same as just using a let in the do block, with the last statement being the runParser. The parens don't do anything semantically. If inserting the parens, vs using a do/let results in different warnings, it's a bug in the compiler.
There was a problem hiding this comment.
Unless more tests are added after this block. Anyway I deleted the parens.
jamesdbrock
force-pushed
the
mkregex
branch
3 times, most recently
from
5f15a68 to
e2e2acd
Compare
|
Thanks for the review @natefaubion ! |
Replace
regexwith different function.Resolves #156 #157
Checklist: