Closed Bug 954828 Opened 10 years ago Closed 10 years ago

[Tab Complete] Do not allow a nick to be completed multiple times

Categories

(Instantbird Graveyard :: Conversation, enhancement)

enhancement
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: clokep, Assigned: aleth)

Details

Attachments

(1 file, 1 obsolete file)

*** Original post on bio 1393 at 2012-04-24 11:58:00 UTC ***

I ran into this while testing the new nick completion code. If there are multiple possible completions, pressing tab cycles through them, but we should filter out nicks that are already completed. E.g. you have nick1 and nick2 in a room.

"ni"<tab> --> "nick"<I type 1 and press tab again> --> "nick1: "<I type ni and press tab> --> "nick1, nick"

I think in this case we should immediately resolve to nick2, as you've already used nick1! (Pretty much it's just filtering out previous completions.)
Attached patch Patch (obsolete) — Splinter Review
*** Original post on bio 1393 as attmnt 1392 at 2012-04-24 19:54:00 UTC ***

- Removes nicks in a multiple completion from the list of possible completions while adding to that list of nicks (so it is still possible to use/complete those nicks in the rest of the message)
Attachment #8353145 - Flags: review?(clokep)
Assignee: nobody → aleth
*** Original post on bio 1393 at 2012-04-24 19:59:10 UTC ***

Comment on attachment 8353145 [details] [diff] [review] (bio-attmnt 1392)
Patch

>diff --git a/chrome/instantbird/content/instantbird/conversation.xml b/chrome/instantbird/content/instantbird/conversation.xml
>index 69192d2..0b54d3a 100644
>--- a/chrome/instantbird/content/instantbird/conversation.xml
>+++ b/chrome/instantbird/content/instantbird/conversation.xml
>+                  // Remove preceding completions from possible completions.
>+                  preceding.forEach(function(c, i, arr) {
>+                    let j = completions.indexOf(c);
>+                    if (j != -1)
>+                      completions.splice(j, 1);
>+                  });
This looks like it wants to use preceding.filter :)
*** Original post on bio 1393 at 2012-04-24 20:17:53 UTC ***

(In reply to comment #2)
> This looks like it wants to use preceding.filter :)

You can't use filter, because all of the elements of preceding are in completions initially. But we don't want to try to remove the same nick twice. Or were you thinking of something more subtle?
*** Original post on bio 1393 at 2012-04-24 20:23:47 UTC ***

or did you mean completions = completions.filter(function(c) preceding.indexOf(c) != -1); ?
*** Original post on bio 1393 at 2012-04-24 20:25:02 UTC ***

(In reply to comment #4)
> or did you mean completions = completions.filter(function(c)
> preceding.indexOf(c) != -1); ?

Yes.
Attached patch PatchSplinter Review
*** Original post on bio 1393 as attmnt 1398 at 2012-04-24 21:12:00 UTC ***

Yes, that's better, thanks.
Attachment #8353151 - Flags: review?(clokep)
Comment on attachment 8353145 [details] [diff] [review]
Patch

*** Original change on bio 1393 attmnt 1392 at 2012-04-24 21:12:25 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8353145 - Attachment is obsolete: true
Attachment #8353145 - Flags: review?(clokep)
Comment on attachment 8353151 [details] [diff] [review]
Patch

*** Original change on bio 1393 attmnt 1398 at 2012-04-24 21:34:12 UTC ***

Works as advertised! :)
Attachment #8353151 - Flags: review?(clokep) → review+
*** Original post on bio 1393 at 2012-04-24 21:34:35 UTC ***

Assigning to aleth.
Status: NEW → ASSIGNED
Whiteboard: [checkin-needed]
*** Original post on bio 1393 at 2012-04-24 22:50:33 UTC ***

Checked in as http://hg.instantbird.org/instantbird/rev/6042f2575b43 thanks!
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → 1.2
Whiteboard: [checkin-needed]
You need to log in before you can comment on or make changes to this bug.