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

[Core] Add case-insensitive String::containsn #91611

Merged
merged 1 commit into from
May 8, 2024

Conversation

AThousandShips
Copy link
Member

@AThousandShips AThousandShips commented May 6, 2024

Realized the potential need for this when reviewing:

There aren't extremely many cases, and not all String methods have a case-insensitive version, but I felt this one was a prime candidate.

There were a few cases of to_lower().contains cases that I left alone as they already operate on a lowe-case search term so probably faster to use that format

Optionally converted some uses of find with contains where applicable, for readability
Separated these to their own PR

Edit: Will add exposing the method in an optional commit later today

@AThousandShips AThousandShips added this to the 4.x milestone May 6, 2024
@AThousandShips AThousandShips requested review from a team as code owners May 6, 2024 09:31
@AThousandShips AThousandShips force-pushed the string_containsn branch 2 times, most recently from 131c6cc to 30dd881 Compare May 6, 2024 13:45
@AThousandShips AThousandShips requested review from a team as code owners May 6, 2024 13:45
@AThousandShips AThousandShips removed request for a team May 6, 2024 14:27
tests/core/string/test_string.h Outdated Show resolved Hide resolved
editor/animation_track_editor.cpp Show resolved Hide resolved
@akien-mga akien-mga modified the milestones: 4.x, 4.3 May 8, 2024
@akien-mga akien-mga merged commit 1d10132 into godotengine:master May 8, 2024
16 checks passed
@akien-mga
Copy link
Member

Thanks!

@AThousandShips AThousandShips deleted the string_containsn branch May 8, 2024 13:59
@AThousandShips
Copy link
Member Author

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants