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

Add HTML parser for Matrix messages #17

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

Friskygote
Copy link
Contributor

Sometimes Matrix messages are more complex and are expressed with HTML markup. This introduces some challenges with bridging such messages to Matrix. Things like spoilers, lists, headers, code blocks should be expressed properly on Discord side as well. Projects that focus on converting messages back and fourth between two platforms already exist but I haven't found similar converter for Python.

Worth noting that Use embeds on Discord side for replies. is on TODO list for the project, however this PR still uses non embed way of replies. Yet with changes in this PR I think it should be easier to complete that TODO in the future (even though it is not my goal for this PR to accomplish that).

This PR does not introduce any new external dependencies.

Currently PR is in draft mode until I finish working with it completely (most notably, emoji conversion should be done, also, polishing and bug squashing). Wanted to create this PR before I finish to extend the time for discussion over this PR.
I'm personally running modified branch with those changes on my own server however additional testing and feedback is appreciated.

@lgtm-com
Copy link

lgtm-com bot commented Feb 1, 2022

This pull request introduces 2 alerts when merging 1512dc5 into 79308b6 - view on LGTM.com

new alerts:

  • 2 for Unused import

The one thing we need now is just better over-the-limit message protection/improvement which I'm not yet sure how to do
@git-bruh
Copy link
Owner

git-bruh commented Feb 8, 2022

Hi, thanks a lot for the PR! I'm a bit occupied with school stuff for a couple of weeks so won't be able to review this until then.

I've been holding off from writing unit tests for this project due to laziness, but such a component definitely needs some. I'll write them for the PR once I review it, and hopefully write some for the other components as well.

@Friskygote
Copy link
Contributor Author

Friskygote commented Feb 8, 2022

I believe I have now finished the most difficult part of writing code, now I'll continue testing this PR on my own instance, however the effects and formatting done is visible on images below. Big thanks go to @Markus-Rost for huge help testing it and giving me feedback on this PR.
(noticed that masked link example isn't visible, that's due to a faulty HTML I've fed to Matrix, not code)
2022-02-08_13-47
2022-02-08_13-47_1

@Friskygote
Copy link
Contributor Author

Hi, thanks a lot for the PR! I'm a bit occupied with school stuff for a couple of weeks so won't be able to review this until then.

I've been holding off from writing unit tests for this project due to laziness, but such a component definitely needs some. I'll write them for the PR once I review it, and hopefully write some for the other components as well.

That's fine, please take care of your school stuff first :) I don't yet feel comfortable merging this PR anyways because I barely finished writing the code for it and there are probably still some nasty issues there and there I'd need to take care of.
When I feel like it's good enough I'll remove draft state from it. But whoever uses the bridge and wants to test I invite you to help out!

@Friskygote
Copy link
Contributor Author

Hey, so I know it took a while for me to get motivation. I consider the majority of the work finished. I think there are still some bugs with links in replies however I cannot find the messages which caused issues in the past so I can't remember how to recreate that issue. I'll undraft this PR because for the entire time I've ran bridge with this PR we didn't have any significant issues and I don't know whether I'll actually have the time to work on it in the future due to private matters.
I'll still be happy to apply changes in the near future if something comes up in review/during testing.

@Friskygote Friskygote marked this pull request as ready for review April 29, 2022 12:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants