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

Implement CHANTYPES support #248

Merged
merged 5 commits into from Sep 9, 2015
Merged

Implement CHANTYPES support #248

merged 5 commits into from Sep 9, 2015

Conversation

LemonBoy
Copy link
Member

It's almost a matter of replacing stuff here and there, I tested it for a couple of minutes and found no problems (doesn't crash and allows me to join channels).

@LemonBoy LemonBoy force-pushed the chantypes branch 2 times, most recently from 23e55e1 to 16c71cf Compare May 15, 2015 15:04
chantypes = "#&!+"; /* normal, local, secure, modeless */

/* @#channel, @+#channel */
if (data[0] == '@' && data[1] == '+')
Copy link
Member

Choose a reason for hiding this comment

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

You got review'd from a different ticket: #253 (comment)

the logic introduced at [this line] does not account for the fact that an arbitrary number of channel types can be prefixed to the "#channel", including duplicates

The function now skips all the leading characters that are in the STATUSMSG. If
the server didn't send the STATUSMSG option then it's assumed to be "@+" for
compatibility with bahamut 2.4 (sic).
@ahf
Copy link
Member

ahf commented Aug 24, 2015

This looks good to me - I want someone else to review it as well. I like anything that kills these odd defaults.

@@ -262,7 +262,7 @@ static void handle_client_cmd(CLIENT_REC *client, char *cmd, char *args,

ignore_next = TRUE;
if (*msg != '\001' || msg[strlen(msg)-1] != '\001') {
signal_emit(ischannel(*target) ?
signal_emit(server_ischannel(SERVER(client->server), target) ?
Copy link
Member

Choose a reason for hiding this comment

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

nit: spaces mixed with tabs

@dequis
Copy link
Member

dequis commented Aug 24, 2015

Also looks good to me, tested with a modified bitlbee that used = and * channels

@ahf
Copy link
Member

ahf commented Aug 25, 2015

Fix the nit and I'll merge it :-)

@@ -42,6 +42,9 @@
#include "fe-windows.h"
#include "fe-irc-server.h"

int fe_channel_is_opchannel(IRC_SERVER_REC *server, const char *target);
const char *fe_channel_skip_prefix(IRC_SERVER_REC *server, const char *target);
Copy link
Member

Choose a reason for hiding this comment

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

Would be nice to put these in some header instead

(reposting comment from diff view, thanks github!)

@dequis
Copy link
Member

dequis commented Sep 3, 2015

Did some more testing with the last commit, looks good to me.

Found some weirdness with formats, but it's not this patch's fault. Well, neither was the STATUSMSG issue, but...

(Screenshot from the point of view of dx__)

Actions with target have the target bolded, own actions with target (or normal messages) don't have it bolded.

Oddly enough it doesn't look like that in my main irssi, which uses almost exactly the same theme as the default. It is a mystery. After spending 5 minutes looking at this issue I decided it's not worth spending more time on. Mentioning it here just because.

But yeah, looks good to me. Would be nice to have @ahf's opinion on fe_channel_is_opchannel and fe_channel_skip_prefix (name and placement), check yesterday's irc logs for context.

@dequis
Copy link
Member

dequis commented Sep 5, 2015

@ahf beep

const char *statusmsg;

/* Quick check */
if (server == NULL || server->prefix[(int)(unsigned char)*target] == 0)
Copy link
Member

Choose a reason for hiding this comment

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

The cast's here deserves a comment in the code. Make a follow up patch which cleans that up a bit please :-)

@ahf
Copy link
Member

ahf commented Sep 9, 2015

01:36:20 < dx> that's from the old code, anyway, not added by lemon

@ahf
Copy link
Member

ahf commented Sep 9, 2015

LGTM, let's get it in.

ahf added a commit that referenced this pull request Sep 9, 2015
Implement CHANTYPES support
@ahf ahf merged commit 7b46dae into irssi:master Sep 9, 2015
@dequis dequis mentioned this pull request Sep 11, 2015
@LemonBoy LemonBoy deleted the chantypes branch December 9, 2015 15:31
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.

None yet

3 participants