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

builtin: fix snake_to_camel when passing a SCREAMING_SNAKE_CASE string #21722

Merged
merged 4 commits into from
Jun 24, 2024

Conversation

ttytm
Copy link
Member

@ttytm ttytm commented Jun 24, 2024

@ttytm
Copy link
Member Author

ttytm commented Jun 24, 2024

Related to this and potential simplifications, would a .is_upper / .is_lower method for chars make it as a builtin?

@spytheman
Copy link
Member

We have u8.is_capital/0 already, which does c >= A && c <= Z, but I could not find a separate implementation for u8.is_lower/0 .

@spytheman spytheman merged commit c5c49d3 into vlang:master Jun 24, 2024
76 checks passed
@ttytm ttytm deleted the builtin/fix-screaming-snake-to-snake branch June 24, 2024 20:16
@kbkpbot
Copy link
Contributor

kbkpbot commented Jun 26, 2024

Bug in code:

assert '_ISspace'.camel_to_snake() == '_i_sspace'
but got '__sspace'
lost the first char I

@ttytm
Copy link
Member Author

ttytm commented Jun 26, 2024

Good catch. Thanks a lot for the test cases @kbkpbot

@spytheman
Copy link
Member

spytheman commented Jun 26, 2024

Why assert '_ISspace'.camel_to_snake() == '_i_sspace' ?

I would have expected _ISspace to become _is_space 🤔 .

@ttytm
Copy link
Member Author

ttytm commented Jun 26, 2024

Good question. From the short research I did before, the initial implementation (without extracting handling of the first characters to reduce the load in the loop) was very similar to the implementation at https://github.com/iancoleman/strcase. So the initial impl and Go's strcase gives _i_sspace 🤔 .

raw-bin pushed a commit to raw-bin/v that referenced this pull request Jul 2, 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.

3 participants