-
-
Notifications
You must be signed in to change notification settings - Fork 190
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
Add bit_array.to_string_lossy
#800
base: main
Are you sure you want to change the base?
Add bit_array.to_string_lossy
#800
Conversation
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.
Thank you!
src/gleam/bit_array.gleam
Outdated
// target due to not using the `utf8_codepoint` bit array segment type. Once | ||
// the JavaScript target supports `utf8_codepoint` this function should be | ||
// removed. | ||
@target(javascript) |
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.
It's a shame so much is needed for JavaScript, and this introduces a new @target
, which we have been working hard to remove.
How challenging would it be to have utf8_codepoint
support in JavaScript? That seems like it would be a better solution.
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.
Bit array pattern matching in JS would need to be expanded to support dynamic sizes first, then utf_codepoint
could be implemented on top of that.
I think it's reasonable to wait for that to happen, or alternatively a @target
can be avoided by either using the longer JS-compatible version on both targets (it works fine on Erlang), or by implementing the JS version directly in JS as an external function.
378c5d3
to
f051874
Compare
f051874
to
26fc288
Compare
This PR is done but can't merge until Gleam supports Converting to draft until that's ready. |
Fixes #797.
The separate version of the function for the JavaScript target is a little bit of an eyesore, but can be removed at some future date once
utf_codepoint
pattern matching is supported on that target.