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

core: add inspircd operprefix !/+y channel mode #1671

Merged
merged 3 commits into from
Jan 13, 2020

Conversation

half-duplex
Copy link
Member

On inspircd 3 with the operprefix module, as oper, create a channel. You'll have !. Add the bot. The bot sees you as having no modes (0), and you can't do anything until you +o yourself.

Might not be a 6.6.x thing, but if I don't open this now I'm not going to remember.

Future enhancements:

  • Have the symbol/mode/constant stuff in one place instead of 5 (!)
  • Just generate this stuff from what the server tells us when we connect

@dgw
Copy link
Member

dgw commented Jul 29, 2019

Parsing supported modes from server connect spam is definitely on the road map. The current state annoys me, too! See #1536 for the parsing bit, after which we can rewrite the mode handling again to take advantage of the new data.

Does this really need to touch the indentation? Not that I think we'll actually put this into 6.6.x, though, so maybe the best choice would be rebasing onto master.

@half-duplex half-duplex force-pushed the inspircd-oper-cmode branch from d2117d2 to 8fd350d Compare July 29, 2019 03:41
@half-duplex half-duplex changed the base branch from 6.6.x to master July 29, 2019 03:41
@dgw dgw added this to the 7.0.0 milestone Jul 29, 2019
@half-duplex half-duplex force-pushed the inspircd-oper-cmode branch from 8fd350d to 5a6e372 Compare July 30, 2019 22:40
Copy link
Contributor

@HumorBaby HumorBaby left a comment

Choose a reason for hiding this comment

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

Regardless of how I feel about adding these highly IRCd-specific modes/prefixes, if you're gonna add Insp3 stuff, might as well get it all 😁

sopel/coretasks.py Outdated Show resolved Hide resolved
@HumorBaby
Copy link
Contributor

Just to address my comment from the review:

Regardless of how I feel about adding these highly IRCd-specific modes/prefixes...

I'm not a fan :/ According to modern specs, the only standard mode/prefixes are (ov)@+ (OP and VOICE). While I understand (and appreciate) the inclusion of commonly used modes like HALFOP and OWNER (founder) etc., I am really looking forward to the re-work of mode/prefix + privilege tracking 😄

half-duplex:

On inspircd 3 with the operprefix module, as oper, create a channel. You'll have !. Add the bot. The bot sees you as having no modes (0), and you can't do anything until you +o yourself.

... as (I believe) it should be. The bot does not need to recognize a server op as anyone special in a channel until they make themselves act that way. If they really needed to override things with the bot, they could just +o themselves as needed, no?

@half-duplex
Copy link
Member Author

half-duplex commented Aug 3, 2019

The bot does not need to recognize a server op as anyone special in a channel until they make themselves act that way. If they really needed to override things with the bot, they could just +o themselves as needed, no?

Until we can support custom/dynamic modes, ignoring +y (and probably +Y) is a POLA violation, and something I expect to trip on regularly. I acknowledge and agree that manually adding support for new modes is the wrong way to do this, but until we have something better, this fixes it simply and without any obvious side effects.

sopel/module.py Outdated Show resolved Hide resolved
@half-duplex half-duplex force-pushed the inspircd-oper-cmode branch from 6f1c60f to 176b1bc Compare August 3, 2019 18:00
Copy link
Contributor

@HumorBaby HumorBaby left a comment

Choose a reason for hiding this comment

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

but until we have something better, this fixes it simply and without any obvious side effects

That's fair; plus, InspIRCd is pretty common, so might as well have it in here until we have something better

@half-duplex half-duplex force-pushed the inspircd-oper-cmode branch from 176b1bc to 007c708 Compare August 3, 2019 18:02
@dgw
Copy link
Member

dgw commented Aug 3, 2019

Until we can support custom/dynamic modes, ignoring +y (and probably +Y) is a POLA violation

I disagree with this. No one should be astonished that a non-standard mode is not handled.

At present I honestly don't intend to merge this. Maybe if we can't get proper dynamic mode handling done for 7.0, it can go in as a stopgap. But if #1536 and follow-ups proceed apace, we shouldn't need it.

so might as well have it in here until we have something better

*sprays water* Or we can just implement something better™ the first time!

@dgw dgw force-pushed the inspircd-oper-cmode branch from b5e4a1d to 7821080 Compare November 4, 2019 05:51
@dgw
Copy link
Member

dgw commented Nov 4, 2019

Updated Just In Case™. I still don't really like this change.

@dgw
Copy link
Member

dgw commented Dec 6, 2019

@half-duplex I asked this on IRC, but for the sake of keeping the conversation together:

Would you be interested in implementing "proper" mode parsing based on the server's PREFIX token, on top of the ISUPPORT parsing to be introduced soon by #1758? That could straight-up replace this and all the other nasty hard-coded privilege prefixes at once.

Understandable if you want to leave it for a future release, though. 7.0 is going to come out this month no matter what if I have anything to say about it, and we might have to alter some API (e.g. the privilege constants in sopel.module) around such a change. That would probably take longer to design than is reasonable for this late in the cycle.

@dgw dgw force-pushed the inspircd-oper-cmode branch from 7821080 to 02d1638 Compare January 12, 2020 19:00
@dgw
Copy link
Member

dgw commented Jan 12, 2020

7.0 is going to come out this month no matter what if I have anything to say about it

Ended up not finishing 7.0 in time for my long (and very busy) overseas trip, so here we are in the next month with another rebase. No way am I delaying 7.0 again to design dynamic mode parsing/handling, so looks like this will get to be a stopgap after all.

When merging, I'll make sure we have an issue to remind us about dynamic mode/prefix parsing.

@dgw dgw merged commit 3389856 into sopel-irc:master Jan 13, 2020
@half-duplex half-duplex deleted the inspircd-oper-cmode branch January 17, 2020 01:33
@half-duplex
Copy link
Member Author

See #2197 for prefix parsing 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants