reddit: handle subreddits, and u/ & r/ references

[deathbybandaid]

This PR adds to the functionality of reddit.py by adding:

• Add a subreddit command as well as url pattern match for subreddits

13:32 <@deathbybandaid> https://reddit.com/r/plex
13:32 <reddit-test> [REDDIT] https://www.reddit.com/r/plex | subscribers (124733) | Created at 2008-08-16 - 06:54:02UTC | For questions and comments about the Plex Media Server. The Plex Media Server is smart software that makes playing Movies, TV Shows and other media on your computer simple.
13:32 <@deathbybandaid> .subreddit plex
13:32 <reddit-test> [REDDIT] https://www.reddit.com/r/plex | subscribers (124733) | Created at 2008-08-16 - 06:54:02UTC | For questions and comments about the Plex Media Server. The Plex Media Server is smart software that makes playing Movies, TV Shows and other media on your computer simple.

• add r/ and u/ matching.

13:31 <@deathbybandaid> r/plex
13:31 <reddit-test> [REDDIT] https://www.reddit.com/r/plex | subscribers (124732) | Created at 2008-08-16 - 06:54:02UTC | For questions and comments about the Plex Media Server. The Plex Media Server is smart software that makes playing Movies, TV Shows and other media on your computer simple.
13:31 <@deathbybandaid> u/plex
13:31 <reddit-test> [REDDITOR] plex | https://reddit.com/u/PleX | Link: 1 | Comment: 0

[deathbybandaid]

I mainly wanted to introduce some of this layer in this PR and possibly consider using a separate PR to add a subcommand layer for subreddits.

The other layer would be the basics of this:

    if subcommand == 'new':
        submissions = subreddit.new(limit=1)
    elif subcommand == 'top':
        submissions = subreddit.top(limit=1)
    elif subcommand == 'hot':
        submissions = subreddit.hot(limit=1)
    elif subcommand == 'controversial':
        submissions = subreddit.controversial(limit=1)
    elif subcommand == 'gilded':
        submissions = subreddit.gilded(limit=1)
    elif subcommand == 'rising':
        submissions = subreddit.rising(limit=1)
    elif subcommand == 'random':
        submissions = subreddit.hot(limit=50)

[Exirel]

Nice little feature. My requested changes are mostly cosmetic or minor.

[deathbybandaid]

I have another tweak that I want to add to this, sometimes a subreddit being banned/locked throws a

403 locked

404 banned

[deathbybandaid]

here's the new tweak:

15:23 <@deathbybandaid> r/DarkNetMarkets
15:23 <reddit-test> r/DarkNetMarkets appears to be a banned subreddit!
15:23 <@deathbybandaid> r/sopel
15:23 <reddit-test> r/sopel appears to be a private subreddit!

[Exirel]

Looks pretty good to me!

[dgw]

I haven't had a chance to test this, but the concept is 👍! There are some issues, though. Not sure if they were present before or were introduced in the rebase you just had to do on account of other reddit stuff getting merged.

[deathbybandaid]

All the requested stuff is changed, and pushed,,, I even managed to find a bug that I was able to kill.
(between 3 PRs to a single file, there was bound to be an issue)

[dgw]

Aside from the specific regex suggestion below, we talked a bit on IRC about whether to specifically handle r/all, which "exists" but isn't a normal subreddit, and therefore the code here will incorrectly report it as "banned". I think we'd never falsely report that state in a perfect world, but here in reality we have to decide if it's worth the potential code smell to handle that special case.

Keeping this as just a neutral "Comment" review, because it's only suggestions and not merge-blockers.

[dgw]

I played with some stuff and came up with (what I think is) an even better pattern.

Suggested change

	@rule(r'^(?P<prefix>r|u)/(?P<id>[a-zA-Z0-9-_]+)')
	@rule(r'.*\b(?P<prefix>r|u)/(?P<id>[a-zA-Z0-9-_]+)\b.*')

This will match a user or subreddit reference anywhere in the line, instead of only at the beginning, and properly handles things like r/sopel's moderators or u/deathbybandaid'll get ya!.

It only handles the last reference in the line, though. That's a known issue with the rule decorator, which we'll probably work on for 7.1 (see #1757).

[deathbybandaid]

Your proposed regex is giving me undesired double results, and not even from the "u/" and "r/", but full URL matching.

[deathbybandaid]

I would, though like suggestions for regex that will help me accomplish my second comment on this PR (for my next PR), which would allow users to type "r/plex hot" and get the first item in the sorting "hot"

[dgw]

Discussed more in #1722 (comment) but here's an improved pattern I came up with for this:

(?i)(?<!\S)\/?(?P<prefix>r|u)\/(?P<id>[a-zA-Z0-9-_]+)\b

[dgw]

In Sopel itself, we don't need the flag ((?i)), but we do need to wrap the whole pattern in .* for it to work.

@rule(r'.*(?<!\S)/?(?P<prefix>r|u)/(?P<id>[a-zA-Z0-9-_]+)\b.*')

[deathbybandaid]

"r/all" now displays as

As per a header I found via google. (better than nothing)

[dgw]

Right, so it's not just r/all as the only "special" subreddit. There's also r/popular. Instead of spending time bikeshedding how to handle those now, let's defer any special handling of those to Sopel 7.1.

The only thing (I think) we should worry about in this PR before then is the regex to match the in-text r/ and u/ references. I've played with it a bit just now and come up with this pattern to match just about anything (subject to the limitations of @rule discussed previously):

(?i)(?<!\S)\/?(?P<prefix>r|u)\/(?P<id>[a-zA-Z0-9-_]+)\b

This pattern:

• is case-insensitive
• matches at the start of a line, or in the middle
• does not match inside a URL
• supports both with leading / and without

I think that's everything! Bunch of test cases at https://regexr.com/4p8dh.

One last thing: I retitled the PR to more accurately reflect what it's doing. If you could update your commit message to match, that'd be 💯% awesome!

[deathbybandaid]

I'm stashing what I had for "all" and "popular" for a later PR as per @dgw

If I get hit by a bus,, the below is what i had planned.

    if match.lower() in ['all', 'popular']:
        message = ('[REDDIT] {link}{nsfw} | {public_description}')
        nsfw = ' ' + bold(color('[Possible NSFW]', colors.ORANGE))
        link = "https://reddit.com/r/" + match.lower()
        public_description = ''
        if match.lower() == 'all':
            public_description = ('Today\'s top content from hundreds of '
                                  'thousands of Reddit communities.')
        elif match.lower() == 'popular':
            public_description = ('The top trending content from some of '
                                  'Reddit\'s most popular communities')
        message = message.format(
            link=link, nsfw=nsfw, public_description=public_description)
        bot.say(message)
        return NOLIMIT

[dgw]

Somehow, processing links appears to be broken in the current version of this PR (all I changed locally was the regex for slash-refs). Ran out of time to debug it right now, so I'm leaving the note as a reminder for one of us to continue that.

[deathbybandaid]

I made the suggestions,, and I now see what you mean about links not working too

[deathbybandaid]

However, this works (partly)

[dgw]

@deathbybandaid See #1760 about the link-handling problem. It's not related to this. Please try out this PR with my fixup (just pushed), and squash + reword the commit message to match the PR title if you're happy with it. (I'd appreciate a co-author credit, if you would add one for me during the squash. I've spent so much time on this this week! 😹)

[dgw]

At long last, we've managed to coerce this PR into doing exactly what we want (mostly by playing with regex a lot). Some more tweaks and a regression fix or two (unrelated to these changes) will come from #1760 after this is merged.

[Exirel]

I didn't check the regex. It looks good, but I haven't checked the rules for reddit's URLs. So, I'd say I trust you. :)

LGTM!