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

coretasks: NAMESX when multi-prefix is not available #1997

Merged
merged 1 commit into from
Jan 4, 2021

Conversation

Exirel
Copy link
Contributor

@Exirel Exirel commented Dec 10, 2020

Description

As per the discussion in #1496 this makes Sopel send a PROTOCTL NAMESX command when:

  1. we receive a RPL_ISUPPORT message from the server
  2. multi-prefix is not an available capability
  3. NAMESX is now activated

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 the Feature label Dec 10, 2020
@Exirel Exirel added this to the 7.1.0 milestone Dec 10, 2020
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.

The actual implementation of this is so stupidly simple that I can't imagine why I didn't think of this approach when I dug into NAMESX stuff a month ago. Of course it works to check if the bot already knows about NAMESX being advertised. Big brain time!

Just editing the comments like I do. One of them reminded me of a very rude meme phrase starting, "Do you are have" and ending with an insult, so I had to fix. x)

sopel/coretasks.py Outdated Show resolved Hide resolved
sopel/coretasks.py Outdated Show resolved Hide resolved
Co-authored-by: dgw <dgw@technobabbl.es>
@Exirel Exirel requested a review from dgw December 10, 2020 16:00
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.

So I just thought of a different way to write the comments added to handle_isupport() but it's not enough of an improvement to bother.

Are you planning to work on the WHOX part of #1496? If you are, I should merge this next (like later tonight when Travis' queue is shorter) so you can build on it.

@Exirel
Copy link
Contributor Author

Exirel commented Dec 10, 2020

Are you planning to work on the WHOX part of #1496? If you are, I should merge this next (like later tonight when Travis' queue is shorter) so you can build on it.

Not really. I don't get how WHOX is supposed to work (yet). Maybe another time!

@dgw
Copy link
Member

dgw commented Dec 10, 2020

OK, one of us will look at WHOX in the coming weeks. I tweaked how Sopel detects WHOX availability in #1998, for a start. Need more research before going any further, but not sure when I'll get the time to do that. Probably next (not this coming) weekend, at the earliest, and I have doubts about that timeline. It's gonna be a busy month. :-/

@dgw dgw merged commit 626d7cd into sopel-irc:master Jan 4, 2021
@Exirel Exirel deleted the protoctl-namesx branch May 1, 2021 17:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants