Closed Bug 955118 Opened 6 years ago Closed 6 years ago

/whois in a private irc conversation should support being parameterless

Categories

(Chat Core :: IRC, enhancement)

enhancement
Not set

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: clokep, Assigned: clokep)

Details

Attachments

(1 file, 4 obsolete files)

*** Original post on bio 1690 at 2012-09-07 15:44:00 UTC ***

Typing /whois without parameter in a private irc conversation to see
more info about the nick talking to you.
Attached patch Patch (obsolete) — Splinter Review
*** Original post on bio 1690 as attmnt 2183 at 2013-01-08 01:06:00 UTC ***

Fairly simple patch for this. If it isn't a chat, we request information on the conversation name.
Attachment #8353945 - Flags: review?(aleth)
Assignee: nobody → clokep
Status: NEW → ASSIGNED
Comment on attachment 8353945 [details] [diff] [review]
Patch

*** Original change on bio 1690 attmnt 2183 at 2013-01-17 11:11:05 UTC ***

Just a nit: If whois was passed multiple parameters, I think we should still return false and not return whois info for the conversation nick, as whatever the user intended, it was not that.
Attachment #8353945 - Flags: review?(aleth) → review-
Attached patch Patch v2 (obsolete) — Splinter Review
*** Original post on bio 1690 as attmnt 2199 at 2013-01-17 11:38:00 UTC ***

This explicitly checks for multiple parameters and returns false. I couldn't think of a way to further reduce the logic in here, but if you can...let me know!
Attachment #8353962 - Flags: review?(aleth)
Comment on attachment 8353945 [details] [diff] [review]
Patch

*** Original change on bio 1690 attmnt 2183 at 2013-01-17 11:38:09 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353945 - Attachment is obsolete: true
Attached patch Patch v3 (obsolete) — Splinter Review
*** Original post on bio 1690 as attmnt 2200 at 2013-01-17 11:58:00 UTC ***

Without the nick.contains, aleth said this won't work until Moz 18 (which my local build is on).
Attachment #8353963 - Flags: review?(aleth)
Comment on attachment 8353962 [details] [diff] [review]
Patch v2

*** Original change on bio 1690 attmnt 2199 at 2013-01-17 11:58:51 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353962 - Attachment is obsolete: true
Attachment #8353962 - Flags: review?(aleth)
Comment on attachment 8353963 [details] [diff] [review]
Patch v3

*** Original change on bio 1690 attmnt 2200 at 2013-01-17 12:03:58 UTC ***

>+      if (nick.indexOf(" ") != 0)
>         return false;
A typo I think...
Attachment #8353963 - Flags: review?(aleth) → review-
Attached patch Patch v3.1 (obsolete) — Splinter Review
*** Original post on bio 1690 as attmnt 2201 at 2013-01-17 12:06:00 UTC ***

Searches for -1, not 0.
Attachment #8353964 - Flags: review?(aleth)
Comment on attachment 8353963 [details] [diff] [review]
Patch v3

*** Original change on bio 1690 attmnt 2200 at 2013-01-17 12:06:09 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353963 - Attachment is obsolete: true
Comment on attachment 8353964 [details] [diff] [review]
Patch v3.1

*** Original change on bio 1690 attmnt 2201 at 2013-01-17 12:07:35 UTC ***

Thanks for this improvement!
Attachment #8353964 - Flags: review?(aleth) → review+
Whiteboard: [checkin-needed]
Attached patch Patch v3.2Splinter Review
*** Original post on bio 1690 as attmnt 2204 at 2013-01-19 04:17:00 UTC ***

Change the property name & doesn't rename aMsg to nick. This should probably get reviewed by someone again.
Attachment #8353967 - Flags: review?
Comment on attachment 8353964 [details] [diff] [review]
Patch v3.1

*** Original change on bio 1690 attmnt 2201 at 2013-01-19 04:17:16 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353964 - Attachment is obsolete: true
Comment on attachment 8353967 [details] [diff] [review]
Patch v3.2

*** Original change on bio 1690 attmnt 2204 at 2013-01-19 10:16:43 UTC ***

Sorry I missed the string change issue.
Attachment #8353967 - Flags: review? → review+
*** Original post on bio 1690 at 2013-01-19 12:37:25 UTC ***

http://hg.instantbird.org/instantbird/rev/8b44e1abd23f
Status: ASSIGNED → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Whiteboard: [checkin-needed]
Target Milestone: --- → 1.4
You need to log in before you can comment on or make changes to this bug.