Add partial support for formatting incoming HTML #75
Conversation
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 torhve#60.
Also note in TODO comment that contiguous tags are currently broken.
| if x:sub(m, m) == y:sub(n, n) then | ||
| m = m - 1 | ||
| n = n - 1 | ||
| elseif n > start and (m == start or memo[m][n-1] > memo[m-1][n]) then |
There was a problem hiding this comment.
I'm getting:
Error in http_cb: …/matrix.lua:335: attempt to index field '?' (a nil value)
stack traceback:
…/matrix.lua:928: in function <…/matrix.lua:924>
There was a problem hiding this comment.
I can't reproduce this. Do you know what input triggers this?
The most common formatting this allows is text with both italics and bold, but it fixes a very large amount of corner cases.
|
This look terribly complex for just a string substitution. What am I missing? |
|
@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. |
|
Do you think we should merge this currently? How well does it work in practice? |
|
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, 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. |
|
maybe you could provide an example so that everyone can reproduce? |
|
Sorry, I haven’t got time to debug it. I just enabled the plugin, and it crashed almost immediately. |
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.