-
Notifications
You must be signed in to change notification settings - Fork 469
Add missing docstring for String #7571
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
runtime/Stdlib_String.res
Outdated
@@ -1,6 +1,6 @@ | |||
type t = string | |||
|
|||
@val external make: 'a => string = "String" | |||
@new external make: 'a => string = "String" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, that sounds right, but how does setSymbol
work then?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sadly, it won't. Seems will only work on string object instances.
So the binding is incorrect IMHO. We'd need another abstract type for string object instances, and could then define getSymbol
/setSymbol
on that.
I assume this is a pretty niche use case anyway. Personally I never tried setting a value with a symbol key on a string before. Maybe we should just remove the getSymbol
/setSymbol
bindings from the stdlib?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe we should just remove the getSymbol/setSymbol bindings from the stdlib?
Yes, I think that makes more sense.
rescript
@rescript/darwin-arm64
@rescript/darwin-x64
@rescript/linux-arm64
@rescript/linux-x64
@rescript/win32-x64
commit: |
what? i was using this to get a char iterator for strings. what do i do now?
|
I think we can re-add |
I'm only using
generate the same code, but is terribly ugly. I have this in one single place in 12k lines of code. not sure adding it back in is the right choice, but there should definitely be a way to get a string iterator without hacking it like above I think. in my cause I'm using it for a string tokenizer |
I don't believe we are opposed to adding it back; it was removed because we couldn't find a genuine use case for it. We only had a more artificial test case. See #7626 |
This one is a bit weird, I had to make the
@val
to@new
change to make the examples work.I noticed this in plain JavaScript as well, that not having the
new
keyword, doesn't make it work.Happy to hear your thoughts here, not sure what is best.