-
-
Notifications
You must be signed in to change notification settings - Fork 21.9k
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 crash caused by trailing spaces #103825
base: master
Are you sure you want to change the base?
Fix crash caused by trailing spaces #103825
Conversation
I can confirm that this fixes #103797 for me. |
Vector<String> str_stripped_split = str.strip_edges().split_spaces(); | ||
if (!str_stripped_split.is_empty() && | ||
str_stripped_split[0] != "#region" && | ||
str_stripped_split[0] != "#endregion") { | ||
match = false; | ||
} |
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.
Vector<String> str_stripped_split = str.strip_edges().split_spaces(); | |
if (!str_stripped_split.is_empty() && | |
str_stripped_split[0] != "#region" && | |
str_stripped_split[0] != "#endregion") { | |
match = false; | |
} | |
String stripped_str = str.strip_edges(); | |
if (!stripped_str.begins_with("#region") && !stripped_str.begins_with("#endregion")) { | |
match = false; | |
} |
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.
This works but undoes the original fix where this bug was introduced.
For example, "#region" and "#endregion" here are highlighted, but don't actually create a region you can collapse or expand.
We need to check for whitespace after region/endregion to fix this.
To improve this code's efficiency, it would be nice if get_spaces
had a maxsplit parameter like the normal split
function does, but I don't know if its worth adding that extra functionality just for this.
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.
Yes, we can use the maxsplit
parameter since we are only checking the 0th substring.
Or alternatively we could extract the alphanumeric identifier after the #
. Since the current version does not take into account cases like #region:Name
. I think we should add methods like String.get_{ascii,unicode}_identifier(from: int = 0) -> String
.
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.
Is there a source for what characters should be after #region
or is that up in the air? The only examples the docs show are with whitespace.
Fixes #103797
Issue introduced in #101319
(Also is mildly more efficient since it isn't redundantly splitting twice)