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 emoji unified/non-qualified version for sending & parsing #4840

Merged
merged 20 commits into from
Oct 7, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
abac7be
fix: use unified emoji for sending, & use both unified & nonQualified…
pajlada Sep 24, 2023
8d6834f
refactor: rename parsed shortCodes to shortNames to avoid name clashes
pajlada Sep 24, 2023
7d942a5
refactor: mark emojis name as unused in the emoji set loading
pajlada Sep 24, 2023
62548c3
refactor: use contains(..) instead of count(..)
pajlada Sep 24, 2023
af0a92b
refactor: const auto the emoji
pajlada Sep 24, 2023
424f0b7
refactor: move anon namespace out to flatten in
pajlada Sep 24, 2023
93618aa
refactor: constify & rename TONE_NAMES
pajlada Sep 24, 2023
a9736f5
refactor: parseEmoji make shortCode const ref
pajlada Sep 24, 2023
ff9d123
move map include
pajlada Sep 24, 2023
89129a8
Merge remote-tracking branch 'origin/master' into fix/emoji-unified-v…
pajlada Oct 5, 2023
bc8e731
Do non-qualified emoji checking & remove duplicate comment
pajlada Oct 5, 2023
027dcf6
Move byte parsing assert to right after the unified code is parsed
pajlada Oct 5, 2023
5198a28
Parse non-qualified-code better better
pajlada Oct 5, 2023
c479a6c
unrelated change: add missing unordered_map include in XDGDirectory
pajlada Oct 5, 2023
31026b3
wip: work on benchmarks for emoji parsing
pajlada Oct 5, 2023
92133d8
Add new benchmark function for QStringView
pajlada Oct 7, 2023
9531758
Use parse2
pajlada Oct 7, 2023
30458d9
Merge remote-tracking branch 'origin/master' into fix/emoji-unified-v…
pajlada Oct 7, 2023
1213173
use QString::size_type or auto where possible in parse method
pajlada Oct 7, 2023
8a1dfc2
Add changelog entry
pajlada Oct 7, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
- Minor: The account switcher is now styled to match your theme. (#4817)
- Minor: Add an invisible resize handle to the bottom of frameless user info popups and reply thread popups. (#4795)
- Minor: The installer now checks for the VC Runtime version and shows more info when it's outdated. (#4847)
- Bugfix: Fixed an issue where certain emojis did not send to Twitch chat correctly. (#4840)
- Bugfix: Fixed capitalized channel names in log inclusion list not being logged. (#4848)
- Bugfix: Trimmed custom streamlink paths on all platforms making sure you don't accidentally add spaces at the beginning or end of its path. (#4834)
- Bugfix: Fixed a performance issue when displaying replies to certain messages. (#4807)
Expand Down
117 changes: 117 additions & 0 deletions benchmarks/src/Emojis.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -55,3 +55,120 @@ static void BM_ShortcodeParsing(benchmark::State &state)
}

BENCHMARK(BM_ShortcodeParsing);

static void BM_EmojiParsing(benchmark::State &state)
pajlada marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: invalid case style for function 'BM_EmojiParsing' [readability-identifier-naming]

Suggested change
static void BM_EmojiParsing(benchmark::State &state)
eParsing);bmEmojiParsing

benchmarks/src/Emojis.cpp:136:

- 
- sing);
+ 
+ sing);bmEmojiParsing

{
Emojis emojis;

emojis.load();

struct TestCase {
QString input;
std::vector<boost::variant<EmotePtr, QString>> expectedOutput;
};

const auto &emojiMap = emojis.getEmojis();
std::shared_ptr<EmojiData> penguin;
emojiMap.tryGet("1F427", penguin);
auto penguinEmoji = penguin->emote;

std::vector<TestCase> tests{
pajlada marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: variable 'tests' of type 'std::vector' can be declared 'const' [misc-const-correctness]

benchmarks/src/Emojis.cpp:75:

- ase> tests{
+ ase> const tests{

{
// 1 emoji
"foo 🐧 bar",
// expected output
{
"foo ",
penguinEmoji,
" bar",
},
},
{
// no emoji
"foo bar",
// expected output
{
"foo bar",
},
},
{
// many emoji
"foo 🐧 bar 🐧🐧🐧🐧🐧",
// expected output
{
"foo ",
penguinEmoji,
" bar ",
penguinEmoji,
penguinEmoji,
penguinEmoji,
penguinEmoji,
penguinEmoji,
},
},
};

for (auto _ : state)
{
for (const auto &test : tests)
{
auto output = emojis.parse(test.input);

bool areEqual = std::equal(output.begin(), output.end(),
pajlada marked this conversation as resolved.
Show resolved Hide resolved
test.expectedOutput.begin());

if (!areEqual)
{
qDebug() << "BAD BENCH";
for (const auto &v : output)
{
if (v.type() == typeid(QString))
{
qDebug() << "output:" << boost::get<QString>(v);
}
}
}
}
}
}

BENCHMARK(BM_EmojiParsing);

template <class... Args>
static void BM_EmojiParsing2(benchmark::State &state, Args &&...args)
pajlada marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

warning: invalid case style for function 'BM_EmojiParsing2' [readability-identifier-naming]

Suggested change
static void BM_EmojiParsing2(benchmark::State &state, Args &&...args)
sing);bmEmojiParsing2

benchmarks/src/Emojis.cpp:166:

- ualNumEmojis;
- ng2, one_emoji, "foo 🐧 bar", 1);
- ing2, two_emoji, "foo 🐧 bar 🐧", 2);
+ ualNumEmojis;
+ ng2, one_emoji, "foo 🐧 bar", 1);
+ ing2bmEmojiParsing2o 🐧 bar 🐧", 2);bmEmojiParsing2

benchmarks/src/Emojis.cpp:170:

- APTURE(
+ APTURE(bmEmojiParsing2

{
Emojis emojis;

emojis.load();

auto argsTuple = std::make_tuple(std::move(args)...);
auto input = std::get<0>(argsTuple);
auto expectedNumEmojis = std::get<1>(argsTuple);
for (auto _ : state)
{
auto output = emojis.parse(input);
int actualNumEmojis = 0;
for (const auto &part : output)
{
if (part.type() == typeid(EmotePtr))
{
++actualNumEmojis;
}
}

if (actualNumEmojis != expectedNumEmojis)
{
qDebug() << "BAD BENCH, EXPECTED NUM EMOJIS IS WRONG"
<< actualNumEmojis;
}
}
}

BENCHMARK_CAPTURE(BM_EmojiParsing2, one_emoji, "foo 🐧 bar", 1);
BENCHMARK_CAPTURE(BM_EmojiParsing2, two_emoji, "foo 🐧 bar 🐧", 2);
BENCHMARK_CAPTURE(
BM_EmojiParsing2, many_emoji,
"😂 😂 😂 😂 😂 😂 😂 😂 😂 😂 😂 😂 😂 😂 😂 😂 😂 😂 😂 😂 😂 😂 😂 😂 "
"😂 😂 😂 😂 😂 😂 😂 😂 😂 😂 😂 😂 😂 😂 😂 😂 😂 😂 😂 😂 😂 😂 😂 😂 "
"😂 😂 😂 😂 😂 😂 😂 😂 😂 😂 😂 😂 😂 ",
61);
Loading
Loading