Skip to content

Conversation

@Exirel
Copy link
Contributor

@Exirel Exirel commented Nov 2, 2022

Description

Fix #1886.

In this PR, I modified PreTrigger to parse the sender to check if the prefix was a STATUSMSG prefix, i.e. a message intended to a specific privilege level in the channel (such as ops, voice, etc.). Once this is done, the PreTrigger remove that prefix from the sender to create an instance of Identifier. On the bot's side, every time a PreTrigger is instantiated, bot.isupport.get('STATUSMSG') is provided.

I added some tests to show how that works and ensure it works as intended without breaking anything, so we should be all good.

Two notes to consider for the future:

  • we may want to change the list of channel prefix to something more restrictive, or not
  • we may want to set a default value for the STATUSMSG value, or have something a bit smarter about that

Both to be considered later, not in this PR.

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

@Exirel Exirel added Bugfix Generally, PRs that reference (and fix) one or more issue(s) Core/IRC Protocol Handling labels Nov 2, 2022
@Exirel Exirel added this to the 8.0.0 milestone Nov 2, 2022
@Exirel Exirel changed the title trigger, irc: use STATUSMSG to trigger, irc: use STATUSMSG to handle status specific messages in channels Nov 2, 2022
Copy link
Member

@dgw dgw left a comment

Choose a reason for hiding this comment

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

I don't think this version of the patch complies with the STATUSMSG spec. See line notes.

Don't worry, there are also some of the usual doc/comment nitpicks. 😜

@Exirel Exirel self-assigned this Nov 21, 2022
@Exirel Exirel requested a review from dgw January 5, 2023 11:54
@Exirel Exirel removed their assignment Jan 5, 2023
Copy link
Member

@dgw dgw left a comment

Choose a reason for hiding this comment

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

I think it's squash time. Might be spaghetti squash, but time will tell us that. 🙃

@Exirel Exirel force-pushed the identifier-statusmsg branch from da4d18b to d3fb9ac Compare January 21, 2023 15:29
@Exirel Exirel force-pushed the identifier-statusmsg branch from d3fb9ac to 08ed2e6 Compare January 21, 2023 15:36
@Exirel
Copy link
Contributor Author

Exirel commented Jan 21, 2023

Squashed!

@dgw dgw merged commit 20be0ce into sopel-irc:master Jan 25, 2023
@dgw dgw added the Breaking Change Stuff that probably should be mentioned in a migration guide label Apr 8, 2023
@Exirel Exirel deleted the identifier-statusmsg branch April 8, 2023 22:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Breaking Change Stuff that probably should be mentioned in a migration guide Bugfix Generally, PRs that reference (and fix) one or more issue(s) Core/IRC Protocol Handling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

STATUSMSG not handled in Identifier.is_nick()

2 participants