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

Add alternate_nick as a network-specific property #771

Merged
merged 2 commits into from Oct 18, 2017

Conversation

Projects
None yet
3 participants
@paultownsend
Copy link
Contributor

commented Oct 11, 2017

Reference issue #120 - I've added the requested feature and updated the docs to reflect the new parameter.

@ailin-nemui

This comment has been minimized.

Copy link
Contributor

commented Oct 11, 2017

where is it saved to the config?

@paultownsend

This comment has been minimized.

Copy link
Contributor Author

commented Oct 11, 2017

Ah I knew I'd forget something! That was on my list of things to check and I missed it. I'll take a look now.

@@ -4,6 +4,7 @@ int chat_type; /* chat_protocol_lookup(xx) */
char *name;

char *nick;
char *alternate_nick;

This comment has been minimized.

Copy link
@LemonBoy

LemonBoy Oct 11, 2017

Member

This is never freed in chatnet_destroy nor is saved/loaded in chatnet_config_save/chatnet_read.

@@ -69,7 +69,14 @@ static void sig_server_setup_fill_chatnet(IRC_SERVER_CONNECT_REC *conn,
return;
g_return_if_fail(IS_IRCNET(ircnet));

if (ircnet->nick != NULL) g_free_and_null(conn->alternate_nick);
if (ircnet->nick != NULL) {

This comment has been minimized.

Copy link
@LemonBoy

LemonBoy Oct 11, 2017

Member

This is already done in server_setup_fill_chatnet.

This comment has been minimized.

Copy link
@paultownsend

paultownsend Oct 11, 2017

Author Contributor

Should I revert line 72 to what it was or remove it entirely? The check on ircnet->nick and subsequent free on conn->alternate_nick didn't make sense to me, and as you pointed out it's already done elsewhere.

This comment has been minimized.

Copy link
@LemonBoy

LemonBoy Oct 12, 2017

Member

I don't have an answer to that question, sorry.
I think the logic behind the code goes like "if a chatnet specifies nick then try hard to use it instead of the global alternate_nick" and since the latter is now chatnet-specific you can probably just nuke it.

g_free_and_null(conn->nick);
conn->nick = g_strdup(ircnet->nick);
}
if (ircnet->alternate_nick != NULL) {

This comment has been minimized.

Copy link
@LemonBoy

LemonBoy Oct 11, 2017

Member

I think this can be safely moved to server_setup_fill_chatnet if it's not irc-specific.
But if it limited to irc-only the whole alternate_nick field should go in _IRC_CHATNET_REC.

@paultownsend

This comment has been minimized.

Copy link
Contributor Author

commented Oct 11, 2017

Thanks for the feedback and pointers, I'll have to dig into it a bit more.

@paultownsend

This comment has been minimized.

Copy link
Contributor Author

commented Oct 14, 2017

I've dug into it a bit more and from what I can see, the alternate_nick setting is specific to IRC as the only place it's referenced is in event_nick_in_use() in src/core/irc/irc-nicklist.c. As such I've made my changes based on the above feedback to the irc-specific code as advised by @LemonBoy.

@LemonBoy LemonBoy merged commit 28d0b8c into irssi:master Oct 18, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@LemonBoy

This comment has been minimized.

Copy link
Member

commented Oct 18, 2017

@paultownsend thanks for your contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.