Skip to content
This repository has been archived by the owner on Feb 20, 2023. It is now read-only.

Manage subscriptions #392

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

Manage subscriptions #392

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
003eb2b
praw fixes
Jul 13, 2017
41560f0
subscription management
Jul 13, 2017
1069f7c
controls for subcription management
Jul 13, 2017
b5193da
additional control documentation
Jul 14, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
22 changes: 22 additions & 0 deletions CONTROLS.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,3 +73,25 @@ In submission mode you can view the self text for a submission and browse commen
:``o`` or ``ENTER``: Open the comment permalink with your web browser
:``SPACE``: Fold the selected comment, or load additional comments
:``b``: Display URLs with urlview

-----------------
Subscription Mode
-----------------

In subscription mode you can view either the subreddits you are subscribed to
or the multireddits you've curated.

:``l`` or ``►``: Open the selected subreddit in a new window
:``SPACE``: View contents of multireddit
:``d``: Unsubscribe from subreddit or delete multireddit

The ``/`` prompt allows you to subscribe to subreddits or create multireddits.

When viewing your subscriptions you may add new subscriptions:

* ``python``
* ``python+commandline+linux``

When viewing a list of your multireddits you may initialize a new one:

``programming/python+ruby+haskell``
Copy link
Owner

Choose a reason for hiding this comment

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

Documentation looks great!

11 changes: 9 additions & 2 deletions rtv/content.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -264,8 +264,8 @@ def strip_praw_subscription(subscription):
else:
data['type'] = 'Subscription'
data['name'] = "/r/" + subscription.display_name
data['title'] = subscription.title

data['title'] = subscription.title if subscription._has_fetched \
else ''
return data

@staticmethod
Expand Down Expand Up @@ -694,6 +694,13 @@ def from_user(cls, reddit, loader, content_type='subreddit'):

return cls(name, items, loader)

@classmethod
def from_multireddit(cls, reddit, loader, multi):
items = iter(multi.subreddits)
content = cls('My Multireddit: {}'.format(multi.path), items, loader)
content._multireddit = multi
Copy link
Owner

Choose a reason for hiding this comment

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

I would rather not make this a private variable if you intend on accessing it from outside of the instance. Would you be opposed to adding this to the SubscriptionContent class initializer?

    def __init__(self, name, subscriptions, loader, multireddit=None):

        self.name = name
        self.multireddit = multireddit
        self.order = None
        self.query = None
        self._loader = loader
        self._subscriptions = subscriptions
        self._subscription_data = []

return content

@property
def range(self):
return 0, len(self._subscription_data) - 1
Expand Down
4 changes: 2 additions & 2 deletions rtv/packages/praw/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,7 @@ def _req_error(*_, **__):
self.http.proxies['http'] = self.config.http_proxy
if self.config.https_proxy:
self.http.proxies['https'] = self.config.https_proxy
self.modhash = None
self.modhash = ''
Copy link
Owner

Choose a reason for hiding this comment

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

Any changes to praw should be made in a separate pull request here https://github.com/praw-dev/praw. That way, the praw test suite will be run against the changes and we can document everything that has diverged from the official release. Whenever changes are made to the praw branch, I run scripts/update_package.py which copies the changes over and updates the commit hash.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay. I'll send those changes upstream. I wasn't sure if the older version would be accepting changes.

Copy link
Owner

Choose a reason for hiding this comment

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

They don't, my bad. I meant to link here https://github.com/michael-lazar/praw3


# Check for updates if permitted and this is the first Reddit instance
# if not disable_update_check and not BaseReddit.update_checked \
Expand Down Expand Up @@ -631,7 +631,7 @@ def request_json(self, url, params=None, data=None, as_objects=True,
# Update the modhash
if isinstance(data, dict) and 'data' in data \
and 'modhash' in data['data']:
self.modhash = data['data']['modhash']
self.modhash = data['data']['modhash'] or ''
return data


Expand Down
2 changes: 1 addition & 1 deletion rtv/packages/praw/objects.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1688,7 +1688,7 @@ def __init__(self, reddit_session, author=None, name=None,
json_dict=None, fetch=False, **kwargs):
"""Construct an instance of the Multireddit object."""
author = six.text_type(author) if author \
else json_dict['path'].split('/')[-3]
else json_dict['path'].split('/')[-4]
Copy link
Owner

Choose a reason for hiding this comment

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

What is the purpose of this change, is it fixing a bug in praw?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes it's a bugfix:
author = "/user/multi-mod/m/android".split('/')[-4] = "multi-mod"

Copy link
Owner

Choose a reason for hiding this comment

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

>>> "/user/multi-mod/m/android".split('/')[-4]
'user'

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry, I meant this (note the trailing slash):

>>> "/user/multi-mod/m/android/".split('/')[-4]
'multi-mod'

if not name:
name = json_dict['path'].split('/')[-1]

Expand Down
91 changes: 80 additions & 11 deletions rtv/subscription_page.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
from .page import Page, PageController
from .content import SubscriptionContent, SubredditContent
from .objects import Color, Navigator, Command
from .packages import praw


class SubscriptionController(PageController):
Expand Down Expand Up @@ -43,17 +44,49 @@ def refresh_content(self, order=None, name=None):
self.nav = Navigator(self.content.get)

@SubscriptionController.register(Command('PROMPT'))
def prompt_subreddit(self):
"Open a prompt to navigate to a different subreddit"

name = self.term.prompt_input('Enter page: /')
if name is not None:
with self.term.loader('Loading page'):
content = SubredditContent.from_name(
self.reddit, name, self.term.loader)
if not self.term.loader.exception:
self.selected_subreddit = content
self.active = False
def add_subreddit(self):
"Open a prompt to add or remove multi/subreddit"

context = self.content.name
obj = self.get_selected_item()['object']

if context == 'My Subreddits':
name = self.term.prompt_input('Subscribe to: /')
if name is not None:
with self.term.loader('Adding /r/{}'.format(name, context)):
for n in name.split('+'):
Copy link
Owner

Choose a reason for hiding this comment

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

Is supporting the + format really necessary here? I understand using it for initializing multireddits, but I can't imagine that somebody is going to want to subscribe to more than a single subreddit at once. The reason I bring it up is that the error handling could be a lot clearer (show a terminal notification if the subscription fails) if there is only one subreddit. I worry that silently ignoring invalid subreddits could be seen as an error from the end user's perspective.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Not really necessary. I just wanted to keep the format consistent with the multireddit initialization syntax. I'm fine with removing it.

try:
self.reddit.get_subreddit(n).subscribe()
except:
pass
elif context == 'My Multireddits':
name = self.term.prompt_input('Initialize multireddit '
'(multi/sub+sub): ')
if name is not None:
try:
multi, subreddits = name.split('/')
subreddits = subreddits.split('+')
except:
self.term.show_notification('Initialization Format: '
'multireddit/subreddit+subreddit+...')
return None
else:
with self.term.loader('Creating {} with {}'.format(multi,
subreddits)):
self.reddit.create_multireddit(multi,
subreddits=subreddits)
elif context.startswith('My Multireddit:'):
name = self.term.prompt_input('Add subreddits(s): /')
if name is not None:
with self.term.loader('Adding /r/{}'.format(name)):
for n in name.split('+'):
Copy link
Owner

Choose a reason for hiding this comment

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

Same comment here as above

try:
self.content._multireddit.add_subreddit(n)
except:
pass
else:
return None
self.refresh_content()

@SubscriptionController.register(Command('SUBSCRIPTION_SELECT'))
def select_subreddit(self):
Expand All @@ -73,6 +106,42 @@ def close_subscriptions(self):

self.active = False

@SubscriptionController.register(Command('DELETE'))
def delete_reddit(self):
"Delete the selected reddit"

context = self.content.name
data = self.get_selected_item()
listing = data['object']
name = data['name']

if isinstance(listing, praw.objects.Multireddit) and \
self.term.prompt_y_or_n('Delete {}? (y/n): '.format(name)):
with self.term.loader('Deleting {} from {}'.format(name, context)):
self.reddit.delete_multireddit(listing.name)
self.refresh_content()
else:
with self.term.loader('Deleting {} from {}'.format(name, context)):
Copy link
Owner

Choose a reason for hiding this comment

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

The language could be more specific here to avoid confusion - "Unsubscribing {} from {}"

if hasattr(self.content, '_multireddit'):
self.content._multireddit.add_subreddit(
listing.display_name, _delete=True)
elif isinstance(listing, praw.objects.Subreddit):
listing.unsubscribe()
self.refresh_content()

@SubscriptionController.register(Command('SUBMISSION_TOGGLE_COMMENT'))
Copy link
Owner

Choose a reason for hiding this comment

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

In this case, I think we should add a new binding since SUBMISSION_* is reserved for the submission page. It can still be bound to the space bar (0x20) but it should have a separate definition like SUBSCRIPTION_VIEW_MULTIREDDIT.

def open_multireddit(self):
"View content of multireddit"

context = self.content.name
multi = self.get_selected_item()['object']
if context == 'My Multireddits':
with self.term.loader():
self.content = SubscriptionContent.from_multireddit(
self.reddit, self.term.loader, multi)
if not self.term.loader.exception:
self.nav = Navigator(self.content.get)

def _draw_banner(self):
# Subscriptions can't be sorted, so disable showing the order menu
pass
Expand Down