Adding the rest of string sdk #494
Conversation
❌ Deploy Preview for scala-morphir failed.
|
examples/morphir-elm-projects/evaluator-tests/src/Morphir/Examples/App/StringTests.elm
Outdated
Show resolved
Hide resolved
Wait, I forgot to review test cases. Consider that a tentative lgtm while I go back with a magnifying glass, please :) |
| @@ -1875,42 +1881,49 @@ basicsAlwaysTest x = | |||
| in | |||
| f [ 4 ] | |||
|
|
|||
|
|
|||
| {-| Test: SdkBasics/basicsClampTest | |||
| -} | |||
| basicsClampTest : a -> a -> a -> a | |||
There was a problem hiding this comment.
I'd change all of these type variables from a to number... IDK if it's the intention that that be a rigidly enforced in morphir, but it is in elm, and enforced or not is the only way of indicating that the type must belong to the number typeclass.
There was a problem hiding this comment.
i can do this in a separate pr since this was just part of running the formatter
There was a problem hiding this comment.
The formatter changed number to a?
There was a problem hiding this comment.
Oh wait, nvm I see what you meant, sure
| {-| Test: SdkBasics/basicsIdentityTest | ||
| -} | ||
| basicsIdentityTest : a -> a | ||
| basicsIdentityTest x = | ||
| x | ||
|
|
||
|
|
||
| {-| Test: SdkBasics/basicsPowerTest | ||
| -} | ||
| basicsPowerTest : a -> a -> a |
There was a problem hiding this comment.
See above regarding number
|
I think finding a "split" in java/scala that takes a string rather than regex (so we can't throw) is importnat. After that's fixed, lgtm. |
|
|
||
| val slice = DynamicNativeFunction3("slice") { | ||
| (context: NativeContext) => (start: RT.Primitive.Int, end: RT.Primitive.Int, str: RTString) => | ||
| val result = str.value.slice(start.valueAsInt, end.valueAsInt) |
There was a problem hiding this comment.
Sorry, one last difference. Elm allows negative indexing in its slice [0], and java does not.
This seems like a difference that we can live with to me (meaning, leave this impl as-is), and would fit with the java/scala morphir backend tending to use default java behaviors instead of matching elm. @DamianReeves , any opinion?
[0] https://package.elm-lang.org/packages/elm/core/latest/String#slice
There was a problem hiding this comment.
I'm okay with filing an issue (and JIRA) to track the need to align slice functionality... I do think it is desired to have this behavior match
Adding the following functions: