-
Notifications
You must be signed in to change notification settings - Fork 10k
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
Add YoutubeSearchURLIE back #27749
Add YoutubeSearchURLIE back #27749
Conversation
This should be implemented vice versa: search URLs should be handled by tab extractor and |
What do you think about this now? Should it be brought up-to-date, or should the alternative approach
be implemented? |
I assume you are asking me. The search and tab API endpoints are slightly different. So fully generalizing the two is arguably not a good design. In yt-dlp, we have generalized as much of the code as possible and also moved the functions to a If you want a quick and dirty fix, this PR should still work well enough. But otherwise, I don't recommend merging this as-is. But it would also be quite difficult to directly cherry-pick yt-dlp's code now because of how much it has diverged. |
Thanks, it looks like further study is needed. I don't see a lot of complaint about this although I do wonder how it was removed given the conservatism shown about other features. |
I did not see #30568 when I wrote my previous reply Would you like me to clean up this PR a bit? Adding full support for playlist and channel searches (like yt-dlp has now) would be difficult, but I can fix the architectural issues mentioned above |
Sorry, I realised I should have linked that after posting! If you can easily get the PR into a state that you'd be happy with, please do. Otherwise we can let the issue mature until a better solution magically appears. I seem to recall managing to create some interesting self-referential loops when I tried to fiddle with the same problem. |
79c0bf7
to
ed99d68
Compare
@dirkf I've made the changes and merged with master |
Thanks, that's great, I'll review it as soon as time allows: as you can imagine there is a lot to do and it's also tax return time in the UK! |
You dropped the tests? ef49dda |
Please follow the guide below
x
into all the boxes [ ] relevant to your pull request (like that [x])Before submitting a pull request make sure you have:
In order to be accepted and merged into youtube-dl each piece of code must be in public domain or released under Unlicense. Check one of the following options:
What is the purpose of your pull request?
Description of your pull request and other information
Add back YoutubeSearchURLIE which was removed during the reworking of youtube.py
Note that I have not set any
_MAX_RESULTS
. The user can provide the same using--playlist-end
The code is taken from the cumulative changes in yt-dlp. Relevent commits: yt-dlp/yt-dlp@cc16383 yt-dlp/yt-dlp@a6213a4 yt-dlp/yt-dlp@386e1dd yt-dlp/yt-dlp@3462ffa
Closes: #27740