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

Matrix bridge #46

Merged
merged 19 commits into from
Mar 26, 2024
Merged

Matrix bridge #46

merged 19 commits into from
Mar 26, 2024

Conversation

austinhuang0131
Copy link
Contributor

@austinhuang0131 austinhuang0131 commented Feb 3, 2024

gotta get it out of proof of concept

  • to matrix
    • create
    • update
    • delete
    • profile
  • from matrix
    • create
    • update
    • delete
    • profile
  • formats
    • text
    • attachments
    • embed (to matrix only; convert to html)
  • on joining matrix room, check if either the room is public or the bot has invite permission

* fix name
* fix localpart reservation and request
* add appserviceUrl option (a web server where Matrix homeserver should send messages to)
* remove encryption
@williamhorning williamhorning marked this pull request as ready for review February 3, 2024 23:53
@austinhuang0131 austinhuang0131 marked this pull request as draft February 3, 2024 23:56
Copy link
Owner

@williamhorning williamhorning left a comment

Choose a reason for hiding this comment

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

So far, this all looks pretty good, and I really appreciate the fact that someone's actually taking a look at Matrix support. There're a few nitpicky things I've got but overall this is great!

@@ -31,23 +31,6 @@ export default class MatrixPlugin extends BoltPlugin {
homeserverUrl: this.config.homeserverUrl,
domain: this.config.domain,
registration: this.config.reg_path,
bridgeEncryption: {
Copy link
Owner

Choose a reason for hiding this comment

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

Is there any reason to remove all the encryption code? I'd prefer it if bolt-matrix supported encrypted rooms but if there's a reason I guess that's fine

Copy link
Contributor Author

Choose a reason for hiding this comment

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

because it doesn't launch on my end with it, i might revisit it in the future, but almost all matrix-appservice-${platform} don't have encryption

Copy link
Owner

Choose a reason for hiding this comment

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

ah okay, i guess we could go back to that in the future then

packages/bolt-matrix/events.ts Outdated Show resolved Hide resolved
packages/bolt-matrix/events.ts Outdated Show resolved Hide resolved
packages/bolt-matrix/events.ts Outdated Show resolved Hide resolved
packages/bolt-matrix/events.ts Outdated Show resolved Hide resolved
packages/bolt-matrix/mod.ts Show resolved Hide resolved
packages/bolt-matrix/mod.ts Show resolved Hide resolved
packages/bolt-matrix/mod.ts Outdated Show resolved Hide resolved
Copy link
Owner

@williamhorning williamhorning left a comment

Choose a reason for hiding this comment

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

this all looks pretty nice so far but could you make a new PR with the changes to bolt-revolt and bolt/lib? i want to avoid future merge conflicts if possible and that’d really help

packages/bolt-matrix/deps.ts Outdated Show resolved Hide resolved
@austinhuang0131
Copy link
Contributor Author

remind me to cherrypick later

@austinhuang0131
Copy link
Contributor Author

fair warning: i will have to let coreToMessage return a list of messages because matrix doesn't allow more than 1 attachments per message, or attachments with text

@williamhorning
Copy link
Owner

williamhorning commented Feb 7, 2024

fair warning: i will have to let coreToMessage return a list of messages because matrix doesn't allow more than 1 attachments per message, or attachments with text

that should be fine and isn't out of the ordinary, bolt-guilded does that to allow for file uploads (at least on the 0.4.x-deno branch). also, you can split those functions into their own file if you want to

remind me to cherrypick later

cherry pick the stuff outside of bolt-matrix pls

Copy link
Owner

@williamhorning williamhorning left a comment

Choose a reason for hiding this comment

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

just a few other things i noticed when looking at this.

also, when you get the chance, could you please cherry pick your changes to bolt-revolt and bolt/lib?

.gitignore Outdated
@@ -2,3 +2,4 @@
/config
/config.ts
/docker-compose.yml
db
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
db
/db

): Promise<BoltMessage<WeakEvent>> {
const sender = await intent.getProfileInfo(event.sender);
return {
author: {
username: sender.displayname || event.sender,
rawname: event.sender,
id: event.sender,
profile: sender.avatar_url
profile: `${sender.avatar_url.replace("mxc://", `${homeserverUrl}/_matrix/media/v3/thumbnail/`)}?width=96&height=96&method=scale`
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
profile: `${sender.avatar_url.replace("mxc://", `${homeserverUrl}/_matrix/media/v3/thumbnail/`)}?width=96&height=96&method=scale`
profile: `${sender.avatar_url.replace("mxc://", `${homeserverUrl}/_matrix/media/v3/thumbnail/`)}?width=96&height=96&method=scale`,
color: '#0DBD8B'

@williamhorning
Copy link
Owner

jsyk 0.5.5 was merged (see #44) so there're a few merge conflicts and public api changed but i can fix those for you if you'd like

@williamhorning
Copy link
Owner

see austinhuang0131#1

@williamhorning williamhorning mentioned this pull request Mar 14, 2024
@williamhorning williamhorning linked an issue Mar 14, 2024 that may be closed by this pull request
@williamhorning williamhorning merged commit fe91041 into williamhorning:main Mar 26, 2024
@williamhorning
Copy link
Owner

thats not quite what i wanted to do, whoops

@williamhorning
Copy link
Owner

i couldn't actually redo this in a sane way so i guess ill try to find a way to implement embeds and stuff or you could make a pr

@austinhuang0131
Copy link
Contributor Author

austinhuang0131 commented Mar 26, 2024

to matrix

in principle embeds is just converting to some html that looks similar. for allowed tags see https://spec.matrix.org/v1.10/client-server-api/#mroommessage-msgtypes

attachments would be more complicated since it requires uploading first, then attaching the mxc to the message. for custom emotes it'd also need to work the same way, but you'll need to use some db to store emoji-to-mxc mappings.

from matrix

html? good luck converting that

images? use the download url on the homeserver

@r3k2
Copy link

r3k2 commented Mar 26, 2024

I think when you merge by mistake you closed the tied issues :) I saw the email and was all happy! Only to come here and see it was a mistake :D :D cheers.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Matrix
3 participants