Use unsafeCrashWith for unsafeRegex (future fromEither not Partial) + Unix line endings

[toastal]

The future version of Data.Either.fromRight is no longer Partial, so fromJust ∘ hush is a way around this as fromJust is still Partial but not ideal. Alternatively, Data.Either should have like Data.Either.Partial.fromRight to restore previous behavior (closes either #56).

It was decided that it would be better to unsafeCrashWith on the Left-hand side of Regex instantiation.

Seemed really out of place to have one file with DOS line endings. Also, purty was run on this.

[davidchambers]

I suggest splitting this into two pull requests, @toastal, to make the significant changes more apparent in the diff. :)

[toastal]

This PR is as draft while I can figure out what's going out with Data.Either. It makes more sense to do this as one PR whenever that discussion is complete.

[hdgarrood]

I think unsafeRegex itself probably ought to have a Partial constraint, really. Then you can use fromRight (crashWith "...") with the total version of fromRight.

[hdgarrood]

Then again, adding a Partial constraint to unsafeRegex could upset a few people, and might not provide much additional safety if it’s mostly only used at sites where you’d want the constraint to be unsafely discharged anyway (which I suspect is the case).

[natefaubion]

Then you can use fromRight (crashWith "...") with the total version of fromRight

Note, you can't actually use this due to strictness. This will just crash immediately.

[natefaubion]

I think unsafeRegex itself probably ought to have a Partial constraint, really. Then you can use fromRight (crashWith "...") with the total version of fromRight.

Do people use anything but unsafeRegex 😆. I don't think I've ever used anything else, personally. Surely there are no sensible defaults or ways to recover from it, and a crash is a crash.

[hdgarrood]

To be honest, I don't think I've ever used regexes in PureScript in any form.

[hdgarrood]

I think sensible recovery is possible and indeed necessary if you're constructing a regex based on user input or any other data obtained at runtime. Also, just because there's no sensible way of recovering from an invalid regex constructed from a string literal, it doesn't mean a Partial constraint isn't appropriate - in fact I'd say Partial constraints are only for cases where there's no sensible way of recovering. The point of it is just to indicate that the caller needs to be careful, and to give you the option to either discharge it (if the partiality has been dealt with) or propagate it if not.

[natefaubion]

Also, just because there's no sensible way of recovering from an invalid regex constructed from a string literal, it doesn't mean a Partial constraint isn't appropriate - in fact I'd say Partial constraints are only for cases where there's no sensible way of recovering. The point of it is just to indicate that the caller needs to be careful, and to give you the option to either discharge it (if the partiality has been dealt with) or propagate it if not.

FWIW I think we already had this discussion when we added unsafeRegex. I don't see a point in having both an unsafe prefix and a Partial constraint. I don't think this breaking change is worth it IMO.

[hdgarrood]

I agree with not adding a Partial constraint, I just wanted to point out that, imo, not having a sensible way of recovering doesn't justify omitting a Partial constraint in general. For me, the reason not to do it is that it makes the ergonomics terrible, and it doesn't provide very much safety because 99% of the time you would want to discharge the Partial constraint immediately anyway.

[toastal]

Not sure how to add a CHANGELOGto this. There's nothing existing already. Is there a dependency issue to be finished here? I've not touched the bower stuff in years.

[hdgarrood]

We don’t store changelog files in the core libraries, we just look over the commit log and add release notes along with each release in GitHub, so no action needed there. There’s also no action needed re dependencies since what you have here will work with both old and new versions of either. When we get around to updating this library properly we will update everything to master temporarily, but that’s not necessary just yet.

[hdgarrood]

Can you show what the errors look like now?

[toastal]

@hdgarrood For unsafeRegex "[" noFlags Uncaught Error: unterminated character class. This is the same error as new RegExp("[") in Firefox -- which is less helpful than the Chromium of Uncaught SyntaxError: Invalid regular expression: /[/: Unterminated character class.

[hdgarrood]

Works for me. Thanks!

[JordanMartinez]

This was the same solution I used in #129. Sorry for not including your commits @toastal. I didn't realize this PR already existed.

[toastal]

:'( ... I really wanted to be a contributor