Closed Bug 955058 Opened 10 years ago Closed 10 years ago

Unhandled IRC message: 330 RPL_WHOISACCOUNT

Categories

(Chat Core :: IRC, defect)

defect
Not set
minor

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: clokep, Assigned: clokep)

Details

Attachments

(1 file, 1 obsolete file)

*** 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).
Attached patch Patch (obsolete) — Splinter Review
*** 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)
Attachment #8353584 - Flags: review?(bugzilla)
Assignee: nobody → clokep
Status: NEW → ASSIGNED
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+
*** 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 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+
Attached patch Patch v2Splinter Review
*** 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)
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 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+
Whiteboard: [checkin-needed]
*** 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.