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 additional flags (cookies from browser, max downloads) #60

Merged
merged 3 commits into from
Apr 16, 2024

Conversation

AtlasRW
Copy link
Contributor

@AtlasRW AtlasRW commented Mar 17, 2024

Added support for the following option flags during build :

  • --cookies-from-browser BROWSER[+KEYRING][:PROFILE][::CONTAINER]
  • --max-downloads NUMBER

These are just convenience methods for providing better type checking for parameters and help formatting values correctly.

This is one of my first few PRs on all GitHub, so don't hesitate to tell me if anything could have been done better, and thanks for the work you've already done on the lib !

The --cookies-from-browser introduces a new way of interacting with the lib with optional parameters that are not present in other YoutubeDl methods, and will require users to explicitly pass None to parameters not needed and wrap needed values in Some(), could add more complete documentation for the function to explicit this point if needed.

To clarify the goal of this implementation, requiring the user to pass an already formatted String respecting the format BROWSER[+KEYRING][:PROFILE][::CONTAINER] required by yt-dlp would be needless as it would only be a wrapper around extra_arg that would stay just as prone to error, and other implementations using structs and traits would have added too much to the global architecture of the lib and potential future overhead, so thanks to optional parameters, the logic has been kept straightforward and contained inside the function.

@GyrosOfWar
Copy link
Owner

Hey, thanks for the contribution.

@GyrosOfWar GyrosOfWar merged commit 78985b3 into GyrosOfWar:master Apr 16, 2024
1 check failed
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