Closed
Bug 955058
Opened 10 years ago
Closed 10 years ago
Unhandled IRC message: 330 RPL_WHOISACCOUNT
Categories
(Chat Core :: IRC, defect)
Chat Core
IRC
Tracking
(Not tracked)
RESOLVED
FIXED
1.3
People
(Reporter: clokep, Assigned: clokep)
Details
Attachments
(1 file, 1 obsolete file)
1.98 KB,
patch
|
aleth
:
review+
|
Details | Diff | Splinter Review |
*** Original post on bio 1628 at 2012-08-09 10:44:00 UTC *** :card.freenode.net 330 clokep Erendur|OFF Erendur :is logged in as :hitchcock.freenode.net 330 testib Enjolras Enjolras :is logged in as The form is: <nick> <authname> :<info> We should ignore this when the names are the same or add a WHOIS entry if they're different ("Alternate name" or "Alias of" or something).
Assignee | ||
Comment 1•10 years ago
|
||
*** Original post on bio 1628 as attmnt 1825 at 2012-08-22 00:42:00 UTC *** This adds the information to the whois information /only/ if the authentication name differs from the nickname.
Attachment #8353584 -
Flags: review?(bugzilla)
Assignee | ||
Updated•10 years ago
|
Attachment #8353584 -
Flags: review?(bugzilla)
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → clokep
Status: NEW → ASSIGNED
Comment 2•10 years ago
|
||
Comment on attachment 8353584 [details] [diff] [review] Patch *** Original change on bio 1628 attmnt 1825 by mook.moz+bugs.instantbird AT gmail.com at 2012-08-22 04:26:19 UTC *** Looks reasonable. Random rambling that can be ignored: 1) Might be nice to put those things in temporary variables for clarity: if (aMessage.params.length == 4) { let [, nick, authname, info] = aMessage.params; if (nick != authname) this.setWhois(nick, {registeredAs: authname}); } return true; 2) Do you want to be able to check what server thinks it's running? (not that I know how to ask this... is it in the spew on logon?)
Attachment #8353584 -
Flags: review?(bugzilla) → review+
Assignee | ||
Comment 3•10 years ago
|
||
*** Original post on bio 1628 at 2012-08-22 10:28:28 UTC *** (In reply to comment #2) > Random rambling that can be ignored: > 1) Might be nice to put those things in temporary variables for clarity: > if (aMessage.params.length == 4) { > let [, nick, authname, info] = aMessage.params; > if (nick != authname) > this.setWhois(nick, {registeredAs: authname}); > } I thought of doing this, it does look nicer...we'll see what aleth says. :) > 2) Do you want to be able to check what server thinks it's running? (not that I > know how to ask this... is it in the spew on logon?) I think one of the things sent to us at the beginning is the server software, I hesitate to check/depend on this for two reasons: 1. It would require us to know exactly what each server supports (it's not like they have lists around...plus there's a ridiculous number of servers). 2. Some don't report a version, so if support was added at a certain version...we wouldn't know. I think because of that it makes more sense to check the contents of the message (as best we can) and then use that. I saw some other source that checked the <info> for a certain string ("is registered as", maybe?) to ensure it's the proper message. We could add that.
Comment 4•10 years ago
|
||
Comment on attachment 8353584 [details] [diff] [review] Patch *** Original change on bio 1628 attmnt 1825 at 2012-08-22 17:16:34 UTC *** This looks good, but I agree it would be more readable to remove the duplication of aMessage.params[1,2] as Mook suggests (but keeping the normalize() where needed of course). Carry the r+ forward if you change this...
Attachment #8353584 -
Flags: review?(bugzilla) → review+
Assignee | ||
Comment 5•10 years ago
|
||
*** Original post on bio 1628 as attmnt 1852 at 2012-08-26 20:38:00 UTC *** Cleans up the code a bit with Mook's suggestion.
Attachment #8353611 -
Flags: review?(bugzilla)
Assignee | ||
Comment 6•10 years ago
|
||
Comment on attachment 8353584 [details] [diff] [review] Patch *** Original change on bio 1628 attmnt 1825 at 2012-08-26 20:38:42 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353584 -
Attachment is obsolete: true
Comment 7•10 years ago
|
||
Comment on attachment 8353611 [details] [diff] [review] Patch v2 *** Original change on bio 1628 attmnt 1852 at 2012-08-26 20:41:03 UTC *** One less unhandled message :)
Attachment #8353611 -
Flags: review?(bugzilla) → review+
Updated•10 years ago
|
Whiteboard: [checkin-needed]
Assignee | ||
Comment 8•10 years ago
|
||
*** Original post on bio 1628 at 2012-08-28 00:53:50 UTC *** Committed as http://hg.instantbird.org/instantbird/rev/17567b2219c1
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
•