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: Only display spaces between words in reply context #4977

Merged
merged 4 commits into from
Nov 26, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
- Bugfix: Fixed an issue where the setting `Only search for emote autocompletion at the start of emote names` wouldn't disable if it was enabled when the client started. (#4855)
- Bugfix: Fixed empty page being added when showing out of bounds dialog. (#4849)
- Bugfix: Fixed issue on Windows preventing the title bar from being dragged in the top left corner. (#4873)
- Bugfix: Fixed an issue where reply context didn't render correctly if an emoji was touching text. (#4875)
- Bugfix: Fixed an issue where reply context didn't render correctly if an emoji was touching text. (#4875, #4977)
- Bugfix: Fixed the input completion popup from disappearing when clicking on it on Windows and macOS. (#4876)
- Bugfix: Fixed double-click text selection moving its position with each new message. (#4898)
- Bugfix: Fixed an issue where notifications on Windows would contain no or an old avatar. (#4899)
Expand Down
2 changes: 1 addition & 1 deletion src/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -929,7 +929,7 @@ if (CHATTERINO_DEBUG_NATIVE_MESSAGES)
endif ()

if (MSVC)
target_compile_options(${LIBRARY_PROJECT} PUBLIC /EHsc /bigobj)
target_compile_options(${LIBRARY_PROJECT} PUBLIC /EHsc /bigobj /utf-8)
endif ()

if (APPLE AND BUILD_APP)
Expand Down
21 changes: 13 additions & 8 deletions src/messages/MessageElement.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -664,16 +664,23 @@ void SingleLineTextElement::addToContainer(MessageLayoutContainer &container,
QString currentText;

container.first = FirstWord::Neutral;

bool firstIteration = true;
for (Word &word : this->words_)
{
if (firstIteration)
{
firstIteration = false;
}
else
{
currentText += ' ';
}

for (const auto &parsedWord : app->emotes->emojis.parse(word.text))
{
if (parsedWord.type() == typeid(QString))
{
if (!currentText.isEmpty())
{
currentText += ' ';
}
currentText += boost::get<QString>(parsedWord);
QString prev =
currentText; // only increments the ref-count
Expand Down Expand Up @@ -714,7 +721,8 @@ void SingleLineTextElement::addToContainer(MessageLayoutContainer &container,

container.addElementNoLineBreak(
(new ImageLayoutElement(*this, image, emoteSize))
->setLink(this->getLink()));
->setLink(this->getLink())
->setTrailingSpace(false));
}
}
}
Expand All @@ -723,9 +731,6 @@ void SingleLineTextElement::addToContainer(MessageLayoutContainer &container,
// Add the last of the pending message text to the container.
if (!currentText.isEmpty())
{
// Remove trailing space.
currentText = currentText.trimmed();

int width = metrics.horizontalAdvance(currentText);
container.addElementNoLineBreak(
getTextLayoutElement(currentText, width, false));
Expand Down
6 changes: 6 additions & 0 deletions src/providers/emoji/Emojis.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,12 @@ namespace chatterino {

void Emojis::load()
{
if (this->loaded_)
{
return;
}
this->loaded_ = true;

this->loadEmojis();

this->sortEmojis();
Expand Down
2 changes: 2 additions & 0 deletions src/providers/emoji/Emojis.hpp
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,8 @@ class Emojis : public IEmojis
// Maps the first character of the emoji unicode string to a vector of
// possible emojis
QMap<QChar, QVector<std::shared_ptr<EmojiData>>> emojiFirstByte_;

bool loaded_ = false;
};

} // namespace chatterino
110 changes: 110 additions & 0 deletions tests/src/Emojis.cpp
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
#include "providers/emoji/Emojis.hpp"

#include "common/Literals.hpp"

#include <gtest/gtest.h>
#include <QDebug>
#include <QString>

using namespace chatterino;
using namespace literals;

TEST(Emojis, ShortcodeParsing)
{
Expand Down Expand Up @@ -53,3 +56,110 @@ TEST(Emojis, ShortcodeParsing)
<< "Input " << test.input.toStdString() << " failed";
}
}

TEST(Emojis, Parse)
{
Emojis emojis;

emojis.load();

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

auto getEmoji = [&](auto code) {
std::shared_ptr<EmojiData> emoji;
emojis.getEmojis().tryGet(code, emoji);
return emoji->emote;
};

auto penguin = getEmoji("1F427");
auto cool = getEmoji("1F192");
auto skinTone6 = getEmoji("1F3FF");
auto england = getEmoji("1F3F4-E0067-E0062-E0065-E006E-E0067-E007F");
auto womanRunningtone2 = getEmoji("1F3C3-1F3FC-200D-2640-FE0F");
auto kissWomanManTone1 =
getEmoji("1F468-1F3FB-200D-2764-FE0F-200D-1F48B-200D-1F468-1F3FB");
auto heavyEqualsSign = getEmoji("1F7F0");
auto coupleKissTone1Tone2 =
getEmoji("1F9D1-1F3FB-200D-2764-FE0F-200D-1F48B-200D-1F9D1-1F3FC");
auto hearHands = getEmoji("1FAF6");

const std::vector<TestCase> tests{
{
"abc",
{"abc"},
},
{
"abc def",
{"abc def"},
},
{
"abc🐧def",
{"abc", penguin, "def"},
},
{
"abc 🐧def",
{"abc ", penguin, "def"},
},
{
" abc🐧 def ",
{" abc", penguin, " def "},
},
{
"🐧",
{penguin},
},
{
"🐧🐧🐧🐧",
{penguin, penguin, penguin, penguin},
},
{
// england
u"\U0001F3F4\U000E0067\U000E0062\U000E0065\U000E006E\U000E0067\U000E007F"_s
// cool
"\U0001F192"
// skin tone 6
"\U0001F3FF"
// woman running tone2
"\U0001F3C3\U0001F3FC\u200D\u2640\uFE0F"
// [running] non-qualified
"\U0001F3C3\U0001F3FC\u200D\u2640",
{england, cool, skinTone6, womanRunningtone2, womanRunningtone2},
},
{
// kiss woman tone1 man tone 1
u"\U0001F468\U0001F3FB\u200D\u2764\uFE0F\u200D\U0001F48B\u200D\U0001F468\U0001F3FB"_s
// [kiss] non-qualified
"\U0001F468\U0001F3FB\u200D\u2764\u200D\U0001F48B\u200D"
"\U0001F468"
"\U0001F3FB"
// heavy equals sign
"\U0001F7F0",
{kissWomanManTone1, kissWomanManTone1, heavyEqualsSign},
},
{
// couple kiss tone 1, tone 2
u"\U0001F9D1\U0001F3FB\u200D\u2764\uFE0F\u200D\U0001F48B\u200D\U0001F9D1\U0001F3FC"_s
// [kiss] non-qualified
"\U0001F9D1\U0001F3FB\u200D\u2764\u200D\U0001F48B\u200D\U0001F9D1"
"\U0001F3FC"
// heart hands
"\U0001FAF6",
{coupleKissTone1Tone2, coupleKissTone1Tone2, hearHands},
},
};

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

// can't use EXPECT_EQ because EmotePtr can't be printed
if (output != test.expectedOutput)
{
EXPECT_TRUE(false)
<< "Input " << test.input.toStdString() << " failed";
}
}
}
Loading