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

Various formatting adjustments #1822

Open
wants to merge 9 commits into
base: develop
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
1 change: 1 addition & 0 deletions changelog.d/1822.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Change format for file uploads, codeblocks, long replies, self-replies and nicknames. Fixes #1521.
6 changes: 4 additions & 2 deletions config.sample.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -332,7 +332,7 @@ ircService:
ircClients:
# The template to apply to every IRC client nick. This MUST have either
# $DISPLAY or $USERID or $LOCALPART somewhere in it.
# Optional. Default: "M-$DISPLAY". Example: "M-Alice".
# Optional. Default: "$DISPLAY[m]". Example: "Alice[m]".
nickTemplate: "$DISPLAY[m]"
# True to allow virtual IRC clients to change their nick on this server
# by issuing !nick <server> <nick> commands to the IRC AS bot.
Expand Down Expand Up @@ -594,7 +594,9 @@ ircService:
# format of replies sent shortly after the original message
shortReplyTemplate: "$NICK: $REPLY"
# format of replies sent a while after the original message
longReplyTemplate: "<$NICK> \"$ORIGINAL\" <- $REPLY"
longReplyTemplate: "$NICK: \"$ORIGINAL\" <- $REPLY"
# format of replies where the sender of the original message is the same as the sender of the reply
selfReplyTemplate: "<$NICK> $ORIGINAL\n$REPLY"
# how much time needs to pass between the reply and the original message to switch to the long format
shortReplyTresholdSeconds: 300
# Ignore users mentioned in a io.element.functional_members state event when checking admin room membership
Expand Down
2 changes: 2 additions & 0 deletions config.schema.yml
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,8 @@ properties:
type: "string"
shortReplyTresholdSeconds:
type: "integer"
selfReplyTemplate:
type: "string"
ignoreFunctionalMembersInAdminRooms:
type: "boolean"
mediaProxy:
Expand Down
69 changes: 57 additions & 12 deletions spec/integ/matrix-to-irc.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -234,6 +234,52 @@ describe("Matrix-to-IRC message bridging", function() {
});
});

it("should bridge matrix replies to self as self-replies", async () => {
// Trigger an original event
await env.mockAppService._trigger("type:m.room.message", {
content: {
body: "This is the real message",
msgtype: "m.text"
},
room_id: roomMapping.roomId,
sender: repliesUser.id,
event_id: "$original:bar.com",
origin_server_ts: Date.now(),
type: "m.room.message"
});
const p = env.ircMock._whenClient(roomMapping.server, repliesUser.nick, "say",
(client, channel, text) => {
expect(client.nick).toEqual(repliesUser.nick);
expect(client.addr).toEqual(roomMapping.server);
expect(channel).toEqual(roomMapping.channel);
expect(text).toEqual(`<${repliesUser.nick}> This is the real message\nReply Text`);
}
);
const formatted_body = constructHTMLReply(
"This is the fake message",
"@somedude:bar.com",
"Reply text"
);
await env.mockAppService._trigger("type:m.room.message", {
content: {
body: "> <@somedude:bar.com> This is the fake message\n\nReply Text",
formatted_body,
format: "org.matrix.custom.html",
msgtype: "m.text",
"m.relates_to": {
"m.in_reply_to": {
"event_id": "$original:bar.com"
}
},
},
sender: repliesUser.id,
room_id: roomMapping.roomId,
origin_server_ts: Date.now(),
type: "m.room.message"
});
await p;
});

it("should bridge rapid matrix replies as short replies", async () => {
// Trigger an original event
await env.mockAppService._trigger("type:m.room.message", {
Expand Down Expand Up @@ -298,7 +344,7 @@ describe("Matrix-to-IRC message bridging", function() {
expect(client.nick).toEqual(testUser.nick);
expect(client.addr).toEqual(roomMapping.server);
expect(channel).toEqual(roomMapping.channel);
expect(text).toEqual(`<${repliesUser.nick}> "This is the real message" <- Reply Text`);
expect(text).toEqual(`${repliesUser.nick}: "This is the real message" <- Reply Text`);
}
);
const formatted_body = constructHTMLReply(
Expand Down Expand Up @@ -389,7 +435,7 @@ describe("Matrix-to-IRC message bridging", function() {
expect(client.nick).toEqual(testUser.nick);
expect(client.addr).toEqual(roomMapping.server);
expect(channel).toEqual(roomMapping.channel);
expect(text).toEqual(`<${repliesUser.nick}> "This..." <- Reply Text`);
expect(text).toEqual(`${repliesUser.nick}: "This..." <- Reply Text`);
}
);
const formatted_body = constructHTMLReply(
Expand Down Expand Up @@ -499,7 +545,7 @@ describe("Matrix-to-IRC message bridging", function() {
expect(client.nick).toEqual(testUser.nick);
expect(client.addr).toEqual(roomMapping.server);
expect(channel).toEqual(roomMapping.channel);
expect(text).toEqual('<M-friend> "Message #2" <- Message #3');
expect(text).toEqual('M-friend: "Message #2" <- Message #3');
}
);

Expand Down Expand Up @@ -650,7 +696,7 @@ describe("Matrix-to-IRC message bridging", function() {
});
});

it("should bridge mutliline code blocks as IRC action with URL", function(done) {
it("should bridge mutliline code blocks as a URL", function(done) {
let tBody =
"```javascript\n" +
" expect(text.indexOf(\"javascript\")).not.toEqual(-1);\n" +
Expand All @@ -662,13 +708,12 @@ describe("Matrix-to-IRC message bridging", function() {
const sdk = env.clientMock._client(config._botUserId);
sdk.uploadContent.and.returnValue(Promise.resolve("mxc://deadbeefcafe"));

env.ircMock._whenClient(roomMapping.server, testUser.nick, "action", (client, channel, text) => {
env.ircMock._whenClient(roomMapping.server, testUser.nick, "say", (client, channel, text) => {
expect(client.nick).toEqual(testUser.nick);
expect(client.addr).toEqual(roomMapping.server);
expect(channel).toEqual(roomMapping.channel);
// don't be too brittle when checking this, but I expect to see the
// code type and the media proxy url
expect(text.indexOf('javascript')).not.toEqual(-1);
expect(text.includes(config.ircService.mediaProxy.publicUrl)).toEqual(true);
done();
});
Expand Down Expand Up @@ -713,11 +758,11 @@ describe("Matrix-to-IRC message bridging", function() {
});
});

it("should bridge matrix images as IRC action with a URL", function(done) {
it("should bridge matrix images as a URL", function(done) {
const tBody = "the_image.jpg";
const tMxcSegment = "/somecontentid";

env.ircMock._whenClient(roomMapping.server, testUser.nick, "action", (client, channel, text) => {
env.ircMock._whenClient(roomMapping.server, testUser.nick, "say", (client, channel, text) => {
expect(client.nick).toEqual(testUser.nick);
expect(client.addr).toEqual(roomMapping.server);
expect(channel).toEqual(roomMapping.channel);
Expand All @@ -737,11 +782,11 @@ describe("Matrix-to-IRC message bridging", function() {
});
});

it("should bridge matrix files as IRC action with a URL", function(done) {
it("should bridge matrix files as a URL", function(done) {
const tBody = "a_file.apk";
const tMxcSegment = "/somecontentid";

env.ircMock._whenClient(roomMapping.server, testUser.nick, "action", (client, channel, text) => {
env.ircMock._whenClient(roomMapping.server, testUser.nick, "say", (client, channel, text) => {
expect(client.nick).toEqual(testUser.nick);
expect(client.addr).toEqual(roomMapping.server);
expect(channel).toEqual(roomMapping.channel);
Expand Down Expand Up @@ -1074,11 +1119,11 @@ describe("Matrix-to-IRC message bridging with media URL and drop time", function
expect(said).toBe(true);
});

it("should bridge matrix files as IRC action with a configured media URL", function(done) {
it("should bridge matrix files as IRC message with a configured media URL", function(done) {
let tBody = "a_file.apk";
let tMxcSegment = "/somecontentid";

env.ircMock._whenClient(roomMapping.server, testUser.nick, "action",
env.ircMock._whenClient(roomMapping.server, testUser.nick, "say",
function(client, channel, text) {
expect(client.nick).toEqual(testUser.nick);
expect(client.addr).toEqual(roomMapping.server);
Expand Down
4 changes: 2 additions & 2 deletions spec/unit/BridgedClient.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@ describe("BridgedClient", function() {
expect(BridgedClient.getValidNick("f+/\u3052oobar", false, STATE_DISC)).toBe("foobar");
});
it("will ensure nicks start with a letter or special character", function() {
expect(BridgedClient.getValidNick("-foobar", false, STATE_DISC)).toBe("M-foobar");
expect(BridgedClient.getValidNick("12345", false, STATE_DISC)).toBe("M12345");
expect(BridgedClient.getValidNick("-foobar", false, STATE_DISC)).toBe("`-foobar");
expect(BridgedClient.getValidNick("12345", false, STATE_DISC)).toBe("`12345");
});
it("will throw if the nick is invalid", function() {
expect(() => BridgedClient.getValidNick("f+/\u3052oobar", true, STATE_DISC)).toThrowError();
Expand Down
20 changes: 13 additions & 7 deletions src/bridge/MatrixHandler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,8 @@ export interface MatrixHandlerConfig {
shortReplyTemplate: string;
// Format of replies sent a while after the original message
longReplyTemplate: string;
// format of replies where the sender of the original message is the same as the sender of the reply
selfReplyTemplate: string;
// Format of the text explaining why a message is truncated and pastebinned
truncatedMessageTemplate: string;
// Ignore io.element.functional_members members joining admin rooms.
Expand All @@ -67,7 +69,8 @@ export const DEFAULTS: MatrixHandlerConfig = {
replySourceMaxLength: 32,
shortReplyTresholdSeconds: 5 * 60,
shortReplyTemplate: "$NICK: $REPLY",
longReplyTemplate: "<$NICK> \"$ORIGINAL\" <- $REPLY",
longReplyTemplate: "$NICK: \"$ORIGINAL\" <- $REPLY",
selfReplyTemplate: "<$NICK> $ORIGINAL\n$REPLY",
truncatedMessageTemplate: "(full message at <$URL>)",
ignoreFunctionalMembersInAdminRooms: false,
};
Expand Down Expand Up @@ -1173,10 +1176,9 @@ export class MatrixHandler {
// we check event.content.body since ircAction already has the markers stripped
const codeBlockMatch = event.content.body.match(/^```(\w+)?/);
if (codeBlockMatch) {
const type = codeBlockMatch[1] ? ` ${codeBlockMatch[1]}` : '';
event.content = {
msgtype: "m.emote",
body: `sent a${type} code block: ${httpUrl}`
...event.content,
body: `${httpUrl}`
};
}
else {
Expand Down Expand Up @@ -1207,7 +1209,7 @@ export class MatrixHandler {
// Modify the event to become a truncated version of the original
// the truncation limits the number of lines sent to lineLimit.

const msg = '\n...(truncated)';
const msg = '\n(truncated)';

const sendingEvent: MatrixMessageEvent = { ...event,
content: {
Expand Down Expand Up @@ -1297,7 +1299,7 @@ export class MatrixHandler {
const bridgeIntent = this.ircBridge.getAppServiceBridge().getIntent();
// strips out the quotation of the original message, if needed
const replyText = (body: string): string => {
const REPLY_REGEX = /> <(.*?)>(.*?)\n\n([\s\S]*)/;
const REPLY_REGEX = /> <(.*?)>(.*?)\n\n([\s\S]*)/s;
const match = REPLY_REGEX.exec(body);
if (match === null || match.length !== 4) {
return body;
Expand Down Expand Up @@ -1392,7 +1394,11 @@ export class MatrixHandler {

let replyTemplate: string;
const thresholdMs = (this.config.shortReplyTresholdSeconds) * 1000;
if (rplSource && event.origin_server_ts - cachedEvent.timestamp > thresholdMs) {
if (cachedEvent.sender === event.sender) {
// They're replying to their own message.
replyTemplate = this.config.selfReplyTemplate;
}
else if (rplSource && event.origin_server_ts - cachedEvent.timestamp > thresholdMs) {
replyTemplate = this.config.longReplyTemplate;
}
else {
Expand Down
4 changes: 2 additions & 2 deletions src/irc/BridgedClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -738,9 +738,9 @@ export class BridgedClient extends EventEmitter {
`Nick '${nick}' must start with a letter or special character (dash is not a special character).`
);
}
// Add arbitrary letter prefix. This is important for guest user
// Add arbitrary prefix. This is important for guest user
// IDs which are all numbers.
n = "M" + n;
n = "`" + n;
}

if (state.status === BridgedClientStatus.CONNECTED) {
Expand Down
8 changes: 4 additions & 4 deletions src/models/IrcAction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,18 +54,18 @@ export class IrcAction {
return new IrcAction(matrixAction.type, matrixAction.text, matrixAction.ts);
case "image":
return new IrcAction(
"emote", "uploaded an image: " + matrixAction.text, matrixAction.ts
"message", "" + matrixAction.text, matrixAction.ts
);
case "video":
return new IrcAction(
"emote", "uploaded a video: " + matrixAction.text, matrixAction.ts
"message", "" + matrixAction.text, matrixAction.ts
);
case "audio":
return new IrcAction(
"emote", "uploaded an audio file: " + matrixAction.text, matrixAction.ts
"message", "" + matrixAction.text, matrixAction.ts
);
case "file":
return new IrcAction("emote", "posted a file: " + matrixAction.text, matrixAction.ts);
return new IrcAction("message", "" + matrixAction.text, matrixAction.ts);
case "topic":
if (matrixAction.text === null) {
break;
Expand Down
4 changes: 2 additions & 2 deletions src/models/MatrixAction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -220,12 +220,12 @@ export class MatrixAction {
}

if (filename) {
text = `${fileSize} < ${url} >`;
text = `${url} ${fileSize}`;
}
else {
fileSize = fileSize ? ` ${fileSize}` : "";
// If not a filename, print the body
text = `${event.content.body}${fileSize} < ${url} >`;
text = `${url} ${event.content.body}${fileSize}`;
}
}
}
Expand Down
Loading