Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Comments: Add support for new format #4576

Merged

Conversation

ChunkyProgrammer
Copy link
Contributor

@ChunkyProgrammer ChunkyProgrammer commented Apr 9, 2024

Closes #4566

This should fix the issues with most comments but there are still some more changes that would need to be made like adding support for formatting (bold, italic, underline) and channel emojis. The new format for these changes is similar to how descriptions of commandRuns.
image

src/invidious/comments/youtube.cr Outdated Show resolved Hide resolved
src/invidious/comments/youtube.cr Outdated Show resolved Hide resolved
src/invidious/comments/youtube.cr Outdated Show resolved Hide resolved
@ChunkyProgrammer ChunkyProgrammer marked this pull request as draft April 10, 2024 22:00
Co-Authored-By: Samantaz Fox <coding@samantaz.fr>
@ChunkyProgrammer ChunkyProgrammer marked this pull request as ready for review April 11, 2024 01:31
@wanniethecharacter

This comment was marked as spam.

Co-Authored-By: Samantaz Fox <coding@samantaz.fr>
@Bubba8291

This comment has been minimized.

@Yetangitu
Copy link
Contributor

Yetangitu commented Apr 16, 2024

See #4566 (comment), testing a number of video's with this PR applied shows comments loaded on every load. Given that this PR does not remove the original comment code but uses it as a fallback if the 'new' comment model is not present this seems to be a workable solution. Once YT finishes its A/B testing and settles on one of the current comment models the fallback (or the new code if they decide to ditch the new model) can be removed.

@siddharth-narayan

This comment has been minimized.

Yetangitu added a commit to Yetangitu/invidious that referenced this pull request Apr 17, 2024
@Yetangitu
Copy link
Contributor

For those who can not (or do not want to) build a release themselves and who feel confident (or silly) enough to trust a random binary put on the 'net I put a release on my fork:

https://github.com/Yetangitu/invidious/releases/tag/0.20.1-PR4576

Here's the actual binary, built on Debian stable:

https://github.com/Yetangitu/invidious/releases/download/0.20.1-PR4576/invidious

This is what I'm running on my server, if it works for me it might work for you.

@iv-org iv-org locked and limited conversation to collaborators Apr 17, 2024
Copy link
Member

@SamantazFox SamantazFox left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Outside my comments below, everything works nicely, both on video comments as well as community post comments! Thanks for the PR :)

src/invidious/comments/youtube.cr Outdated Show resolved Hide resolved
src/invidious/comments/youtube.cr Outdated Show resolved Hide resolved
src/invidious/comments/youtube.cr Show resolved Hide resolved
src/invidious/comments/youtube.cr Outdated Show resolved Hide resolved
@SamantazFox
Copy link
Member

I'll definitely need to work on decoupling the upstream from the downstream JSON logic one day. This is becoming quite a problem. Also I realized that the community posts use a different chunk of logic, where all of that could be unified.

Co-Authored-By: Samantaz Fox <coding@samantaz.fr>
@SamantazFox SamantazFox added in-testing This feature has been deployed and is being tested ready and removed in-testing This feature has been deployed and is being tested labels Apr 20, 2024
@TheFrenchGhosty
Copy link
Member

I've been using it for more than a day now via the test instance, seems to work perfectly (seems also faster and more reliable (?) that the previous implementation.

@unixfox
Copy link
Member

unixfox commented Apr 26, 2024

have been running fine on yewtu.be for 2 days: https://github.com/yewtudotbe/invidious-custom/blob/master/patches/012-4576.patch

@SamantazFox SamantazFox changed the title Add support for new comment format Comments: Add support for new format Apr 26, 2024
@SamantazFox SamantazFox merged commit 7c1d271 into iv-org:master Apr 26, 2024
6 of 7 checks passed
@ChunkyProgrammer ChunkyProgrammer deleted the add-support-for-comment-format branch April 27, 2024 00:36
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] Comments won't load on all videos
8 participants