Closed
Bug 955442
Opened 10 years ago
Closed 10 years ago
The /invite command should support taking more than one nick as parameter
Categories
(Chat Core :: IRC, enhancement)
Chat Core
IRC
Tracking
(Not tracked)
RESOLVED
FIXED
1.5
People
(Reporter: florian, Assigned: clokep)
Details
Attachments
(1 file, 2 obsolete files)
6.31 KB,
patch
|
aleth
:
review+
|
Details | Diff | Splinter Review |
*** Original post on bio 2006 at 2013-06-17 10:54:00 UTC *** Suggested syntax: /invite <nick>[ <nick>]* [<channel>] We can just check the last parameter to see if it's a channel or not.
Assignee | ||
Comment 1•10 years ago
|
||
*** Original post on bio 2006 as attmnt 2519 at 2013-06-26 10:55:00 UTC *** I debated whether the syntax should be <nick> <nick> <channel> or <nick>,<nick> <channel>, but the first places better w/ tab complete. Untested (my build is busted).
Attachment #8354287 -
Flags: review?(aleth)
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → clokep
Status: NEW → ASSIGNED
Reporter | ||
Comment 2•10 years ago
|
||
*** Original post on bio 2006 at 2013-06-26 11:09:01 UTC *** Comment on attachment 8354287 [details] [diff] [review] (bio-attmnt 2519) Patch v1 >-command.invite=%S <nick> [<room>]: Invite someone to join you in the specified channel, or the current channel. >+command.invite2=%S <nick>[ <nick>]* [<room>]: Invite someone to join you in the specified channel, or the current channel. What about: Invite one or more nicks to join you in the current channel, or to join the specified channel. Do we want the word "room" or "channel"? The syntax says "room" but the help sentence says "channel" :-S. >- return simpleCommand(aConv, "INVITE", params); >+ // Default to using the current conversation as the channel to invite to. >+ let channel = aConv.name; >+ let account = getAccount(aConv); >+ // Find the first param that could be a channel name. >+ for (let i = 0; i < params.length; ++i) { >+ if (account.isMUCName(params[i])) { >+ // Remove that parameter and store it. >+ channel = params.splice(i, 1); >+ break; >+ } >+ } A few edge cases that don't seem to be handled here: [1] /invite #channel [2] /invite nick #channel1 #channel2 [1] Is obviously a wrong syntax. [2] could likely be supported, but I don't think it's worth it. In both cases I think we could just return false. I think the way I would implement this check is: - if the last parameters looks like a MUC name, use it as the channel and remove it from the list (channel = params.splice) - check all remaining parameters; if any of them is a MUC name, or if the list is empty, return false. (This assumes that isMUCName doesn't have false positives.)
Comment 3•10 years ago
|
||
Comment on attachment 8354287 [details] [diff] [review] Patch v1 *** Original change on bio 2006 attmnt 2519 at 2013-06-26 12:03:30 UTC *** The help string could be improved (I think flo already has a suggestion for that), but the code looks fine to me. (I didn't test it though)
Attachment #8354287 -
Flags: review?(aleth) → review+
Updated•10 years ago
|
Whiteboard: [checkin-needed]
Assignee | ||
Comment 4•10 years ago
|
||
*** Original post on bio 2006 at 2013-06-26 12:04:34 UTC *** Florian already gave some good suggestions and we're not going to check in an untested patch.
Whiteboard: [checkin-needed]
Comment 5•10 years ago
|
||
*** Original post on bio 2006 at 2013-06-26 12:06:21 UTC *** (In reply to comment #2) I thought isMUCName could have false positives (i.e. it was possible to have unprefixed channel names on some servers), so there is no perfect way to handle this?
Whiteboard: [checkin-needed]
Comment 6•10 years ago
|
||
Comment on attachment 8354287 [details] [diff] [review] Patch v1 *** Original change on bio 2006 attmnt 2519 at 2013-06-26 12:13:44 UTC *** >I thought isMUCName could have false positives (i.e. it was possible to have >unprefixed channel names on some servers), so there is no perfect way to handle >this? So I was completely wrong about that, in which case this can be improved a lot :)
Attachment #8354287 -
Flags: review+ → review-
Updated•10 years ago
|
Whiteboard: [checkin-needed]
Assignee | ||
Comment 7•10 years ago
|
||
*** Original post on bio 2006 as attmnt 2528 at 2013-06-28 01:12:00 UTC *** This changes the behavior to reject situations where there are multiple channels given or if there are no parameters besides the channel. It also handles another error message, when you invite someone to a channel they're already in.
Attachment #8354296 -
Flags: review?(aleth)
Assignee | ||
Comment 8•10 years ago
|
||
Comment on attachment 8354287 [details] [diff] [review] Patch v1 *** Original change on bio 2006 attmnt 2519 at 2013-06-28 01:12:18 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8354287 -
Attachment is obsolete: true
Comment 9•10 years ago
|
||
Comment on attachment 8354296 [details] [diff] [review] Patch v2 *** Original change on bio 2006 attmnt 2528 at 2013-06-29 08:28:09 UTC *** > "443": function(aMessage) { // ERR_USERONCHANNEL > // <user> <channel> :is already on channel >- // TODO >+ this.getConversation(aMessage.params[2]) >+ .writeMessage(aMessage.servername, >+ _("message.alreadyInChannel", aMessage.params[1], >+ aMessage.params[2]), {system: true}); > return false; I think you want to return true here. >+ // Remove that parameter and store it. >+ channel = params.splice(i, 1)[0]; >+ break; If you break here, you will never detect the presence of two channel names.
Attachment #8354296 -
Flags: review?(aleth) → review-
Assignee | ||
Comment 10•10 years ago
|
||
*** Original post on bio 2006 as attmnt 2539 at 2013-06-29 15:44:00 UTC *** Thanks aleth, both silly mistakes.
Attachment #8354307 -
Flags: review?(aleth)
Assignee | ||
Comment 11•10 years ago
|
||
Comment on attachment 8354296 [details] [diff] [review] Patch v2 *** Original change on bio 2006 attmnt 2528 at 2013-06-29 15:44:54 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8354296 -
Attachment is obsolete: true
Comment 12•10 years ago
|
||
Comment on attachment 8354307 [details] [diff] [review] Patch v3 *** Original change on bio 2006 attmnt 2539 at 2013-06-29 15:48:00 UTC *** Thanks!
Attachment #8354307 -
Flags: review?(aleth) → review+
Updated•10 years ago
|
Whiteboard: [checkin-needed]
Assignee | ||
Comment 13•10 years ago
|
||
*** Original post on bio 2006 at 2013-06-30 13:03:59 UTC *** http://hg.instantbird.org/instantbird/rev/cd5947be952a
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Whiteboard: [checkin-needed]
Target Milestone: --- → 1.5
You need to log in
before you can comment on or make changes to this bug.
Description
•