weechat-matrix-protocol-script icon indicating copy to clipboard operation
weechat-matrix-protocol-script copied to clipboard

Add partial support for formatting incoming HTML

Open her001 opened this issue 8 years ago • 7 comments

Enable the html_formatting var to use this. Currently supports bold, italics, and underlines with weechat attributes. Code, strikethroughs, quotes, and breaks/rules are supported with plain text decorating them.

It would be reasonable to add support for colors and lists in the future. Unfortunately, some things will never be possible with the limitations of Weechat, but there may be ways to approximate them better. All unsupported HTML tags are simply stripped from the message.

This mostly fixes #60.

her001 avatar May 23 '17 06:05 her001

This look terribly complex for just a string substitution. What am I missing?

erdnaxeli avatar Oct 18 '17 19:10 erdnaxeli

@erdnaxeli I adapted a longest common subsequence algorithm to find the difference between the html and unformatted messages, which extracts just the html tags without needing to do full parsing. Should be faster in many cases while avoiding adding a parsing library just for the limited set of html that matrix clients support and can be displayed nicely in weechat.

...which I really should have explained in a commit message. I'll probably edit the first commit before this is merged.

her001 avatar Oct 19 '17 15:10 her001

Do you think we should merge this currently? How well does it work in practice?

torhve avatar May 11 '18 09:05 torhve

In practice, a terminal can only emulate so much of standard html rendering, but it works completely fine (for me) for the most common cases of bold and italics. Support for lists may be the most urgent feature missing from this. Colors shouldn't be too hard to implement, but are rarely used in practice.

In my opinion, it is okay to merge as the feature is off by default and described as experimental. This is fairly low risk.

I was never able to identify or reproduce the issue that @andrewshadura reported in the review, so I cannot comment on that.

her001 avatar May 11 '18 14:05 her001

@her001, this was happening to me all the time. Even if you cannot trigger it that doesn’t mean the code is correct, and it is definitely not okay to merge it as is.

andrewshadura avatar May 13 '18 06:05 andrewshadura

maybe you could provide an example so that everyone can reproduce?

ptman avatar May 13 '18 08:05 ptman

Sorry, I haven’t got time to debug it. I just enabled the plugin, and it crashed almost immediately.

andrewshadura avatar May 13 '18 08:05 andrewshadura