matrix-org / gomatrix

A Golang Matrix client
Apache License 2.0
285 stars 53 forks source link

Retire `HTMLMessage` #83

Open TheDiscordian opened 4 years ago

TheDiscordian commented 4 years ago

I just discovered HTMLMessage, and it's not clear why it exists in this form to me. I discovered it by looking at #58, and noticed that my PR #82 seems to more naturally integrate. I don't agree with automatically shaping the HTML (what if a user sends <span> with formatting? Now you have something like <span><span></span>, what part's the body? Admittingly, I don't follow the regex there, though), I don't even agree simply with calling it HTML in a public type as it's merely a subset of HTML, it's just formatted text. Rather I think we should make it clear it's a subset, and link to where the differences are found.

I don't think someone receiving or sending formatted text using this library is going to commonly think to look for HTMLMessage, which doesn't seem to be connected to anything, just an oddity. I think SendFormattedText, linked to the spec where it was inspired (what my PR effectively is) is a lot more natural, and fits nicely along-side SendText, we can also re-use the struct there, as that part of the spec was simply extended, we can simply extend the struct, without breaking backward compatibility (unless someone didn't name their fields while declaring, which is considered bad practice anyways).

I think we should have something more natural, and I say this because it is still just text lol. It's all grouped together in the spec, why split it up in the library? :)

In short: HTMLMessage is very confusing as-is, and seems to break the library's flow. Let's look at the intent behind it, and implement it better.