Rich text simplification #412
Conversation
Having the choice explicit makes the code easier to read.
It did not help, just added to the indent.
This was doing double regex matching, calling parse_text was not needed at all.
This is the first step towards easier stubbing
Remove has_urwidgets since we no longer need to worry if we have urwidgets in the richtext module.
|
@danschwarz Can you tell me what's the point of urlencode_url? I tried removing the call to it and encoding urls still worked well. |
|
Hi, thanks for the detailed review and changes. I believe that the issue here was that we need our URL to be urlencoded. But when we were presented with an already-urlencoded URL, we'd urlencode it again, and that'd corrupt it. It's harmless to urldecode an unencoded URL so adding this decode/encode step ensures that our URL is urlencoded once and only once. I should've added a test case for this when I wrote it; I'll try to do so later. |
|
Your PR looks good to me overall, despite the test failure (it can't import the 'urwidgets' library for some reason, even though we satisfy the Python >= 3.7 requirement.) Not sure how to fix that. I see that you've addressed the situation where we don't have urwidgets at runtime by falling back to the old renderer. This eliminates the mess of stub libraries that I added. I'm OK with that tradeoff. |
|
Fixed it to build properly by pulling in the required "extra", which in turn pulls in urwidgets |
Hi! Sorry it took me so long. I found your pull request a bit too complex, so did some work to simplify it to something that's easier to follow. Please check out the commits where I explained briefly what I did.
In general:
I did test the result a bit on mastodon 4.2.0-glitch which I installed locally. I think I didn't break anything.