Add some utility functions to Belt Array

[tsnobip]

I added the reverse version of map, forEach and keep.

I fixed some typos and examples of the docs.

I also added a rescript implementation of joinWith (it's around 3 times faster than Js.Array.joinWith on common examples, and still faster for arrays up to around 10,000 elements (result string of around 100,000 characters), not sure this function should be used for bigger arrays anyway. This function is only applicable for string array, not sure if it should be placed inside Belt.Array since the other functions apply to polymorphic arrays.

I couldn't find any test files for Belt so I just tested the functions manually. I'll add some tests if needed.

[bobzhang]

I am in general in favor of this except some comments. Note I am going to take an vacation, so I will merge after it. Other reviews are welcome

[tsnobip]

Ok thanks, there's no hurry anyway, I'll update my PR accordingly.

[bobzhang]

do you think it's a good idea to use an optional argument here, for example:

map : 'a array -> ?reverse:bool -> ( 'a -> 'b) -> 'b array

[tsnobip]

Good idea! that would indeed be quite elegant. I always forget you don't need an additional unit argument in such cases. That would help reduce the amount of functions needed in the API. We could also do the same for reduceReverse for coherence.

Maybe we could make a little survey on discourse?

[bobzhang]

Maybe we could make a little survey on discourse?

sounds good to me

[tsnobip]

Well there seems to be no real consensus on this: https://forum.rescript-lang.org/t/api-convention-for-reverse-versions-of-belt-functions/457/ Among 22 respondents, the bigger part would prefer to have extra functions, a smaller part would like to have a flag in the existing functions, some suggest to put those functions inside a Reverse module (the same could be done with the WithIndex functions but that would be a breaking change I guess) and some would even prefer not to have these functions at all in Belt.

The integration of these functions is up to you anyway, I'm OK with all these options, you're the Boss 😀

[bobzhang]

hi @tsnobip can we have the least controversial utility toString merged first?

[tsnobip]

sure, would like me to revert the commit where I add the reverse versions and rebase to trunk?

[bobzhang]

@tsnobip you can also branch from that commit and make a separate PR, either works for me, thanks

[tsnobip]

ok I created a separate PR (#4748) to make things easier and cleaner.

[cristianoc]

Could be revisited now if rebased on master, assuming there's still interest.

[tsnobip]

Could be revisited now if rebased on master, assuming there's still interest.

Yeah I think those functions can still be interesting from time to time. I'll rebase it on master when I have some time.