Closed Bug 954082 Opened 10 years ago Closed 10 years ago

Username split for JavaScript protocols

Categories

(Chat Core :: General, defect)

defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: clokep, Assigned: clokep)

References

Details

Attachments

(1 file, 1 obsolete file)

*** Original post on bio 647 at 2011-01-07 02:12:00 UTC ***

From comment in bug 953932 (bio 495):
> >   getUsernameSplit: function() EmptyEnumerator,
> Shouldn't this be implemented like getOptions? May be better to handle
> UsernameSplit in another bug if you want to get the options part landed soon.
> Doesn't matter if you already depend on both though :).
Blocks: 953944
Attached patch Patch v1.0 (obsolete) — Splinter Review
*** Original post on bio 647 as attmnt 470 at 2011-01-07 02:52:00 UTC ***

This implements a username split similar to how the preference lists are being implemented in bug 953932 (bio 495).  Note that this patch won't apply at the same time as the one in bug 953932 (bio 495).
Attachment #8352213 - Flags: review?(florian)
*** Original post on bio 647 at 2011-01-14 11:45:50 UTC ***

Comment on attachment 8352213 [details] [diff] [review] (bio-attmnt 470)
Patch v1.0

>diff -r ba17c12c2206 purple/purplexpcom/src/jsProtoHelper.jsm

>+  getUsernameSplit: function() {
>+    if (!this.usernameSplits || !this.usernameSplits.length)
>+      return EmptyEnumerator;
>+
>+    return new nsSimpleEnumerator(this.usernameSplits.map(function(value)
>+      new UsernameSplit(value.label,
>+                        value.separator,
>+                        value.defaultServer,
>+                        value.reverse)

Do we really want to break the object into its various values to recombine it later into another very similar object? It seems to be offering many opportunities for typos ;).

>+    ));
>+  },
>   // NS_ERROR_XPC_JSOBJECT_HAS_NO_FUNCTION_NAMED errors are too noisy
>   getOptions: function() EmptyEnumerator,
>-  getUsernameSplit: function() EmptyEnumerator,
>   get usernameEmptyText() "",
>   accountExists: function() false, //FIXME
> 
>diff -r ba17c12c2206 purple/purplexpcom/src/jsTestProtocol.js
>--- a/purple/purplexpcom/src/jsTestProtocol.js	Thu Jan 06 16:48:25 2011 +0100
>+++ b/purple/purplexpcom/src/jsTestProtocol.js	Thu Jan 06 21:49:58 2011 -0500
>@@ -98,6 +98,10 @@
> function jsTestProtocol() { }
> jsTestProtocol.prototype = {
>   get name() "JS Test",
>+  usernameSplits: [
>+    {"label": "Server", "separator": "@", "defaultServer": "default.server",
>+     "reverse": true}

I assume you meant defaultValue here?
The " " are not needed around the keys.
*** Original post on bio 647 as attmnt 476 at 2011-01-14 11:51:00 UTC ***

Your patch modified to illustrate what I meant in comment 2. Let me know what you think of it.

We will need to look relatively soon into localization issues.
Attachment #8352219 - Flags: review?(clokep)
Comment on attachment 8352219 [details] [diff] [review]
Patch v1.0 (modified)

*** Original change on bio 647 attmnt 476 at 2011-01-14 14:02:49 UTC ***

Looks good.  Although I might have directly returned the splits object from the map.
Attachment #8352219 - Flags: review?(clokep) → review+
*** Original post on bio 647 at 2011-01-14 14:26:10 UTC ***

(In reply to comment #4)
> (From update of attachment 8352219 [details] [diff] [review] (bio-attmnt 476) [details])
> Looks good.  Although I might have directly returned the splits object from the
> map.

Thanks.
After some more bikesheding over IRC, the line now looks like:
    return new nsSimpleEnumerator(
      this.usernameSplits.map(function(split) new UsernameSplit(split)));
Comment on attachment 8352213 [details] [diff] [review]
Patch v1.0

*** Original change on bio 647 attmnt 470 at 2011-01-14 17:12:21 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8352213 - Attachment is obsolete: true
Attachment #8352213 - Flags: review?(florian)
*** Original post on bio 647 at 2011-01-14 18:06:52 UTC ***

Checked-in as: http://hg.instantbird.org/instantbird/rev/a6c8fbf77e10
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → 0.3a1
You need to log in before you can comment on or make changes to this bug.