TOPIC command doesn't play nicely with send function #98

Closed
qsheets opened this Issue Aug 7, 2012 · 1 comment

Projects

None yet

1 participant

@qsheets
Contributor
qsheets commented Aug 7, 2012

Client.prototype.send automatically inserts a colon before the last argument in a command. However, for the TOPIC command the colon changes the command from checking the topic to setting/erasing the topic.

A colon should only be inserted in this command's line if there is a second argument and the second argument doesn't already begin with a colon.

For more information, see RFC 2812-3.2.4.

@qsheets qsheets closed this Aug 16, 2012
@qsheets qsheets reopened this Aug 16, 2012
@qsheets
Contributor
qsheets commented Aug 16, 2012

My temporary fix is to change lines 605-607 from:

    if ( args[args.length-1].match(/\s/) ) {
        args[args.length-1] = ":" + args[args.length-1];
    }

to:

    if (args[args.length-1] && args[args.length-1] !== ":") {
        args[args.length-1] = ":" + args[args.length-1];
    }

Recommendation

A more permanent fix is to remove this and implement it on a case-by-case basis. As the only commands (of the 46 listed in RFC 2812) that need a colon delimited argument are as follows:

USER
SERVICE *
SQUIT *
PRIVMSG
NOTICE
SQUERY *
KILL
ERROR *
WALLOPS

And commands that have the option to use a colon delimited argument to add a comment or set a message are:

QUIT
PART
TOPIC
KICK
AWAY
USERHOST *

* These commands are rarely (if ever) used by clients

@qsheets qsheets closed this Aug 16, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment