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

Improved handling of mode changes in buffextras.lua #14

Open
wants to merge 8 commits into
base: master
Choose a base branch
from

Conversation

someperson
Copy link

I made some changes to buffextras.lua to try to improve the handling of mode changes.

Here's what HexChat normally displays for some activity in a channel:
1 - original

And here is what buffextras turns this into in ZNC 1.6.3:
2 - buffextras

The original buffextras.lua doesn't quite handle the mode changes properly. The mode changes from the server are unreadable, and the changes from the user clearly have a different format from what is normally used:
3 - plugin

After my changes, this is the result:
4 - plugin-mod

It's not perfect. Obviously, HexChat normally does some parsing of the mode changes and even breaks them apart so that it can display a nice message for particular changes (e.g. +b or +l). This is not implemented here, but I think it's at least an improvement.

Thanks for considering this change!

Sam

@TingPing
Copy link
Owner

TingPing commented Sep 3, 2016

Should probably check hexchat.prefs['irc_raw_modes'] also, if enabled use previous behavior.

@someperson
Copy link
Author

Thanks, I didn't know that preference existed. I just changed it so that it will use "Raw Modes" or "Channel Mode Generic" based on how it is set. Also cleaned it up a bit to avoid duplicate code.

@TingPing
Copy link
Owner

TingPing commented Sep 3, 2016

Your usage of Channel Mode Generic also seems wrong. It takes 4 args: nick, mode sign (+/-), mode letter, channel. You seem to combine the last three into one.

@TingPing
Copy link
Owner

TingPing commented Sep 3, 2016

Also if you want to match HexChat behavior exactly we will have to expand this quite a bit. It parses specific letters into specific events like Channel Ban and falls back to Channel Mode Generic: https://github.com/hexchat/hexchat/blob/master/src/common/modes.c#L438-L572

Also when there are multiple modes (I don't know how znc handles this?) there is a different event Channel Modes.

You can see why I took the lazy route of Raw Modes :P

…ras.lua

This should now pretty much emulate what HexChat does normally. I haven't yet tested it enough for full confidence though.
@someperson
Copy link
Author

someperson commented Sep 3, 2016

I've now implemented support for the specific events. The modes are now also therefore split apart and sent separately, like HexChat does normally. I added a hook for 005, as you suggested would be possible, in order to retrieve the CHANMODES parameter, which affects parsing of arguments.

The "Channel Modes" event seems to only be used when the user requests the modes for a channel by way of '/modes #channel'. This is not logged by buffextras, and doesn't represent a change of any sort, so I didn't bother implementing it.

In theory, this should now pretty much emulate HexChat's normal behavior, though I haven't done much testing with it yet. I'd appreciate a second set of eyes on it.

@TingPing
Copy link
Owner

@someperson BTW the chanmodes property landed and has been out in a release for a few weeks.

@@ -1,10 +1,6 @@
-- SPDX-License-Identifier: MIT
--- SPDX-License-Identifier: MIT
Copy link
Owner

Choose a reason for hiding this comment

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

What is this change? :P

Copy link
Author

Choose a reason for hiding this comment

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

Oops. Will fix.

else
emit('Kick', nick, word[6], channel, word_eol[9])
nickmodes = hexchat.props['nickmodes']
Copy link
Owner

Choose a reason for hiding this comment

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

Should be local, same with others?

Copy link
Author

Choose a reason for hiding this comment

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

Yep. Will fix.

@FichteFoll
Copy link
Contributor

FichteFoll commented Jul 26, 2018

What about using the RECV command and have Hexchat do its normal handling instead? Can that also parse tags (so we can fix the timestamps of mode setting)?

@TingPing
Copy link
Owner

What about using the RECV command and have Hexchat do its normal handling instead? Can that also parse tags (so we can fix the timestamps of mode setting)?

In theory. It also may cause unwanted side-effects but I've not looked into it.

@TingPing
Copy link
Owner

Needs rebased.

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

Successfully merging this pull request may close these issues.

3 participants