-
-
Notifications
You must be signed in to change notification settings - Fork 458
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
Refactor LinkParser
#4576
Refactor LinkParser
#4576
Conversation
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.
clang-tidy made some suggestions
|
||
return exp.match(host).hasMatch(); | ||
if (current > u'9' || current < u'0') |
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.
QChar::isDigit implementation seems to handle unicode stuff, your solution seems good to me
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.
Right, isDigit
also checks if the character is the "Nd" unicode category.
Code looks good, some small nitpicks that you can change if you feel like it's reasonable |
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.
clang-tidy made some suggestions
Description
This PR is the continuation of #4507 and #4436.
These are the main changes:
QStringView
overQStringRef
goto
.In my tests (
chatterino-benchmark --benchmark_filter=LinkParsing --benchmark_repetitions=100
), this version is about 6x faster:Before
After
As you can see, I didn't change the
isValidTld
it's now the bottleneck here. The main reason is that aQString
needs to be created and case-converted. Ideally, it could use aQStringView
and have case-insensitive search functions.