search

ayn2op / discordo

A lightweight, secure, and feature-rich Discord terminal client.
MIT License
2.25k stars 70 forks source link

Individual message rendering overhaul #451

Open Retropaint opened 1 month ago

Retropaint commented 1 month ago

Closes #444

This is quite a hefty overhaul to how messages are rendered. Messages are now individual textViews rendered in a regular box, as opposed to one big textView. The benefits of this are described in the issue.

However, there is a concern I have for my implementation of this, which is the custom scrolling logic. Since tview boxes don't support scrolling, this has to be manually done. My implementation is very verbose but hopefully simple to understand and maintain. I had already tried a more 'compact' solution as seen in older commits but it was fragile in every sense of the word.

Note that this PR is strictly for the overhaul itself (made in the 'easiest' way possible) and doesn't have performance optimization. Attempting it was a time sink which I don't want to get into for now. I may add easy ones to implement into the PR if I find them.

Not sure if this is necessary to add, but I welcome any feedback on this system, particularly the scrolling logic (which I think can still be improved, I've just run out of ideas and time).

Retropaint commented 1 month ago

Marked as draft as I hadn't yet fixed detecting text line counts for those with words longer than the box width.

Retropaint commented 1 month ago

Scrolling logic has improved, not as verbose and still just as stable. Also some optimization with not calling Render on off-screen messages and caching line counts (will need re-calculating for edited messages but thats for another time).

Retropaint commented 1 month ago

There are a few line count padding issues to fix with code blocks but it requires changing renderer.go, and I don't want to add more files changed to this PR lest it have more conflicts down the line.

@ayn2op Please let me know what this PR needs before it can be merged. If it will take a while to review that's fine, just as long as it's merged or closed at some point.

Retropaint commented 3 weeks ago

noticed a bug where messages won't render on their own without any input, since messagesText is now a Box and doesn't get implicitly called to redraw. Will fix a bit later.