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

adding downloadable flag to library list cmd #145

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

rhhayward
Copy link

Hi @mkb79,

I've taken a stab at adding a "--downloadable" flag for the library list command. I tried to duplicate the pattern of the existing "--resolve-podcasts" flag, but if there's a better way please let me know. I also had to add "customer_rights" to the response_groups list to populate the necessary field for the is_downloadable() function.

I also haven't tested this with podcasts, or other types of resources, so if there's tests you'd suggest please let me know.

Thanks,
rhhayward

@mkb79
Copy link
Owner

mkb79 commented May 26, 2023

@rhhayward
Thank you for your pr. I'll review this now. Please give me some time.

@mkb79 mkb79 self-requested a review May 27, 2023 12:02
@@ -189,6 +194,6 @@ def _prepare_item(item):
library = await _get_library(session, client, resolve_podcasts)

books = await asyncio.gather(
*[_prepare_item(i) for i in library]
*[_prepare_item(i) for i in library if downloadable==False or i.is_downloadable()]
Copy link
Owner

Choose a reason for hiding this comment

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

Instead of writing downloadable==False it should be downloadable is False.

Copy link
Owner

@mkb79 mkb79 left a comment

Choose a reason for hiding this comment

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

Code work as is. downloadable==False should be rewritten to downloadable is False.

@mkb79
Copy link
Owner

mkb79 commented May 27, 2023

@rhhayward
Your code works. But instead of adding a downloadable flag we can add a filter option. So a user can select one/multiple predefined filter keywords like downloadable, not downloadable, author. What do you thinking about this?

@rhhayward
Copy link
Author

rhhayward commented May 27, 2023

Sure, I can do that. Here's what I'm thinking:

@click.option('--filters', multiple=True)

and then I'll create flags based on the contents for downloadable, not downloadable, and author=<Name>, and filter by those. Let me give that a shot, and if you want to revise any of the flag names, or how they're handled, let me know.

@rhhayward
Copy link
Author

@mkb79 - I've changed to the --filter format. Let me know what you think.

@rhhayward rhhayward requested a review from mkb79 June 13, 2023 11:49
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