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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix String with escapes prefix matching on Erlang. #3139

Merged
merged 1 commit into from
May 20, 2024

Conversation

aznashwan
Copy link
Contributor

Fixes #3126.

Fixes String prefix matching on Erlang when the prefix contains escape characters or unicode codepoints by ensuring the correct prefix byte length is matched against.

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! Thank you for this

Could you reuse the code here please? I would like to avoid having two different bits of code for this 🙏

fn utf16_no_escape_len(str: &EcoString) -> usize {

It'll need to be make slightly more flexible to specify whether it's utf8 (Erlang) or utf16 (JavaScript)

@aznashwan aznashwan force-pushed the erlang-string-escape-seq-matching branch from 3ee5afd to 4ba43ae Compare May 15, 2024 21:33
Fixes gleam-lang#3126.

Fixes String prefix matching on Erlang when the prefix
contains escape characters or unicode codepoints by ensuring
the correct prefix byte length is matched against.

Signed-off-by: Nashwan Azhari <aznashwan@icloud.com>
@aznashwan aznashwan force-pushed the erlang-string-escape-seq-matching branch from 4ba43ae to beea7b5 Compare May 15, 2024 21:35
@aznashwan
Copy link
Contributor Author

Could you reuse the code here please?

Done, hadn't noticed it before since I only investigated the Erlang part of the compiler tree but it was basically exactly what I needed.

I've adapted the function to directly return the updated string, properly treat the escape chars Gleam supports, and moved it to a top-level crate::strings module for lack of a better place to put it, but please feel free to suggest a new one.

I've also split the specific prefixes tested against to make the transpiled code a little easier to follow.

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.

Lovely! Thank you.

@lpil lpil merged commit be10970 into gleam-lang:main May 20, 2024
12 checks passed
PgBiel added a commit to Glistix/glistix that referenced this pull request May 26, 2024
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.

Inconsistent aliased string prefix matching on Erlang when prefix contains control/unicode characters.
2 participants