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

modules: use bot.say()'s trailing parameter #1995

Merged
merged 7 commits into from
Jan 2, 2021
Merged

Conversation

dgw
Copy link
Member

@dgw dgw commented Nov 30, 2020

Description

This updates every core plugin I could find with an existing handler for "too long" output (instagram, reddit, tld, wiktionary, & wikipedia) to use bot.say()'s new trailing parameter from #1958.

Two things that might be worth amending, if others think my thought process or assumptions were wrong:

  • instagram: I rearranged the output to put the caption (most likely to be over-length) at the end. A subsequent commit to this branch (to wikipedia) showed a neat hack that would allow leaving the caption where it was, in the middle of the line, but it seems to make the most sense at the end anyway.
  • reddit: I did not change submission output to use trailing, because:
    • The title is the very first thing in the output
    • The submission's link might also be very long, and we can't truncate those
    • If the link is long enough, it plus the rest of the metadata could be longer than allowed anyway, even without the title
    • Basically, there were too many edge cases I could see, so I didn't touch the current post/submission behavior

Checklist

  • I have read CONTRIBUTING.md
  • I can and do license this contribution under the EFLv2
  • No issues are reported by make qa (runs make quality and make test)
  • I have tested the functionality of the things this change touches

We have very reliable message truncation now, so it's silly to only add
definitions to the output if the length is less than 150 characters.
Seems like 300 is a more reasonable threshold for stopping, since the
amount of usable space in a line tends to be closer to 450.
The new parameter doesn't really help with overly long submission titles
since those appear near the beginning of the output line, but it does
simplify truncating comment previews & subreddit descriptions.
This plugin was a bit more complicated to switch over, since the old
order of operations placed the post caption in the middle when parsing
Instagram's full-fat JSON data.

Moving the caption handling to the end (and tweaking it a bit to make
better use of Python language features) makes it possible to let Sopel
handle truncating the caption itself, using the new `bot.say()` param.

oEmbed handling only pulls the author's name and post title, and it's
unlikely to run into over-length issues, but adding `trailing` to it
anyway for safety doesn't cost anything.
@dgw dgw added the Tweak label Nov 30, 2020
@dgw dgw added this to the 7.1.0 milestone Nov 30, 2020
@dgw dgw requested a review from a team November 30, 2020 08:48
sopel/modules/wikipedia.py Outdated Show resolved Hide resolved
Honestly it feels really good to remove that awful `while` loop with its
embedded magic numbers. I still don't even know what `18` was for.
We sacrifice the ability to know for sure if another Sopel bot's wiki
output was what triggered this call, but gain the ability to include
more of the article text (and test out a hack of the `trailing` param
that will be useful for other plugins with truncatable content that's
not at the very end of the message).

Replaced the `msg_url == trigger` check with a heuristic: Assume that
the triggering user was a Sopel bot if the trigger started with
`[wikipedia] ` and ended with ` | <our wiki page URL>`. Obviously it
won't be a super effective check if two different versions of Sopel with
different Wikipedia output meet, but the old check would also break in
that case. The change seems worthwhile.
@dgw dgw force-pushed the modules-use-bot.say-trailing branch from eef2637 to 54aa021 Compare December 1, 2020 06:21
@dgw dgw merged commit c4a812e into master Jan 2, 2021
@dgw dgw deleted the modules-use-bot.say-trailing branch January 2, 2021 22:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants