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

tell: avoid errors produced by empty tellee #2584

Merged
merged 2 commits into from
Dec 18, 2023

Conversation

SnoopJ
Copy link
Contributor

@SnoopJ SnoopJ commented Dec 11, 2023

Description

This changeset is an alternative to #2582 that transforms the tellee as early as possible.

The use of lstrip("@") in this patch means that an arbitrary number of prefixing @s will be removed, so that @@Someone is transformed into Someone where it would previously have been transformed to @Someone, but since this is an edge case to begin with, I don't think it's any problem.

Longer term, we'd like to have better awareness of if tellee is a valid nick or not. I've filed #2583 for this purpose.

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 lint and make test)
  • I have tested the functionality of the things this change touches

@SnoopJ SnoopJ added the Bugfix Generally, PRs that reference (and fix) one or more issue(s) label Dec 11, 2023
@SnoopJ SnoopJ added this to the 8.0.0 milestone Dec 11, 2023
Co-authored-by: bronzeguy <151421032+bronzeguy@users.noreply.github.com>
Co-authored-by: dgw <dgw@technobabbl.es>
@dgw dgw force-pushed the bugfix/fix-empty-tellee branch from 47e30a7 to 2a0eb9e Compare December 11, 2023 04:29
@dgw dgw requested a review from Exirel December 11, 2023 04:39
@dgw
Copy link
Member

dgw commented Dec 11, 2023

Note to self: Manually close #2582 when merging, since GH artificially doesn't allow merging a PR to auto-close another PR. (I say "artificially" because you can auto-close issues, and "all PRs are issues but not all issues are PRs" according to the GH API.)

Copy link
Contributor

@Exirel Exirel left a comment

Choose a reason for hiding this comment

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

I'd add a few @plugin.examples for the new cases. Like:

@plugin.example('.ask @', 'ask whom?')
@plugin.example('.tell @', 'tell whom?')
@plugin.example('.tell @Exirel:', 'tell Exirel what?')

That way we can test that the command removes some characters from the tellee.

Co-authored-by: Florian Strzelecki <florian.strzelecki@gmail.com>
@SnoopJ SnoopJ force-pushed the bugfix/fix-empty-tellee branch from cbd2079 to 4edc352 Compare December 14, 2023 23:09
@dgw dgw merged commit 973a489 into sopel-irc:master Dec 18, 2023
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bugfix Generally, PRs that reference (and fix) one or more issue(s)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants