Add more string functions in Extensions, to the String module.
I propose we compile first a list of nice-to-have functions.
This is related to https://github.com/fsharp/fslang-suggestions/issues/102 and it's motivated by a discussion on slack with @cannorin and @ellersoft
Here is what I currently have: https://github.com/cannorin/prelude.fs/blob/master/src/String.fs
Here's an exhaustive list with comments, function by function.
Looking at @cannorin link, we have:
startsWith / endsWith: We have them already. We can eventually add versions for other collections (Seq, List, Array).
contains: We have it already. Since List.contains exists, we need to match, so we'll take a single char. To check substrings we have isSubstring.
findIndex / findIndexAfter / findLastIndex / findLastIndexAfter : I think the most important is findIndex, since List.findIndex exists already, we'll need to match it by accepting a predicate. We can also match by adding tryFindIndex.
insertAt / removeAt / removeAfter: Not sure if they are that useful.
remove : Not sure it worths, if we take the indices out, it's simply replace x "" y.
substringAfter / substring: They use indices, if we have take and skip we have already that functionality in a function that exists also in other collections.
normalize / normalizeWithForm: We have normalize already which in fact is normalizeWithForm.
toLower / toUpper: We have them already, not accepting culture as it's more common to use them as CI, you can always use the member if you want to specify culture.
padLeft/ padLeftBy/ padRight/ padRightBy: We can add them but I would call padLeftWith instead of padLeftBy
trim : We have it as trimWhiteSpaces
trimBy / trimBySeq : We might have to add them, not sure it if worths.
trimStart / trimEnd / trimStartBy / trimStartBySeq : I would vote to add only the version that takes an array of strings since it's the most complete.
removeEmptyEntries : Not sure if it worths.
toChars / ofChars: We need to add them.
nth : We can add item instead.
rev : We can add it and make it match with rev for collections.
whileBase: Looks very specific.
take / skip : We need to add them, we have them already working with the generic functions, we can also add the safe versions limit / drop, see note (*).
takeWhile / skipWhile: We need to add them, we have them already working with the generic functions.
build : We need to add it, not sure of the name.
Now looking at @EBrown8534 work we have:
explode / implode : They seem to be the same as toChars / ofChars. Just a matter of naming, I personally like more the toChars / ofChars names.
splitOpt / split: Seems to be already covered by split.
contains / contains2 : contains2 exists as isSubString, the version with the arguments reversed is not worth adding IMHO.
trim : same comments as in the previous listing.
stripDQuotes / stripSQuotes / stripQuotes / digitsOnly / lettersOnly: Very specific ones, their implementation reads better and are easy to use IMHO.
startsWith: We have it already.
indexOf / indexOfC: I think we can add findIndex see comments above, in the first listing.
padLeft : same comments as in the previous listing.
toCodePoints / fromCodePoints: Not sure, looks interesting, are there good use cases around?
hasText / hasNonWhitespace: Very specific ones, their implementation reads better and are easy to use IMHO.
replace: we have it already.
take: same comments as in the previous listing.
truncate : We can add it, together with drop. Note (*).
getBytes : Nice to have, not sure about the name, we have already generic toBytes / ofBytes
subStr : Same comment as in the previous listing.
(*) String.truncate is the same as limit when used with a string. The decision not to use the name truncate was made in order to avoid shadowing the math truncate function. The question remains if we're going to add this function in the string module, should we add it as truncate, limit or both (by duplicating it)?
This is just a first overview, feel free to disagree, add your comments, also let us know if there are other interesting functions out there.
whileBase is used to implement takeWhile and skipWhile and not exposed to users so you can ignore it.removeEmptyEntries exists while I have splitSkipEmpty. You can ignore it too.split* functions. What's your thought on them?StringBuilder with printf/printfn is very useful in practice: https://github.com/cannorin/prelude.fs/blob/master/src/String.fs#L137-L146 . I love to hear your comment about it too.Thanks for the input ❤️
For the note (*), I like to have both by duplicating it. truncate corresponds well to its BCL member counterpart while limit matches to the other naming convention of F#+, so I can't tell which is better.
Great feedback.
I think I missed the explanation of split as I mentioned already but it was in the channel, not here.
So, if you go to the related issue (which is a fslang suggestion) in the very first comment, you'll see how the split function we have already covers most splitting cases, otherwise let me know a specific case and I will try to show you how it is covered. Also note that that split function is consistent with other collections, and it has a generic version that follows some rules if you consider a string as an array of chars. I'm happy to expand on this.
Regarding the functions accepting a culture, we could consider them if we find a nice way to organize them. Maybe with a naming convention, the problem with this approach is that when you start combining them they become a bit odd.
Another possibility is considering some sort of dependency injection, like opening a module which is in fact a type, accepting a parameter (something like OCaml functors) which provides the same functions but working with a specific culture, this has the advantage that leads to less repetitive code, because you specify the culture only once.
I saw the StringBuilder extensions, definitely we can consider them, do you have some specific use cases to illustrate?
I've also had issues around culture and how some libraries goes with thread culture. It can become quite annoying when the culture is per user and you don't know that execution related to specific users will be grouped into specific threads.
I don't think we'll allow culture sensitive functions as it will break referential transparency, nor would I allow them to be in the .net framework had I the power to decide over their inclusion. I guess we will all agree these functions are out of discussion.
But the functions from @cannorin are OK from that perspective, since they take the culture as an explicit parameter, so that lead us to some interesting discussions.
@gusty For toCodePoints / fromCodePoints, consider strings with surrogate pairs:
The following character: U+2C288 (𬊈), we'll use FSI to evaluate the following:
"𬊈" |> Seq.toArray
// val it : char [] = [|'�'; '�'|]
"𬊈" |> toCodePoints
// val it : seq<int> = seq [180872]
The char is a 16-bit data-type, and represents the UTF-16 values. This means, for a character like the above which is encoded as 0xD870 0xDE88, the UTF-16 string in .NET represents it with _two_ char values. Where Seq.toArray pulls _just_ the char values, String.toCodePoints as I implemented takes the two surrogate-pairs and converts them to a _single_ int value.
Up to you if you want to take it or not, but I use it _a lot_ to ensure I deal with internationalized text properly when I'm doing single-character work. It's also necessary for "emoji" work, as emojis fall under the supplementary planes (U+010000 to U+10FFFF).
With regard to truncate:
Array, List and Seq use truncate, if we intend to match those API's we _must_ use truncate as the name, even though it may shadow the math function.
That said, so long as RequireQualifiedAccess is used, we should be OK, because one would need to String.truncate anyway.
I agree, we need to match them.
I think it's more important to match the .truncate of other collections, than matching the generic limit function.
What is not clear to me is whether to add only truncate or add limit as well.
The current design encourage using the unqualified function for the generic versions, but that's not always possible. There is also another case with nth / item. Apart from the fact that nth was deprecated in F# core, the generic versions have a slightly different meaning (item is more tied to the indexable abstraction).
I don't see a need for limit, TBQH. I think it adds another thing that might cause confusion. Why would we have two identical functions? As a new user, I might now think they do different things. Seq doesn't have limit, and since a String is a char seq, I don't think String should have limit.
That's true. It's better to live with the exceptional case that truncate alone clash with our generic truncate and we use limit ONLY there, to move forward with that problem.
We do our best to have consistent names, specially if the base (FSharp.Core) is not in good shape in this regard.
Agreed. Consistency within the ecosystem should always be priority 1, that's what encourages people to take a look.
I also think the triple-slash XML doc comments are 100% necessary, so even if we want to change them that's fine, I just want to make sure that we use them for others.
All of my string functions have them. If you want to pull any in, you are more than welcome to and I'm waiving any "right to credit" or anything. For the purposes of this library, you have express access to mine and to ignore the MIT license credit / copyright requirements. You can pull the XML doc comments (triple-slashes) as well.
@gusty Functor-like dependency injection sounds good to me. For the StringBuilder extension, it will be used like below with the String.build function from my String.fs:
let s =
String.build (fun builder ->
builder.printfn "%s" "hello"
builder.printfn "%i%i" 4 2
...
)
The motivation to use this extension is to avoid the repetitive use of ignore caused by the fact StringBuilder's Write and WriteLine returns the StringBuilder for method chaining. Also, sb.WriteLine(sprintf "%i" 42).WriteLine(sprintf "%s" "foo").ToString() looks odd and ugly imo, but this might be just a personal impression.
As from the previous discussions, these are the candidates so far:
(*) Generic version already exists.
Note: the build and the StringBuilder functions looks nice to me, but let's open another issue for StringBuilder extensions as at the moment we don't have any of them, and it might require more discussion.
Regarding the functions taking a culture as parameter, I propose also to discuss them in a different issue, because I think they need more in depth study as I can't find a good proposal at the moment. So let's move them to another issue in order to speed up this one.
Thoughts? Anyone willing to take care of the PR ?
@gusty Looks good to me (including about creating new issues). I'll create a PR with @EBrown8534 co-authored later today or maybe tomorrow.
@EBrown8534 Could I just use your noreply github email address or could you tell me your preferred one? It's needed to co-author you and I like to co-author you if you don't mind.
@cannorin (Edit) Actually, use the "company" support email: [email protected]
@EBrown8534 It seems [email protected] is not associated with your GitHub account. See here: https://help.github.com/en/articles/adding-an-email-address-to-your-github-account
@cannorin _grumble_ Probably because GitHub is built around single-user, and that is a company-associated email.
Just use my No-Reply address.
@EBrown8534 iirc you can add secondary email addresses as well
and that will avoid my doing push -f :joy:
ahh you have a company account or org as well, right?
Yeah, the email I gave you was the "Org" account (the "official" author), but it seems that GitHub doesn't like that, so just use my account (however you have to).
imo we should have a variant of findIndex which calls the IndexOf(String) overload. it may better come with a different name than findIndex, as in isSubstring vs contains currently exist in F#+.
I agree, we can add that functions to other collections, I think those functions are useful.
We have to decide which name is better for that function.
I'm tempted to go for a generic name for all these functions (as in the case of contains) as opposed to a specific name for strings (as in the case of isSubstring).
Here are some ideas: findSubstringIndex, findElementsIndex, findSubsequenceIndex or the same names without the suffix Index since it is expected.
Note the relation between this functions and current split implementation.
Considering adding that to other collections, I think findSubsequenceIndex is a good option, although it feels too long for me. I feel like dropping Index and call it findSubsequence, but I think findSequenceIndex is also acceptable. Maybe we can also generalize isSubstring to containsSequence or something like that.
If that is the case, it would be better creating a new issue since it is going out of the topic (about adding string functions).
I think having different names:
String.findSubstringArray.findSubarrayList.findSublistSeq.findSubseqfindSubsequence (generic)is a possibility, but see how redundant is the collection name, as it's already in the module name, which is enforced.
What about findSlice ?
After researching a bit, I found that Slice is currently used to express a "subSequenced part", it also applies to multi-dimensional arrays and we might want to keep consistent with that meaning when finding slices on multidimensional arrays, since we won't specify a sequence but a multidimensional array that it's part of it.
At the same time, the term Item is currently used to name indexed elements, which is consistent with our use in the Indexed abstraction. Note that in this library we have nth and item which are more or less the same, but technically an item can have a multidimensional index, or a string (maps), whereas nth always use an integer.
This lead us to:
The question remains whether to drop the suffix Index or not. I think in FSharp.Core some find functions use predicates and returns results, but those results are items, not slices.
Using predicates for slices is a bit more complicated, one can assume that it will start yielding items when the predicate becomes true, and stop when it becomes false, ignoring the rest of the collection. This functionality is interesting but a bit more specific, so if such a function is eventually added, its name should be very specific as well.
So, my vote goes for findSlice which in a sense would become the inverse of getSlice, leaving aside the fact that indices are overloaded in one sense to be able to play with startings and endings and on the other side only starting is returned, assuming that the end position is redundant as the length is always known.
It sounds great! I prefer to keep the suffix (since it can be confusing with find: ('a -> bool) -> *<'a> -> 'a, as mentioned), so findSliceIndex is my preferred way to go.
I would assume that index postfix indicates that it returns the slice indexes? While if you don't have the postfix I would assume you get a continuous subset of a sequence.
Ok, let's move with findSliceIndex if everybody agree on that.
I'll implement String.findSliceIndex and String.tryFindSliceIndex in #159. For other data types and the generic version, I think it's better to do in a new PR.
Yes, the discussion about the other collections is in principle for the naming, but they don't belong to this PR.
This looks good :+1:
I was wondering if generic Substring is handled somewhere?
I have reviewed helpers I have created and only noted:
let takeLast n (str : string) =
str.Substring(str.Length - n)
@adz I think take and skip will do that job (in your case, str |> String.skip (String.length str - n))
Exactly, more generally speaking substrings are a specific instance of a "slice" as per our discussion above.
So, knowing this you can apply all the functions that manipulate slices, including take, skip, truncate, drop family of functions and F# native's GetSlice or more idiomatically slice notation someCollection.[3..6]
Btw, your code reminded me that once we had negative slices in this library and your function was simply: let takeLast n (str : string) = str.[.. -n] which makes the function redundant as its call is longer that its implementation. Unfortunately a breaking change in F# shadowed our negative slices (see this discussion: https://github.com/dotnet/fsharp/issues/3692)
skip does read more cleanly then Substring... but not as good as takeLast or takeFromEnd.
I didn't know about slices :+1: thanks! I'm learning FSharpPlus and even Fsharp slowly. I have more F# than C# experience so it's interesting :).
I note, you have padLeft/padRight and trimLeft/trimRight and startsWith/endsWith... those are for strings only... but I thought it would be good/consistent to support the same kind of thing working for slices - I find it easy to follow take, drop, but when working from the back of a string I'd have to think down to a lower level (get length subtract x... etc) - so I'd want that helper function.... I know I can always create my own but seems like the point of this library.
1) Why not add similar duals for end of string?
takeFromEnd, dropFromEnd, truncateEnd, skipEnd
Is it because would make a mess of generic types like Take, etc?
2) Negative input?
That negative slice index sounds like it would have been good.
Do you think it makes sense to have negative numbers on take, skip, truncate, drop?
drop -2 = remove last two chars
take -2 = take last two chars
truncate -2 = take last two chars
skip -2 = remove last two chars
I suppose you want to be compatible with .net :(
3) Throwing...
take/skip use indexing and can throw, where truncate/drop are more lenient.
I first assumed take would not throw but I guess it's something that must be learned and not so obvious in FSharpPlus.
I'd like to have tryTake and trySkip to make it clearer....
4) Just for comparison...
Elm has left/right (same as take/takeEnd), dropLeft/dropRight, and slice (allowing negatives)
https://package.elm-lang.org/packages/elm/core/latest/String
I think that is very clear API and functions are self explanatory. They don't throw, and just cut off (acting like truncate/drop).
It's interesting that Elm 'List' has head/tail/take/drop more closer to Haskell, and doesn't try to match to a generic. You could do that here too... use specific functions?
Also, coming from Elm, I kind of wish the last/tryLast pattern was unsafeLast/last, but I'm not going to convince all the F# world :)
5) Finally, to be clear, I like FSharpPlus a lot and hope to maybe give an outsiders perspective. I haven't used it yet, but keep getting drawn back to it. Will probably pull it in soon to my F# codebases.
Thanks for work on this library :heart:
I would not assume that take would throw either, so could be that you have identified a bug.
iirc Seq.take and Seq.skip are the only implementation in the standard library not throwing an exception (unless iterated to the end of the sequence, as it is lazily evaluated).
tryTake / trySkip variant which returns option<f<'a>> is very hard to implement in seq because they are lazy and their length is not available until iterated.
Similarly, takeFromEnd, dropFromEnd, truncateEnd, skipEnd are all hard to properly implement in seq (consider an infinite sequence). Personally I disagree to accept negative inputs on take etc, as it would act differently for different collections (seq does not implement slicing for the same reason above) and just confuse the users (read: me).
Also, imho F# is F# and not Elm or Haskell, and F#+, as the name suggests, is also not a library to make F# look like Elm of Haskell. Each language has its own culture, and we should respect it when writing that language (btw I agree Elm has clearer API, so don't get me wrong. It just does not fit to F# culture imo).
Indeed. I agree. Though Take and Skip works in a certain way in the standard library of .net. Defining take and skip over string you'd expect them to work and fail in a similar manner.
@adz
We could add them, but yes, it will duplicate the number of functions we have (skip, take, truncate, drop, skipWhile, takeWhile). IMHO when designing a library, specially for FP you should not aim to cover all with specific functions but to have composable functions that together can cover the most, and right now combining those functions with length gives you that functionality.
I am a bit less concerned for the infinite seq case since FSharp.Core already has functions that fail on infinite sequences or that are not as efficient in seq's as there was a decision some years ago in FSharp.Core to prioritize consistency. Here in F#+ that decision was from the beginning and it's written in our design guidelines.
I really, really like that idea and it's not just ELM, also python and javascript have them, but as already stated, F# didn't went in that direction (.NET in general didn't), though there is a long standing suggestion to add it, with interesting arguments against it. This is a sensitive topic and it needs some discussion, if they ever solve the "shadowing" problem for indices I would open an issue to start a discussion on negative slices.
Unlike the previous point, here's a decision that F# did which surprisingly did not went in the C# direction. For some reason (maybe by mistake or to align with slices) FSharp.Core came to light with take and skip functions that throw. As much as we disagree with it, we can't fix that here as it would cause confusion, possible unexpected and hidden breaking changes to existing code that is starting to reference this library.
So that's why this library added the safe versions with different names drop and limit (or truncate when non generic). The set of try functions have a different meaning, they would be return Some x (or None) and we'll be force to match all the time. I would like to see a scenario where they are justified, given that we have the "dropping" versions.
These comparisons are interesting and other languages are a great source of inspiration, I have a look at Haskell and Purescript mainly to see how they solve specific issues and if it doesn't create a problem in F# we can adopt it, but Haskell is old and also has its own problems due to wrong decisions made in the past, so it's not something to follow blindly and new languages (like ELM) are also interesting.
I do sympathize with the convention of unsafeX and I would like to add as much as possible total functions by default but unfortunately if we take it to extremes we will also start contradicting and introducing unexpected changed to F# core functions. Actually I think there is a problem in that there is no current convention as tryX normally returns an option but it could also return a Choice or a Result (Either in other languages).
5.
I'm glad to have a view from someone without C# / F# experience, I can imagine that many critics from such point of view will be for things that are actually consequence decisions that were are actually made earlier, in FSharp.Core and that we are kind of forced in that direction, even if we're not happy with that. As stated before, we have to be careful in this concern, this library already went into fields that are controversials and not specially loved by some part of the F# community, but we can stand it as long as it doesn't break or force you to use it. We try not to be invasive, just add more stuff to F# as its name suggest it.
There were some discussions recently because people (possible like you) who are beginners in F# are learning F# with this library and getting confused and I think that's an aspect to consider, as this library aims for more consistency it should be exactly the opposite experience IMHO.
@cannorin Fair enough. I agree on most points.
I'd still like it on string :) I noticed FSharpLu went for 'right':
https://github.com/microsoft/fsharplu/blob/master/FSharpLu/Text.fs#L104
@gusty thanks for your thoughts too. I guess I'd prefer FSharpTotal but it's not reasonable with culture :)
I like the idea of being consistent. It helps discoverability.
BTW I'm not learning F# from this library, but keep hitting things I'm making little utils for (using Result and would like computation expression, or String functions, or Async.map... etc).
I learned C# a few years back, and after a few months, I translated our .net codebases to F#. They're not big, so doable - it's a smaller side system from the main app we work on. So more F# than C# :+1: but still need to look at how .net or C# works often.
I'd still like it on string :) I noticed FSharpLu went for 'right':
Indeed for String there's a good case for it, as opposed to Seq<_>.
I didn't expect Seq.skip and Seq.take to work like that. I'm more used to the c# variant I guess 😉
Thanks @gusty!
Now we should create new issues regarding, iirc,
String.build and StringBuilder extensionfindSliceIndexSure !
Feel free to go ahead with those issues, or any other you think.
Most helpful comment
@gusty Looks good to me (including about creating new issues). I'll create a PR with @EBrown8534 co-authored later today or maybe tomorrow.
@EBrown8534 Could I just use your noreply github email address or could you tell me your preferred one? It's needed to co-author you and I like to co-author you if you don't mind.