Closed
Bug 955064
Opened 10 years ago
Closed 10 years ago
The /join IRC command should work without argument when typed in a parted room.
Categories
(Chat Core :: IRC, enhancement)
Chat Core
IRC
Tracking
(Not tracked)
RESOLVED
FIXED
1.3
People
(Reporter: florian, Assigned: aleth)
Details
Attachments
(1 file, 4 obsolete files)
1.17 KB,
patch
|
aleth
:
review+
|
Details | Diff | Splinter Review |
*** Original post on bio 1634 at 2012-08-09 12:41:00 UTC *** As discussed on IRC, this seems to make sense and not be too difficult to implement.
Reporter | ||
Updated•10 years ago
|
Severity: normal → enhancement
OS: Other → All
Hardware: x86 → All
Assignee | ||
Comment 1•10 years ago
|
||
*** Original post on bio 1634 as attmnt 1816 at 2012-08-21 19:58:00 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353575 -
Flags: review?(clokep)
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → aleth
Status: NEW → ASSIGNED
Assignee | ||
Comment 2•10 years ago
|
||
*** Original post on bio 1634 as attmnt 1817 at 2012-08-21 20:01:00 UTC *** Forgot to generate the diff ;)
Attachment #8353576 -
Flags: review?(clokep)
Assignee | ||
Comment 3•10 years ago
|
||
Comment on attachment 8353575 [details] [diff] [review] Patch *** Original change on bio 1634 attmnt 1816 at 2012-08-21 20:01:06 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353575 -
Attachment is obsolete: true
Attachment #8353575 -
Flags: review?(clokep)
Comment 4•10 years ago
|
||
Comment on attachment 8353576 [details] [diff] [review] Patch *** Original change on bio 1634 attmnt 1817 at 2012-08-21 20:08:57 UTC *** >diff --git a/modules/ircCommands.jsm b/modules/ircCommands.jsm >@@ -156,18 +156,23 @@ var commands = [ > name: "join", > run: function(aMsg, aConv) { > let params = aMsg.trim().split(/,\s*/); >- if (!params[0]) >- return false; >+ let conv = getConv(aConv); >+ if (!params[0]) { >+ if (conv.isChat && conv.left) >+ params = [conv.name]; I think you want to use the_chatRoomFields here so it will have a password, etc.: params = [conv._chatRoomFields]; (From http://lxr.instantbird.org/instantbird/source/chat/protocols/irc/ircBase.jsm#336) >+ else >+ return false;
Attachment #8353576 -
Flags: review?(clokep) → review-
Assignee | ||
Comment 5•10 years ago
|
||
*** Original post on bio 1634 at 2012-08-21 20:10:37 UTC *** > I think you want to use the_chatRoomFields here so it will have a password, > etc.: > params = [conv._chatRoomFields]; No, when the user has parted the channel, we purposely delete the chatRoomFields entry. http://lxr.instantbird.org/instantbird/source/chat/protocols/irc/irc.js#224
Comment 6•10 years ago
|
||
*** Original post on bio 1634 at 2012-08-21 20:13:16 UTC *** (In reply to comment #4) > > I think you want to use the_chatRoomFields here so it will have a password, > > etc.: > > params = [conv._chatRoomFields]; > > No, when the user has parted the channel, we purposely delete the > chatRoomFields entry. > http://lxr.instantbird.org/instantbird/source/chat/protocols/irc/irc.js#224 We seem to be doing it to avoid automatic reconnection...but it seems like it'd be wanted for us to keep this information now. Bah...
Assignee | ||
Comment 7•10 years ago
|
||
*** Original post on bio 1634 as attmnt 1818 at 2012-08-21 20:28:00 UTC *** I'm not so sure. It's not like the user told IB to remember the password, and the user explicitly parted. I changed the patch to make use of the stored chatRoomFields info when it is available (that is, when the user did not /part), e.g. if the user was kicked.
Attachment #8353577 -
Flags: review?(clokep)
Assignee | ||
Comment 8•10 years ago
|
||
Comment on attachment 8353576 [details] [diff] [review] Patch *** Original change on bio 1634 attmnt 1817 at 2012-08-21 20:28:44 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353576 -
Attachment is obsolete: true
Assignee | ||
Comment 9•10 years ago
|
||
*** Original post on bio 1634 as attmnt 1819 at 2012-08-21 20:32:00 UTC *** Get rid of some unneccessary nesting.
Attachment #8353578 -
Flags: review?(clokep)
Assignee | ||
Comment 10•10 years ago
|
||
Comment on attachment 8353577 [details] [diff] [review] Patch *** Original change on bio 1634 attmnt 1818 at 2012-08-21 20:32:22 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353577 -
Attachment is obsolete: true
Attachment #8353577 -
Flags: review?(clokep)
Comment 11•10 years ago
|
||
Comment on attachment 8353578 [details] [diff] [review] Patch *** Original change on bio 1634 attmnt 1819 at 2012-08-21 21:42:44 UTC *** This looks good, I think it would be good to have a comment or two in here (about when we expect chatRoomFields to exist and not and what the parameterless version is doing). Carry my r+ forward with the next patch. :)
Attachment #8353578 -
Flags: review?(clokep) → review+
Assignee | ||
Comment 12•10 years ago
|
||
*** Original post on bio 1634 as attmnt 1821 at 2012-08-21 21:56:00 UTC *** Add comment.
Attachment #8353580 -
Flags: review+
Assignee | ||
Comment 13•10 years ago
|
||
Comment on attachment 8353578 [details] [diff] [review] Patch *** Original change on bio 1634 attmnt 1819 at 2012-08-21 21:56:33 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353578 -
Attachment is obsolete: true
Assignee | ||
Updated•10 years ago
|
Whiteboard: [checkin-needed]
Comment 14•10 years ago
|
||
*** Original post on bio 1634 at 2012-08-22 00:11:08 UTC *** Committed as http://hg.instantbird.org/instantbird/rev/4286f8fd2621 with a slight modification by flo before check-in. (Moving let conv... to inside the if statement.)
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Whiteboard: [checkin-needed]
Target Milestone: --- → 1.3
You need to log in
before you can comment on or make changes to this bug.
Description
•