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

Change when the autocmds are sent. #399

Merged
merged 2 commits into from Jan 26, 2016

Conversation

Projects
None yet
3 participants
@LemonBoy
Member

LemonBoy commented Jan 14, 2016

As per #175 if a -botcmd is specified for a given channel without a
-bots parameter then the command is sent right after joining the
channel.

Change when the autocmds are sent.
As per #175 if a -botcmd is specified for a given channel without a
-bots parameter then the command is sent right after joining the
channel.
@madduck

This comment has been minimized.

madduck commented on 2502080 Jan 19, 2016

I have not tried this yet (am on holiday), but just looking at the patch, I do have one question:

Couldn't this result in the botcmd being called twice, i.e. once right after join, and then again if the who list is received for whatever reason? Or is it guaranteed that this list doesn't get received in such a way that the signal fires if no -bot is specified?

This comment has been minimized.

Owner

LemonBoy replied Jan 19, 2016

The wholist is actually retrieved when the channel joined signal is fired. So we're sure to execute at least once the function with wholist == false. If the function gets called another time it means the wholist has been retrieved so wholist == true and no duplicate botcmd is sent.

This comment has been minimized.

madduck replied Jan 24, 2016

+1 from me, this seems to fix what it set out to fix. Thanks a lot, @LemonBoy

@ailin-nemui

This comment has been minimized.

Contributor

ailin-nemui commented Jan 19, 2016

mhm, maybe 2 separate signal handlers would be better and more clear

@ailin-nemui

This comment has been minimized.

Contributor

ailin-nemui commented Jan 25, 2016

@madduck could you also re-test this alternate approach?

@madduck

This comment has been minimized.

madduck commented Jan 26, 2016

That also seems to work.

I am not sure I like it better though, as it requires more conditionals in each of the hooks. I feel like the first approach with a little prose in a comment to help people understand might be better.

But using the botmask variable as a "global state" to decide whether to fire autosendcmd in the first or the second hook does does seem like another way to do it, and it's not even unelegant. ;)

ailin-nemui added a commit that referenced this pull request Jan 26, 2016

Merge pull request #399 from LemonBoy/bots
Change when the autocmds are sent.

@ailin-nemui ailin-nemui merged commit c7a3d28 into irssi:master Jan 26, 2016

1 check passed

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

This comment has been minimized.

Contributor

ailin-nemui commented Jan 26, 2016

thanks

@LemonBoy LemonBoy deleted the LemonBoy:bots branch Jul 14, 2016

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