Closed
Bug 955581
Opened 10 years ago
Closed 10 years ago
Allow tab completion of nicks which have left the room
Categories
(Instantbird Graveyard :: Conversation, enhancement)
Instantbird Graveyard
Conversation
Tracking
(Not tracked)
RESOLVED
FIXED
1.5
People
(Reporter: aleth, Assigned: aleth)
Details
Attachments
(1 file, 4 obsolete files)
1.51 KB,
patch
|
florian
:
review+
|
Details | Diff | Splinter Review |
*** Original post on bio 2142 at 2013-08-30 11:34:00 UTC *** At least for a short amount of time. I often find myself completing to the wrong nick, because someone has temporarily or recently left the room. But would this change make things better or worse overall?
Comment 1•10 years ago
|
||
*** Original post on bio 2142 at 2013-08-30 12:03:23 UTC *** Seems like Florian has had this issue, I've also had this issue.
OS: Linux → All
Hardware: x86 → All
Comment 2•10 years ago
|
||
*** Original post on bio 2142 at 2013-08-30 12:20:48 UTC *** I had the issue too. I'm not sure if completing left nicks is the right fix though. (As I suspect I sometimes use tab completion to check if someone is around.) But yeah, maybe for a short amount of time :-).
Assignee | ||
Comment 3•10 years ago
|
||
*** Original post on bio 2142 as attmnt 2810 at 2013-08-31 17:26:00 UTC *** We could try this... Random thought: in the long run, it might be nice to recognise when a message is addressed to a parted nick, and memoserv it automatically.
Attachment #8354580 -
Flags: review?(florian)
Assignee | ||
Updated•10 years ago
|
Assignee: nobody → aleth
Status: NEW → ASSIGNED
Comment 4•10 years ago
|
||
*** Original post on bio 2142 at 2013-09-01 21:56:17 UTC *** (In reply to comment #3) > Random thought: in the long run, it might be nice to recognise when a message > is addressed to a parted nick, and memoserv it automatically. This may be a little bit too magical (and half the time if you say something to someone just before he leaves and end up sending the message after he has left, it's not necessarily something worth showing the next day. Eg 'Good night!'). Could be a nice add-on though :).
Comment 5•10 years ago
|
||
Comment on attachment 8354580 [details] [diff] [review] Patch *** Original change on bio 2142 attmnt 2810 at 2013-09-04 07:33:04 UTC *** >diff --git a/instantbird/content/conversation.xml b/instantbird/content/conversation.xml >+ // Add recently parted nicks. >+ const kIncludeNickTimespan = 300000; >+ for (let nick of Object.keys(this.partedBuddies)) Are the keys in this.partedBuddies already sorted by part time? I suspect they are; in which case you may want to consider a binary search, and you can concat a slice of the array, instead of pushing each recently parted nick. >+ if (Date.now() - this.partedBuddies[nick].partTime < kIncludeNickTimespan) What about putting |Date.now() - kIncludeNickTimespan| in a variable before the loop? I'm not sure how important these performance concerns are as I'm not sure how many nicks we will have in the partedBuddies object, but I figure just after a netsplit it may be in the hundreds.
Attachment #8354580 -
Flags: review?(florian) → review-
Assignee | ||
Comment 6•10 years ago
|
||
*** Original post on bio 2142 as attmnt 2817 at 2013-09-04 17:37:00 UTC ***
>Are the keys in this.partedBuddies already sorted by part time? I suspect they
>are; in which case you may want to consider a binary search, and you can concat
>a slice of the array, instead of pushing each recently parted nick.
They appear to be sorted by part time, but do we really want this code to depend on the implementation interna of objects? Even looping over a thousand parted buddies should not take too long in this context I think.
Attachment #8354587 -
Flags: review?(florian)
Assignee | ||
Comment 7•10 years ago
|
||
Comment on attachment 8354580 [details] [diff] [review] Patch *** Original change on bio 2142 attmnt 2810 at 2013-09-04 17:37:21 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8354580 -
Attachment is obsolete: true
Comment 8•10 years ago
|
||
Comment on attachment 8354587 [details] [diff] [review] Patch *** Original change on bio 2142 attmnt 2817 at 2013-09-04 18:01:23 UTC *** >diff --git a/instantbird/content/conversation.xml b/instantbird/content/conversation.xml >@@ -634,16 +634,24 @@ >+ >+ // Add recently parted nicks. >+ const kIncludeNickTimespan = 300000; >+ let cutoffTime = Date.now() - kIncludeNickTimespan; >+ for (let nick of Object.keys(this.partedBuddies)) >+ if (this.partedBuddies[nick].partTime > cutoffTime) >+ completions.push(nick); Nit: { } around the if statement, we only drop the { } if the inner statement is a single line, not a single statement.
Attachment #8354587 -
Flags: review-
Comment 9•10 years ago
|
||
*** Original post on bio 2142 at 2013-09-04 21:26:03 UTC *** (In reply to comment #7) > >+ for (let nick of Object.keys(this.partedBuddies)) > >+ if (this.partedBuddies[nick].partTime > cutoffTime) > >+ completions.push(nick); > Nit: { } around the if statement, we only drop the { } if the inner statement > is a single line, not a single statement. I assume you meant 'for statement'. (In reply to comment #6) > >Are the keys in this.partedBuddies already sorted by part time? I suspect they > >are; in which case you may want to consider a binary search, and you can concat > >a slice of the array, instead of pushing each recently parted nick. > > They appear to be sorted by part time, but do we really want this code to > depend on the implementation interna of objects? I would be really surprised if this 'implementation detail' changed without a lot of prior notices. > Even looping over a thousand > parted buddies should not take too long in this context I think. A binary search may be an excessive complication as yes, it's just js objects without traversing xpconnect wrappers. But looping from the end to search for the oldest nick we are interested in seems reasonable to me. Random thought: would we want to remove from the parted list nicks that have been there for too long?
Assignee | ||
Comment 10•10 years ago
|
||
*** Original post on bio 2142 as attmnt 2824 at 2013-09-05 13:21:00 UTC *** (In reply to comment #8) > Random thought: would we want to remove from the parted list nicks that have > been there for too long? We'd need an interval timer that cleans up the list every hour or day or so. The same would apply to the show nick list. I suppose it might be a good idea if people are lurking on #ubuntu without restarting for weeks. Definitely a separate bug though...
Attachment #8354594 -
Flags: review?(florian)
Assignee | ||
Comment 11•10 years ago
|
||
Comment on attachment 8354587 [details] [diff] [review] Patch *** Original change on bio 2142 attmnt 2817 at 2013-09-05 13:21:30 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8354587 -
Attachment is obsolete: true
Attachment #8354587 -
Flags: review?(florian)
Assignee | ||
Comment 12•10 years ago
|
||
Comment on attachment 8354594 [details] [diff] [review] Patch *** Original change on bio 2142 attmnt 2824 at 2013-09-05 13:28:18 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8354594 -
Flags: review?(florian)
Assignee | ||
Comment 13•10 years ago
|
||
*** Original post on bio 2142 as attmnt 2825 at 2013-09-05 15:19:00 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8354595 -
Flags: review?(florian)
Assignee | ||
Comment 14•10 years ago
|
||
Comment on attachment 8354594 [details] [diff] [review] Patch *** Original change on bio 2142 attmnt 2824 at 2013-09-05 15:19:58 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8354594 -
Attachment is obsolete: true
Assignee | ||
Comment 15•10 years ago
|
||
*** Original post on bio 2142 as attmnt 2826 at 2013-09-05 15:42:00 UTC *** for -> while.
Attachment #8354596 -
Flags: review?(florian)
Assignee | ||
Comment 16•10 years ago
|
||
Comment on attachment 8354595 [details] [diff] [review] Patch *** Original change on bio 2142 attmnt 2825 at 2013-09-05 15:42:43 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8354595 -
Attachment is obsolete: true
Attachment #8354595 -
Flags: review?(florian)
Comment 17•10 years ago
|
||
Comment on attachment 8354596 [details] [diff] [review] Patch *** Original change on bio 2142 attmnt 2826 at 2013-09-05 15:51:13 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8354596 -
Flags: review?(florian) → review+
Assignee | ||
Updated•10 years ago
|
Whiteboard: [checkin-needed]
Comment 18•10 years ago
|
||
*** Original post on bio 2142 at 2013-09-06 00:18:36 UTC *** http://hg.instantbird.org/instantbird/rev/a17743c6cca1
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
•