Skip to content
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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Added a find function to bit_array #604

Closed
wants to merge 0 commits into from

Conversation

codemonkey76
Copy link

No description provided.

Copy link
Member

@lpil lpil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hello! Could we have this in the form of split_once please 馃檹

This will also be in the hot path of many applications so we'll need to have the most optimised implementation possible. For Erlang that means using the Erlang standard library implementation. I'm not sure what's best on JavaScript- we'll need to see if the typed array buffer class being used as some method for this.

Thank you! This will be super nice to have and use. 馃挏

test/gleam/bit_array_test.gleam Outdated Show resolved Hide resolved
<<>>
|> bit_array.find(<<"bar":utf8>>)
|> should.be_error
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's also have some tests for non-byte aligned bit arrays. These tests would need to have the @target(erlang) attribute as currently the JavaScript target only supports byte aligned bit arrays.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are still needed! Thank you

Copy link
Member

@lpil lpil left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good so far but the build is currently failing!

src/gleam/bit_array.gleam Outdated Show resolved Hide resolved
src/gleam_stdlib.mjs Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants