purescript-strings icon indicating copy to clipboard operation
purescript-strings copied to clipboard

Add startsWith and endsWith

Open triallax opened this issue 4 years ago • 12 comments

These functions are already present in purescript-stringutils but many people, especially ones coming from JavaScript, expect them to be present in the main strings library.

Should I make a PR to fix this?

triallax avatar Sep 24 '20 17:09 triallax

I would say I'm not really in favour of it, personally. We're not trying to replicate the JS API here, and it already has stripPrefix and stripSuffix which can be used for this purpose - generally these functions are more useful too, as often a startsWith would be followed by chopping the prefix off too. Not having the test-only versions hopefully means people will stumble over these more useful functions, rather than being stuck in their JS ways.

That's just my opinion though 🙂

garyb avatar Sep 24 '20 18:09 garyb

To be clear, startsWith prefix is isJust <<< stripPrefix prefix.

natefaubion avatar Sep 24 '20 18:09 natefaubion

@garyb you do make a good point. I just think that the discoverability of this is low for new people. Should I add an example to the cookbook?

triallax avatar Sep 24 '20 18:09 triallax

Sure! That sounds like a good idea.

garyb avatar Sep 24 '20 19:09 garyb

I actually ran into this issue a week ago. I ended up using stringutils because my goal was to solve my problem (does this string start with "X"?), not think about how I could achieve my goal via some other way. For some reason, stripPrefix never stood out to me as being helpful.

So, I would agree that discoverability is the issue here.

JordanMartinez avatar Sep 24 '20 19:09 JordanMartinez

FWIW I've somewhat often wanted startsWith/endsWith. I would not be opposed to adding them with docs that refer to stripPrefix/stripSuffix.

natefaubion avatar Sep 24 '20 23:09 natefaubion

I'm not sure what to do here. Should I open a PR here and add startsWith and endsWith like @natefaubion is suggesting, or should I add examples to the cookbook?

triallax avatar Sep 26 '20 09:09 triallax

@mhmdanas I'm just throwing in a point-of-view that startsWith/endsWith are not always functions you don't want. I've written lots of code that checks a prefix to classify something, but still needs to retain the prefix for convenience. I'm not really fighting for their inclusion, other than saying that it's expected functionality, and I think it's fine if we did include them while also suggesting that stripPrefix/stripSuffix may be more appropriate.

natefaubion avatar Sep 28 '20 00:09 natefaubion

What about performance, is it not a consideration? I mean if this is a standard functions of String in JS.

wclr avatar Mar 10 '21 06:03 wclr

Stripping a string prefix is cheap (i.e. O(1)) in JS because JS strings are immutable. We should avoid unnecessary FFI here because that creates problems for alternative backends.

hdgarrood avatar Mar 10 '21 12:03 hdgarrood

Stripping a string prefix is cheap (i.e. O(1)) in JS because JS strings are immutable.

We can't be sure that is the case on other backends (it might as well be for at least most; I'm not sure).

We should avoid unnecessary FFI here because that creates problems for alternative backends.

I don't think it should be much of a problem because it's just two extra functions, but yeah, it's still two more functions to implement in the FFI.

triallax avatar Mar 10 '21 13:03 triallax

I think a backend which uses a mutable type for Strings is probably incorrect; if you want to write a PureScript backend for a language which does not provide an immutable String type, then you should probably map PureScript strings to some other type in your backend which is an immutable string. We should really write a spec for what is expected of a backend, and specifically how the primitive types work. There are also problems around the encoding of a String as well (whether invalid UTF-16 is allowed, for instance).

hdgarrood avatar Mar 10 '21 13:03 hdgarrood