Added some shortcuts for commonly used flags

[Risto-Stevcev]

No description provided.

[paf31]

What about making flags into a Monoid and having individual values for each flag?

[Risto-Stevcev]

Added the changes 😄

[garyb]

The Monoid looks good to me, but maybe it should live in its own module, since you might want to imports the options under qualification independent from the regex functions. I'd probably give them real names too, rather than the single letters.

Also, and this is just a style thing, but please don't whitespace-align things in columns. 😄 While it's true these things are unlikely to change, since it's a JS interface, we prefer not to do that as they make for worse diffs if something comes along that needs the "column" to be wider. We don't have an officially adopted style guide, but there's this: https://github.com/ianbollinger/purescript-style-guide/ which is being worked on by consensus, and has a section about not aligning code.

[Risto-Stevcev]

Is something like Data.String.Regex.Class for the module name okay? I can put the Semigroup, Monoid, and Show instances in there.
What if the flags are called noFlag, gFlag, iFlag, etc.. ? I was also thinking of keeping them in Data.String.Regex, unless you think it makes more sense to keep them elsewhere

[paf31]

Why not use the full names, like global instead of gFlag etc.?

[Risto-Stevcev]

My reasoning was that it's less likely to clash with existing names in a module and the names are shorter. It's an interface to the JS RegExp object, so devs already know the flags as g, i, u, ... rather than global, ignoreCase, etc. That's why I went with single letters.

And in the situation where you use a few of them, you can fit that in one line nicely: (g <> i <> u). It could potentially clash with existing names (especially i), but I wouldn't mind that, especially if they live in a separate module like Data.String.Regex.Flags, which can be qualified if needed (ie. Flags.i)

[garyb]

Data.String.Regex.Flags is exactly what I had in mind, not the .Class thing - for the exact reason that then you can import the options qualified. I'd prefer proper names to g and i too.

[paf31]

It could potentially clash with existing names (especially i), but I wouldn't mind that, especially if they live in a separate module

I'm not too worried about this. Names will tend to overlap but we can import qualified, and we should try to improve on JS where possible - I certainly find myself having to look up the meanings of the various one-letter flags regularly.

[Risto-Stevcev]

That's true, you end up having to look up them up. Ok, I'll update

[paf31]

Please use newtype and derive Newtype RegexFlags _ here.

[paf31]

Oh, never mind the Newtype instance. It won't work because of the row there. Ugh, we need ~ types.

[garyb]

It won't? I can see that it wouldn't work as long as the RegexFlagsRec synonym is used rather than the type, but what's the problem here?

[paf31]

It would desugar to instance Newtype RegexFlags {..}

[garyb]

Ah of course. Damn, that's a big deal actually.

[garyb]

Looks good 👍

[garyb]

@paf31 any other comments before merge? (not meaning to hassle you with updates, I get that you're at work 😄 just wanted to double check)

[paf31]

looks good!