Conversation
hdgarrood
left a comment
hdgarrood
left a comment
There was a problem hiding this comment.
Thanks! I'm in favour of course but I'll wait and see what other maintainers think.
|
Oh actually, do you think you could update the comments so that it's clear that with |
|
👍 from me, but I assume we're going to wait for the next round of breaking changes? |
hdgarrood
left a comment
hdgarrood
left a comment
There was a problem hiding this comment.
Oh actually I've just realised, Data.String.CodePoints needs a corresponding change at the same time.
Speaking of the next round of breaking changes, were we going to do that before or after 0.12?
Regarding the comments, I think with what you have there, it would be reasonable to expect splitAt 0 "hi" to return { before: "h", after: "i" }, so I think it would be good to make it absolutely clear (while this is at the front of my mind). How about this:
Returns a string split into two substrings at the given index, where
beforeincludes all of the characters up to (but not including) the given index, andafteris the rest of the string, from the given index on. That is,(splitAt i str).beforecontains exactlyicharacters (code units), unlessiis negative (in which case it will be the empty string), orstritself is not that long (in which case it is equal tostr).
|
Thanks for the review! Does this look good? |
| splitAt :: Int -> String -> Maybe { before :: String, after :: String } | ||
| -- | Splits a string into two substrings, where `before` contains the code | ||
| -- | points up to (but not including) the given index, and `after` contains the | ||
| -- | rest of the string, from that index on. |
There was a problem hiding this comment.
Why not have the same comment here as in Data.String?
|
Other than my last comment I think this looks great now, thank you! |
|
Okay, thanks! |
4 participants
Fixes #78, breaking change.