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

trigger: inconsistent sender for other general messages like QUIT #2331

Closed
dgw opened this issue Jul 23, 2022 · 1 comment
Closed

trigger: inconsistent sender for other general messages like QUIT #2331

dgw opened this issue Jul 23, 2022 · 1 comment
Labels
Bug Things to squish; generally used for issues Core/IRC Protocol Handling
Milestone

Comments

@dgw
Copy link
Member

dgw commented Jul 23, 2022

This is a follow-up to #2217, which reports an exception case that is technically fixed, but the underlying cause (weird behavior with certain message types) still exists.

Code referenced below sets an intermediate variable to None, then populates it based on the arguments to whatever command Sopel received unless that command was QUIT. This leaves some edge cases hanging about, like handling of AWAY on servers with away-notify turned on—another command that's just generally sent to any clients that share a channel with the acting user, but isn't associated with any channel or private-message buffer.

sopel/sopel/trigger.py

Lines 213 to 225 in 71380df

# If we have arguments, the first one is the sender
# Unless it's a QUIT event
target: Optional[identifiers.Identifier] = None
if self.args and self.event != 'QUIT':
target = self.make_identifier(self.args[0])
# Unless we're messaging the bot directly, in which case that
# second arg will be our bot's name.
if target.lower() == own_nick.lower():
target = self.nick
self.sender = target

Making this consistent would be best. trigger.sender being None is safer now that #2217 was fixed in #2306, and it's definitely not ideal for the sender to be set to Identifier("the away message") on received AWAY commands.

@dgw dgw added Bug Things to squish; generally used for issues Needs Triage Issues that need to be reviewed and categorized labels Jul 23, 2022
@dgw dgw added this to the 8.0.0 milestone May 2, 2023
@dgw dgw added Core/IRC Protocol Handling and removed Needs Triage Issues that need to be reviewed and categorized labels May 2, 2023
@dgw
Copy link
Member Author

dgw commented May 2, 2023

Going to say #2359 resolves this. If we find other cases, we'll reopen/follow-up.

@dgw dgw closed this as completed May 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Things to squish; generally used for issues Core/IRC Protocol Handling
Projects
None yet
Development

No branches or pull requests

1 participant