sopel-irc / sopel

:robot::speech_balloon: An easy-to-use and highly extensible IRC Bot framework. Formerly Willie.
https://sopel.chat
Other
948 stars 402 forks source link

title: when used with no argument, refer to the last url in the channel #2357

Open eoli3n opened 2 years ago

eoli3n commented 2 years ago

Requested Feature

toto: https://github.com
tata: nice link
tata: .title
bot: tata: GitHub: Where the world builds software · GitHub | github.com

Problems Solved

No need to copy/paste the url anymore.

Alternatives

No response

Notes

No response

eoli3n commented 2 years ago

aahh, forget it, it already does, my first test failed...

eoli3n commented 2 years ago

No ! It repeats the last use

toto: https://github.com
tata: nice link
tata: .title https://github.com
bot: tata: GitHub: Where the world builds software · GitHub | github.com
toto: check this one too https://gitlab.com
tata: .title
bot: tata: GitHub: Where the world builds software · GitHub | github.com
tata: hey I wanted gitlab, not github title.
dgw commented 2 years ago

This already works as described if url is set to auto-title links. Do you have url.enable_auto_title = False or similar in your config? (Of course, I realize that if auto-title is on, there is no need to say .title because the bot will have already done it.)

Both the existing feature and the proposed feature get confusing if the last message (or .title command) contains more than one URL, though. Assuming url.enable_auto_title = False, what should happen in this case?

<toto> https://github.com https://gitlab.com https://sr.ht
<tata> .title

Possible actions for bot in response to tata's .title command:

  1. Bot titles the first URL from the last message containing at least one URL
  2. Bot titles a random URL from the last message containing at least one URL
  3. Bot titles the last URL from the most recent message containing at least one URL
  4. Bot titles all URLs from the last message containing at least one URL
  5. Bot repeats the title of the last .title command (obviously not, but included anyway for completeness)

I should note that this would not become configurable. Only one of these possible behaviors must be chosen as a sane default. For reference, the last_seen_url is currently saved as the last URL processed, which is usually the last URL in the message:

https://github.com/sopel-irc/sopel/blob/71380df85212747a6c02d8c37c9ffb7836001278/sopel/modules/url.py#L321

There's no technical reason not to save a list of URLs in the last message instead; the last_seen_url for each channel is already fetched into a (one-element) list when .title is called with no argument.

https://github.com/sopel-irc/sopel/blob/71380df85212747a6c02d8c37c9ffb7836001278/sopel/modules/url.py#L255

I should also note that any custom plugin is welcome to do something like save the most recent message containing at least one URL for each channel, and define a custom command that calls on functions from sopel.modules.url to do the hard work (actually fetching titles). Wouldn't be too hard to write that, but the caveat is that nothing in sopel.modules is considered public API (so such a custom plugin could break at any time).

eoli3n commented 2 years ago

Do you have url.enable_auto_title = False or similar in your config?

Yes. By the way, it would be great to be able to disable auto_title per channel.

4. Bot titles all URLs from the last message containing at least one URL

I vote for this, which seems to be, to me, the most logical.

There's no technical reason not to save a list of URLs in the last message instead

This would be the way to.

I should also note that any custom plugin is welcome to do something like save the most recent message containing at least one URL for each channel

This is the clever way. A dirty one would be to parse log in reverse mode from the end to search for the ~first~ message containing at least one url.

RustyBower commented 2 years ago

I'm not a fan of this.

I like the explicit functionality of .title URL displaying the URL's title, or enabling auto title and having it implement that behavior automatically.

dgw commented 2 years ago

@RustyBower Even if this just means updating last_seen_url when enable_auto_title == False here so it can be used later if/when someone calls .title with no arg?

https://github.com/sopel-irc/sopel/blob/13ea685cddf489328ed51d50ce282ec5a8445037/sopel/modules/url.py#L291-L293

(May also require shuffling a few conditionals, e.g. the one right after that to skip commands sent to other plugins.)

RustyBower commented 2 years ago

I mean that's fine, I just think we're overloading the plugins with some weird lookback functionality /shrug