Repository navigation
Regex for "match everything till eol" matches nothing #231
Description
Describe the bug
Using a pretty simple ".*$" regexp results in No Regex pattern match. That is with noFlags flag that implies multiline: false, so $ should match something.
To Reproduce
Run this code:
module Main where
import Prelude
import Data.Either (Either(..))
import Data.String.Regex.Flags (noFlags)
import Effect (Effect)
import Effect.Console (logShow)
import Parsing (Parser, fail, runParser)
import Parsing.String (regex)
-- The regex from `Parsing` but with more convenient interface.
regexP :: String -> Parser String String
regexP re = case regex re noFlags of
Left compileError -> fail $ "failed to compile regex: " <> compileError
Right ret -> ret
s = """My
multiline
string
"""
main :: Effect Unit
main = do
logShow $ runParser s $ regexP ".*$"Expected behavior
The parser ought to return string My
Actual behavior
ParseError "No Regex pattern match" (Position { column: 1, index: 0, line: 1 }
Activity
garyb commentedon Oct 15, 2024
You would need the
multilineflag to be enabled for it to match"My"- but it should return"", because it'll be matching on$but nothing else (.doesn't match newlines withoutdotAll- if you remove the trailing\nit should return"string").I'm not really sure where the culprit is here, neither the parsing nor
Data.Stringregex stuff looks wrong from an initial glance.garyb commentedon Oct 15, 2024
Oh actually, immediately after saying that I see it. The parser regex inserts a
^at the start of the pattern somultilineordotAllwould be required for anything to match. I think it's correct that it only matches from the current parser position, but maybe the consequence of that needs emphasising in the comment for it (it does already mention it matches from the current position).natefaubion commentedon Oct 15, 2024
It seems like there's maybe a slight misunderstanding wrt JS RegExp semantics. Without the multiline flag
$always matches end of the string, not end of the line. Maybe you are expecting the inverse, where the multiline flag is where$matches end of string, but it is in fact matching end of line? It's definitely confusing.Hi-Angel commentedon Oct 15, 2024
Oh, just as I was typing out, you clarified, thank you! Yes, I am not a web-programmer, and in all other languages "multline regex being true" means the opposite, i.e. when you consider whole string more or less as a single line. Okay, I see now, let me try that.
Hi-Angel commentedon Oct 15, 2024
Sorry, I'm confused here 😅 So, the
regexmatches from current position, and for that it opaquely inserts a^to the beginning of the expression. This sounds like a bug on its own, because if a user haveline1\n|line2where the|marks the caret position, and then a user asks for^line2, inserting the^will result in^^line2.natefaubion commentedon Oct 15, 2024
It gets wrapped with
^(...). So If you add an extra ^, it's unnecessary, but benign. Matching from the current cursor position is the only thing that makes sense for a parser that needs to consume it's input. I'm not sure what it would mean for aregexcombinator to match arbitrary locations. The JS API doesn't tell you where matches happened, so we need an anchor that assumes we can just jump based on the length of the match.Document that for "usual" regex behavior `multiline` is required
Hi-Angel commentedon Oct 15, 2024
Please see my docs change proposal here
Document that for "usual" regex behavior `multiline` is required
jamesdbrock commentedon Oct 16, 2024
Thanks for the docs change proposal.
Also note that if you want “The regex from
Parsingbut with more convenient interface” then you can just use the Data.String.Regex module directly instead of usingParsing.jamesdbrock commentedon Oct 16, 2024
Also, I know this is just your test code, but note that in real code you shouldn't compile the
regexduring the parse for the reasons discussed in Parsing.String.regex.Document that for "usual" regex behavior `multiline` is required
Document that for "usual" regex behavior `multiline` is required
Hi-Angel commentedon Oct 16, 2024
Thank you!
In the
Parsinglibrary context, the convenient interface is one that (in particular) returns aParsing String String. TheData.String.Regex.regexreturnsEither, so still requires wrapping, and if I have to wrap, there's not much difference which one it's gonna be.Fair warning, because it's a snippet from an actual project. However, I've read the documentation paragraph referred (the discussion about constant vs dynamic generation in particular) but didn't grasp why should't I be compiling it in runtime.
Now that I'm experimenting with
spyI see that it might be due to the fact that regexp doesn't get generated just once but on every call, perhaps that's what you were referring to? In that case though that's a problem, but not the one related to the code or the library, but one in the PureScript compiler. E.g. when I have this code:compiler should detect that it's a pure function-chain, so the calls can be replaced with a constant.
But it's part of the optimization process, and from what I understand the PureScript designers for some reason relegated optimizations to a separate project. So that means that if (or rather I'm hoping "when") my project comes to production, I'll have to use this separate project to produce the production code.
garyb commentedon Oct 16, 2024
That is what he meant, yeah.
Document that for "usual" regex behavior `multiline` is required