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

Added playlist-end option. #15

Closed
wants to merge 1 commit into from
Closed

Added playlist-end option. #15

wants to merge 1 commit into from

Conversation

v01d-cypher
Copy link

Hi Ricardo,

Here's the patch for the playlist-end option that I unsuccessfully tried to email to you :-)

Regards,
Nevar

Added code and commentary for the playlist-end option.
Removed check > 0 on playlist-start in favor of using slice on video-ids.
@rg3
Copy link
Collaborator

rg3 commented Nov 4, 2010

Thanks for the code. I've now been able to examine it in more detail. It will not be merged directly because it contains some errors. For example, you don't substract 1 from the playlistend parameter and the range approach you take does not work. When someone doesn't provide playlistend, the default value is -1, which removes the last item from the playlist.

>>> mylist = ['a', 'b', 'c', 'd']
>>> mylist[0:-1]
['a', 'b', 'c']

However, I'll correct those mistakes and commit the changes to the repository.

@rg3
Copy link
Collaborator

rg3 commented Nov 4, 2010

Nevermind the comment about substracting one. Obviously it's not needed.

@rg3
Copy link
Collaborator

rg3 commented Nov 4, 2010

Crap, I didn't know video_encrypted_id was appearing at the end of playlists and that's why I thought your code was wrong, but it turned up in my tests. Still, my manual modifications are done, so it's faster for me to commit my changes now.

Still, thanks for your time, patience, code and for making me discover video_encrypted_id. :)

@v01d-cypher
Copy link
Author

No problem. Yeah, I discovered video_encrypted_id as well during tests. Had no idea what it was, but obviously it wasn't required here :-)

@rg3
Copy link
Collaborator

rg3 commented Nov 4, 2010

I'm also doing some in-depth tests with playlists and everything seems fine so far. The commit should be up in a few minutes.

@ghost ghost mentioned this pull request Aug 17, 2018
5 tasks
@FelixChery FelixChery mentioned this pull request Apr 24, 2019
joedborg referenced this pull request in joedborg/youtube-dl Nov 17, 2020
[pull] master from rg3:master
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants