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

youtube-video-duration: filter out videos by their duration #622

Merged
merged 18 commits into from Jan 19, 2024

Conversation

Nomes77
Copy link
Member

@Nomes77 Nomes77 commented Jan 18, 2024

Copy link
Contributor

github-actions bot commented Jan 18, 2024

Thanks for your contribution! The automated tests passed, we will review your PR shortly!

@xvello xvello changed the title youtube-short-videos.yaml: Filter out short videos, which are not shorts youtube-short-videos: Filter out short videos, which are not shorts Jan 18, 2024
@xvello
Copy link
Member

xvello commented Jan 18, 2024

I'll trust your final judgement, but I have reservations about this approach:

  • If my memory serves me well, I don't think that ^ and $ work reliably in :has-text selectors, and @3DWORX-SA reported that the rules don't work on their side. Something is definitely necessary to avoid matching a video that runs 10:31 long though. Our old rules used \s around the time regexp, but with more experience I'd try using \b (word boundary) as a more generic boundary

  • While flexible, regexp are difficult to understand for new users, and error prone for everyone. I deployed the branch at https://staging.letsblock.it/filters/youtube-short-videos, and I would not know what to input, had I not looked at the PR. We definitely need to write a proper description with some examples that people can just copy:

    • less than a minute
    • less than 10 minutes
    • one hour or more
    • 2 hours or more
  • If we keep a generic regexp input, the template can be used to filter long videos, so I'd rename it youtube-video-duration, for consistency with youtube-video-title

@Nomes77 Nomes77 changed the title youtube-short-videos: Filter out short videos, which are not shorts youtube-video-duration: Filter out videos on duration Jan 19, 2024
@Nomes77
Copy link
Member Author

Nomes77 commented Jan 19, 2024

You were right about ^ and $ not working. However \b had some unintentional side effects.
With \b , \d\d:06 will also block 10:06:28 in stead of only 28:06.
I choose for keeping it a regexp. Hopefully, the explanation is sufficient.

I think that more example are not needed.

@Nomes77 Nomes77 requested a review from JohnyP36 January 19, 2024 06:38
Copy link
Member

@xvello xvello left a comment

Choose a reason for hiding this comment

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

Nice work! Didn't test the rules, but I trust you did.
It's deployed at https://staging.letsblock.it/filters/youtube-video-duration

data/filters/templates/youtube-video-duration.yaml Outdated Show resolved Hide resolved
data/filters/templates/youtube-video-duration.yaml Outdated Show resolved Hide resolved
@xvello xvello changed the title youtube-video-duration: Filter out videos on duration youtube-video-duration: filter out videos by their duration Jan 19, 2024
@xvello
Copy link
Member

xvello commented Jan 19, 2024

Indeed @Nomes77, \b will not work here, as the : separator does trigger a word boundary! \s it is then.

@xvello xvello merged commit 93eee61 into main Jan 19, 2024
5 checks passed
@xvello xvello deleted the Nomes77-patch-1 branch January 19, 2024 22:00
@JohnyP36
Copy link
Member

Late reply: yes, the rules work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
3 participants