Update embedding Reddit preview links to include captions and support i.redd.it embeds - #92
Conversation
… more in line with gallery posts
|
Sorry for the constant fixes I swear I test this stuff 😅 |
| if !image_text.is_empty() { | ||
| image_text.remove(0); | ||
| image_text.pop(); | ||
| image_text.pop(); | ||
| image_text.pop(); | ||
| image_text.pop(); | ||
| } |
There was a problem hiding this comment.
Not sure exactly what this does but it should probably operate on a substring like image_text[1..image_text.len()-4] - if what you intend to do is remove the last 4 and first. Curious why this is done like this - can you think of a better way to do this?
There was a problem hiding this comment.
The first character of image_text is a >, and could stay, but it would require readding the .replace(">>", ">") to image_to_replace's assignment since image_url also contains a > at the end of it meaning we end up with >> in between {image_url} and {image_text} instead of just a single >. I figured just removing the first character is better than theoretically having the potential to replace an intentional >> somewhere else inside of image_text and having the replacement fail.
The last four need to be removed since they'll be an </a>, which we don't want appearing inside of the <figcaption> block. Although now that I think about it that can just be done within the if block since it isn't needed otherwise.
I'll modify it to use the substring instead and only drop the </a> when needed.
There was a problem hiding this comment.
Just made it use the substring as well as renaming image_text to image_caption and adding comments to make thing's functions and usages clearer.
I didn't make it so that the </a> is only dropped when image_caption is actually used to add a caption to the image as it seemed like an unnecessary extra step when it can be done at the same time as removing the first character.
However I did make it so that the quotes inside image_caption are only replaced when it's actually used as a caption since it wasn't needed otherwise.
…_caption to better reflect its usage, only replace quotes in image_caption when needed, and add comments for what some of the code does
|
how do i go about and run this locally (https://github.com/ButteredCats/redlib/tree/fix_preview_captions) |
|
You can run: git clone https://github.com/ButteredCats/redlib
cd redlib/
git checkout fix_preview_captions
cargo runto test it out. |
…text is inside them with the image, update test to reflect change in image replacing
|
Just ran into an issue with /r/australian/comments/1c3jn4x/australia_right_now/ where image_caption would be empty, and Redlib would panic. Now it checks if it's empty before trying to mess with it. Also problems with specifically this comment /r/australian/comments/1c3jn4x/australia_right_now/kzioqh7/?context=3 where there's text inside of the paragraph before the image link, which is something I haven't seen till now. It would fail to embed because image_to_replace doesn't account for this. It seems Reddit refuses to embed things like that as well. I updated the image replacement not to replace the |
|
It looks like there might still be issues with this getting stuck in an infinite loop for some reason. I've been running this branch in order to test and a couple times over the past two weeks I've had my instance die after Redlib starts using a lot of CPU and RAM and stops responding. I'll look into it and comment back with what I find. |
|
Not sure what's up with the failed warnings check, I can't recreate those on my machine and they also don't seem to relate to anything I was touching? Anyways, turns out the infinite loop had nothing to do with the image replacements themselves, but was instead caused by things like $0, $1, $2, etc to be interpreted as a regex capture group when doing the replace in I opted to basically just make it ignore it by directly using a capture group, |
…g only an image doesn't get a margin
|
After updating rust I see those warnings on my machine, however they happen on the main branch which was previously fine so I assume this is just the rust update itself. I've added a little prettying for embed only comments, reducing weird gaps they had before. I've also added support for embedding |
ButteredCats
changed the title
|
Thanks! |
3 participants
Fixes the caption part of #91
Also makes the embedded images lazy load as a bonus.
Using the post in #91 it was kind of hard to tell what was a caption and what wasn't, but it was very clear in something like /r/test/comments/d9hta9/test_multiple_images/. Maybe the captions need tweaking CSS wise to stand out some more but I wasn't sure what the best way to do that was.
The code itself also isn't pretty. Especially removing the extra stuff from the image's caption. I couldn't find a better way to do that or to get the variable in scope for the if else where it determines whether it'll have a figcaption block or not. If there's a better way to go about any of it I heavily encourage you to just go ahead and change it.