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

Sed expression editing #1542

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
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
33 changes: 33 additions & 0 deletions src/UserSettingsPage.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ UserSettings::load(std::optional<QString> profile)
settings.value(QStringLiteral("user/timeline/enlarge_emoji_only_msg"), false).toBool();
markdown_ = settings.value(QStringLiteral("user/markdown_enabled"), true).toBool();
invertEnterKey_ = settings.value(QStringLiteral("user/invert_enter_key"), false).toBool();
sedEditing_ = settings.value(QStringLiteral("user/sed_editing"), false).toBool();
bubbles_ = settings.value(QStringLiteral("user/bubbles_enabled"), false).toBool();
smallAvatars_ = settings.value(QStringLiteral("user/small_avatars_enabled"), false).toBool();
animateImagesOnHover_ =
Expand Down Expand Up @@ -342,6 +343,17 @@ UserSettings::setInvertEnterKey(bool state)
save();
}

void
UserSettings::setSedEditing(bool state)
{
if (state == sedEditing_)
return;

sedEditing_ = state;
emit sedEditingChanged(state);
save();
}

void
UserSettings::setBubbles(bool state)
{
Expand Down Expand Up @@ -910,6 +922,7 @@ UserSettings::save()
settings.setValue(QStringLiteral("scrollbars_in_roomlist"), scrollbarsInRoomlist_);
settings.setValue(QStringLiteral("markdown_enabled"), markdown_);
settings.setValue(QStringLiteral("invert_enter_key"), invertEnterKey_);
settings.setValue(QStringLiteral("sed_editing"), sedEditing_);
settings.setValue(QStringLiteral("bubbles_enabled"), bubbles_);
settings.setValue(QStringLiteral("small_avatars_enabled"), smallAvatars_);
settings.setValue(QStringLiteral("animate_images_on_hover"), animateImagesOnHover_);
Expand Down Expand Up @@ -1023,6 +1036,8 @@ UserSettingsModel::data(const QModelIndex &index, int role) const
return tr("Send messages as Markdown");
case InvertEnterKey:
return tr("Use shift+enter to send and enter to start a new line");
case SedEditing:
return tr("Allow editing your last message with sed expressions");
case Bubbles:
return tr("Enable message bubbles");
case SmallAvatars:
Expand Down Expand Up @@ -1177,6 +1192,8 @@ UserSettingsModel::data(const QModelIndex &index, int role) const
return i->markdown();
case InvertEnterKey:
return i->invertEnterKey();
case SedEditing:
return i->sedEditing();
case Bubbles:
return i->bubbles();
case SmallAvatars:
Expand Down Expand Up @@ -1341,6 +1358,11 @@ UserSettingsModel::data(const QModelIndex &index, int role) const
return tr(
"Invert the behavior of the enter key in the text input, making it send the message "
"when shift+enter is pressed and starting a new line when enter is pressed.");
case SedEditing:
return tr("If you send a message that is a valid sed expression (e.g. s/foo/bar), try "
"to apply it to your last sent message as an edit instead of sending it as a "
"regular message. If the sed expression cannot be applied to your last "
"message, it will be sent as a normal message.");
case Bubbles:
return tr(
"Messages get a bubble background. This also triggers some layout changes (WIP).");
Expand Down Expand Up @@ -1512,6 +1534,7 @@ UserSettingsModel::data(const QModelIndex &index, int role) const
case ScrollbarsInRoomlist:
case Markdown:
case InvertEnterKey:
case SedEditing:
case Bubbles:
case SmallAvatars:
case AnimateImagesOnHover:
Expand Down Expand Up @@ -1762,6 +1785,13 @@ UserSettingsModel::setData(const QModelIndex &index, const QVariant &value, int
} else
return false;
}
case SedEditing: {
if (value.userType() == QMetaType::Bool) {
i->setSedEditing(value.toBool());
return true;
} else
return false;
}
case Bubbles: {
if (value.userType() == QMetaType::Bool) {
i->setBubbles(value.toBool());
Expand Down Expand Up @@ -2221,6 +2251,9 @@ UserSettingsModel::UserSettingsModel(QObject *p)
connect(s.get(), &UserSettings::invertEnterKeyChanged, this, [this]() {
emit dataChanged(index(InvertEnterKey), index(InvertEnterKey), {Value});
});
connect(s.get(), &UserSettings::sedEditingChanged, this, [this]() {
emit dataChanged(index(SedEditing), index(SedEditing), {Value});
});
connect(s.get(), &UserSettings::bubblesChanged, this, [this]() {
emit dataChanged(index(Bubbles), index(Bubbles), {Value});
});
Expand Down
6 changes: 6 additions & 0 deletions src/UserSettingsPage.h
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@ class UserSettings final : public QObject
Q_PROPERTY(bool markdown READ markdown WRITE setMarkdown NOTIFY markdownChanged)
Q_PROPERTY(
bool invertEnterKey READ invertEnterKey WRITE setInvertEnterKey NOTIFY invertEnterKeyChanged)
Q_PROPERTY(bool sedEditing READ sedEditing WRITE setSedEditing NOTIFY sedEditingChanged)
Q_PROPERTY(bool bubbles READ bubbles WRITE setBubbles NOTIFY bubblesChanged)
Q_PROPERTY(bool smallAvatars READ smallAvatars WRITE setSmallAvatars NOTIFY smallAvatarsChanged)
Q_PROPERTY(bool animateImagesOnHover READ animateImagesOnHover WRITE setAnimateImagesOnHover
Expand Down Expand Up @@ -172,6 +173,7 @@ class UserSettings final : public QObject
void setScrollbarsInRoomlist(bool state);
void setMarkdown(bool state);
void setInvertEnterKey(bool state);
void setSedEditing(bool state);
void setBubbles(bool state);
void setSmallAvatars(bool state);
void setAnimateImagesOnHover(bool state);
Expand Down Expand Up @@ -244,6 +246,7 @@ class UserSettings final : public QObject
int privacyScreenTimeout() const { return privacyScreenTimeout_; }
bool markdown() const { return markdown_; }
bool invertEnterKey() const { return invertEnterKey_; }
bool sedEditing() const { return sedEditing_; }
bool bubbles() const { return bubbles_; }
bool smallAvatars() const { return smallAvatars_; }
bool animateImagesOnHover() const { return animateImagesOnHover_; }
Expand Down Expand Up @@ -315,6 +318,7 @@ class UserSettings final : public QObject
void startInTrayChanged(bool state);
void markdownChanged(bool state);
void invertEnterKeyChanged(bool state);
void sedEditingChanged(bool state);
void bubblesChanged(bool state);
void smallAvatarsChanged(bool state);
void animateImagesOnHoverChanged(bool state);
Expand Down Expand Up @@ -384,6 +388,7 @@ class UserSettings final : public QObject
bool scrollbarsInRoomlist_;
bool markdown_;
bool invertEnterKey_;
bool sedEditing_;
bool bubbles_;
bool smallAvatars_;
bool animateImagesOnHover_;
Expand Down Expand Up @@ -491,6 +496,7 @@ class UserSettingsModel : public QAbstractListModel
ReadReceipts,
Markdown,
InvertEnterKey,
SedEditing,
Bubbles,
SmallAvatars,

Expand Down
64 changes: 58 additions & 6 deletions src/timeline/InputBar.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -450,15 +450,67 @@ InputBar::generateRelations() const
void
InputBar::message(const QString &msg, MarkdownOverride useMarkdown, bool rainbowify)
{
auto trimmed = msg.trimmed();
mtx::events::msg::Text text = {};
text.body = msg.trimmed().toStdString();
text.body = trimmed.toStdString();

// we don't want to have unexpected behavior if somebody was genuinely trying to edit a message
// to be a regex
if (UserSettings::instance()->sedEditing() && room->edit().isEmpty()) [[unlikely]] {
// The bulk of this logic was shamelessly stolen from Neochat. Thanks to Carl Schwan for
// letting us have this :)
static const QRegularExpression sed("^s/([^/]*)/([^/]*)(/g)?$");
Copy link
Member

Choose a reason for hiding this comment

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

this means there is no way to escape a / in the regex. Probably fine, but not great. Also I really like that Vim allows you to use any non-alphanumeric character as the separator. So maybe actually go with the /s/ command :)

auto match = sed.match(trimmed);

if (match.hasMatch()) {
const QString regex = match.captured(1);
const QString replacement = match.captured(2).toHtmlEscaped();
const QString flags = match.captured(3);

std::optional<mtx::events::collections::TimelineEvents> event;
if (!room->reply().isEmpty()) {
auto ev = room->eventById(room->reply());
if (ev && room->data(*ev, TimelineModel::IsEditable).toBool())
event = ev;
} else {
for (int i = 0; i < room->rowCount(); ++i) {
const auto idx = room->index(i);
if (room->data(idx, TimelineModel::IsEditable).toBool()) {
event = room->eventById(room->data(idx, TimelineModel::EventId).toString());
break;
}
}
}

if (event) {
auto other = room->data(*event, TimelineModel::FormattedBody).toString();
const auto original{other};
if (flags == "/g")
other.replace(regex, replacement);
else
other.replace(other.indexOf(regex), regex.size(), replacement);
Comment on lines +488 to +491
Copy link
Member

Choose a reason for hiding this comment

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

That way the first one will do \1 group replacement, the other one won't.

The proper solution is probably to extract the first match and only do a replace on that, then stitch it back together. The regex.size() also is wrong, since it will behave incorrectly with /s/.*// imo.

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm not sure that I understand what you are saying here. Could you elaborate?

Copy link
Member

Choose a reason for hiding this comment

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

  1. If you do a replacement with s/.*//, I think your code will just delete 2 characters at the start of the message.
  2. If you do a replacement like s/[abc]*/__\1__/ the first one will replace aaacbbbaaa with __aaacbbbaaa__, the second one will replace it with the literal text __\1__, I think.


// don't bother sending a pointless edit
if (original != other) {
text.formatted_body = other.toStdString();
text.format = "org.matrix.custom.html";
text.relations.relations.push_back(
{.rel_type = mtx::common::RelationType::Replace,
.event_id =
room->data(*event, TimelineModel::EventId).toString().toStdString()});
room->sendMessageEvent(text, mtx::events::EventType::RoomMessage);
return;
}
}
}
}

if ((ChatPage::instance()->userSettings()->markdown() &&
useMarkdown == MarkdownOverride::NOT_SPECIFIED) ||
useMarkdown == MarkdownOverride::ON) {
text.formatted_body = utils::markdownToHtml(msg, rainbowify).toStdString();
// Remove markdown links by completer
text.body = replaceMatrixToMarkdownLink(msg.trimmed()).toStdString();
text.body = replaceMatrixToMarkdownLink(trimmed).toStdString();

// Don't send formatted_body, when we don't need to
// Specifically, if it includes no html tag and no newlines (which behave differently in
Expand All @@ -473,7 +525,7 @@ InputBar::message(const QString &msg, MarkdownOverride useMarkdown, bool rainbow
// disable all markdown extensions
text.formatted_body = utils::markdownToHtml(msg, rainbowify, true).toStdString();
// keep everything as it was
text.body = msg.trimmed().toStdString();
text.body = trimmed.toStdString();

// always send formatted
text.format = "org.matrix.custom.html";
Expand All @@ -484,9 +536,9 @@ InputBar::message(const QString &msg, MarkdownOverride useMarkdown, bool rainbow
auto related = room->relatedInfo(room->reply());

// Skip reply fallbacks to users who would cause a room ping with the fallback.
// This should be fine, since in some cases the reply fallback can be omitted now and the
// alternative is worse! On Element Android this applies to any substring, but that is their
// bug to fix.
// This should be fine, since in some cases the reply fallback can be omitted now and
// the alternative is worse! On Element Android this applies to any substring, but that
// is their bug to fix.
if (!related.quoted_user.startsWith("@room:")) {
QString body;
bool firstLine = true;
Expand Down