diff --git a/changelog.d/1822.bugfix b/changelog.d/1822.bugfix new file mode 100644 index 000000000..2fb1694b3 --- /dev/null +++ b/changelog.d/1822.bugfix @@ -0,0 +1 @@ +Change format for file uploads, codeblocks, long replies, self-replies and nicknames. Fixes #1521. diff --git a/config.sample.yaml b/config.sample.yaml index ea2fbb686..a3e967003 100644 --- a/config.sample.yaml +++ b/config.sample.yaml @@ -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 commands to the IRC AS bot. @@ -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 diff --git a/config.schema.yml b/config.schema.yml index 74bd389a1..41f988eb8 100644 --- a/config.schema.yml +++ b/config.schema.yml @@ -169,6 +169,8 @@ properties: type: "string" shortReplyTresholdSeconds: type: "integer" + selfReplyTemplate: + type: "string" ignoreFunctionalMembersInAdminRooms: type: "boolean" mediaProxy: diff --git a/spec/integ/matrix-to-irc.spec.js b/spec/integ/matrix-to-irc.spec.js index 5393e0cb5..688e1c416 100644 --- a/spec/integ/matrix-to-irc.spec.js +++ b/spec/integ/matrix-to-irc.spec.js @@ -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", { @@ -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( @@ -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( @@ -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(' "Message #2" <- Message #3'); + expect(text).toEqual('M-friend: "Message #2" <- Message #3'); } ); @@ -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" + @@ -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(); }); @@ -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); @@ -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); @@ -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); diff --git a/spec/unit/BridgedClient.spec.js b/spec/unit/BridgedClient.spec.js index 0c1738ed5..6e7076200 100644 --- a/spec/unit/BridgedClient.spec.js +++ b/spec/unit/BridgedClient.spec.js @@ -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(); diff --git a/src/bridge/MatrixHandler.ts b/src/bridge/MatrixHandler.ts index 4e4616732..b833d6a5a 100644 --- a/src/bridge/MatrixHandler.ts +++ b/src/bridge/MatrixHandler.ts @@ -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. @@ -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, }; @@ -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 { @@ -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: { @@ -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; @@ -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 { diff --git a/src/irc/BridgedClient.ts b/src/irc/BridgedClient.ts index 932bceb47..8e085c43e 100644 --- a/src/irc/BridgedClient.ts +++ b/src/irc/BridgedClient.ts @@ -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) { diff --git a/src/models/IrcAction.ts b/src/models/IrcAction.ts index 3768eb5e8..d4e3749c9 100644 --- a/src/models/IrcAction.ts +++ b/src/models/IrcAction.ts @@ -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; diff --git a/src/models/MatrixAction.ts b/src/models/MatrixAction.ts index 6f211b45d..50ec3ce60 100644 --- a/src/models/MatrixAction.ts +++ b/src/models/MatrixAction.ts @@ -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}`; } } }