Bridge Matrix threads #1503
Bridge Matrix threads #1503
Conversation
| @@ -126,6 +129,9 @@ export class MatrixHandler { | |||
| // required because invites are processed asyncly, so you could get invite->msg | |||
| // and the message is processed before the room is created. | |||
| private readonly eventCache: Map<string, CachedEvent> = new Map(); | |||
| // keep track of the last message sent to each thread | |||
| // so that we can form replies to the most recent one rather than the first one | |||
| private lastMessageInThreadCache: Map<string, string> = new Map(); | |||
There was a problem hiding this comment.
Do we want to make this a LRU to avoid this growing ever larger, or nah?
| let lastReplyId = this.lastMessageInThreadCache.get(threadId); | ||
| if (!lastReplyId) { | ||
| try { | ||
| const getThread = async (from?: string) => this.ircBridge.getAppServiceBridge().getIntent().matrixClient.doRequest( |
There was a problem hiding this comment.
Might be worth chucking a PR at the bot sdk too, even if it will come too late for this PR.
| let from: string|undefined; | ||
| // locate the message we're now formatting in the thread chain and find its predecessor | ||
| for (let i = 0; i < this.config.threadLookupRequestLimit; i++) { | ||
| let thread = await getThread(from); | ||
|
|
||
| const currentMessageIdx = thread['chunk'].findIndex((ev: any) => ev.event_id = event.event_id); | ||
| if (currentMessageIdx == -1) { | ||
| from = thread.next_batch; | ||
| if (!from) break; | ||
| continue; | ||
| } else { | ||
| if (currentMessageIdx === thread['chunk'].length) { | ||
| // we're at a chunk boundary, need to load one more | ||
| thread = await getThread(thread.next_batch); | ||
| lastReplyId = thread['chunk'][0].event_id; | ||
| } else { | ||
| lastReplyId = thread['chunk'][currentMessageIdx + 1].event_id; | ||
| } | ||
| break; | ||
| } | ||
| } | ||
| } catch (err) { | ||
| console.error('Error fetching thread from homeserver:', err); | ||
| } | ||
| lastReplyId ||= threadId; // fallback to the original message if we really have no better idea |
There was a problem hiding this comment.
Personal feeling is this logic feels too complex to sit in this function. Should at least be it's own function, if not in the bot/bridge sdk itself.
This is what it goes down to, yeah. If it'll end up the giving the false impression that we can transparently bridge threads, and yet we can't bridge replies from IRC to Matrix it'll just become a mess. Perhaps it'd make more sense to do channels-as-threads on the IRC side? |
Wouldn't it be best to just support IRCv3 reply client tag as per #1414? |
I realise it's a bit chicken and egg, but atm only IRCCloud supports it...so we're unlikely to do the work to support it until Server/Client impls catch up. |
It's also supported by Limnoria and Palaver has expressed interest towards implementing it. On servers at least Ergo has it and I would think InspIRCd to at least have it planned if not implemented already. |
Servers don't need to implement it. It's a client-tag, and servers just pass all client tags without explicitly implementing them. (Though UnrealIRCd has a whitelist, but it already has the tag on its whitelist.) |
Mm, though client-tags is still lagging behind a bit as well (solanum-ircd/solanum#167). Granted, we could probably implement given enough servers do support it. |
|
I am under impresson that Solanum is going to get a rewrite at some point and is not going to implement a lot of IRCv3 features before it happens, so I wouldn't include them in considerations how widely supported something is. |
This treats each thread reply as an old message reply, as in: always quotes the previous message from a thread, regardless of how long ago it happened – with the possibility of multiple threads going on concurrently, short replies just turn into a mess.
It is quite noisy, but I think it's the best we can do to preserve the context. Replies from IRC won't work, obviously :(