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

bot: only suppress echo for PRIVMSG and NOTICE #1672

Merged
merged 1 commit into from
Oct 27, 2019

Conversation

half-duplex
Copy link
Member

thijseigenwijs on IRC reported that the bot doesn't update privilege tracking when it's the one to change a mode. This is because the server's MODE message is ignored by the echo suppression. This PR changes the echo suppression to only suppress PRIVMSG and NOTICE events.

  • Can we assume the event will always be upper case?
  • Any others that should be suppressed?

thijseigenwijs reported that the bot doesn't update privilege tracking
when it's the one to change a mode
is_echo_message = nick.lower() == self.nick.lower()
is_echo_message = (
nick.lower() == self.nick.lower() and
event in ["PRIVMSG", "NOTICE"]
Copy link
Member

Choose a reason for hiding this comment

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

No need to wonder if the event will always be uppercase if you do this.

Suggested change
event in ["PRIVMSG", "NOTICE"]
event.upper() in ["PRIVMSG", "NOTICE"]

But there's plenty of code already assuming these will always be uppercase, so it's probably not worth doing only here. (FWIW, most of that code seems to use 'single quotes', but that's also likely not worth changing here.)

@Exirel
Copy link
Contributor

Exirel commented Aug 9, 2019

* Can we assume the event will always be upper case?

Yes.

@dgw dgw added Bugfix Generally, PRs that reference (and fix) one or more issue(s) Low Priority Tweak labels Aug 11, 2019
@dgw dgw added this to the 7.0.0 milestone Aug 11, 2019
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 haven't managed to come up with any reasons this would break something. Might as well put it in the merge queue now and fix any edge cases down the road.

@dgw dgw merged commit 1c460a8 into sopel-irc:master Oct 27, 2019
@half-duplex half-duplex deleted the echo-mode-fix branch October 29, 2019 20:34
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) Low Priority Tweak
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants