[polari/wip/raresv/userTracker] userTracker part 3

[Rares Visalom <raresvisalom src gnome org>]

commit ae327362f55d9e14c8ae8f86c9290fd551f55c75
Author: raresv <rares visalom gmail com>
Date:   Tue Jun 28 15:47:08 2016 +0300

    userTracker part 3

 src/chatView.js    |    6 +-
 src/userTracker.js |  172 ++++++++-------------------------------------------
 2 files changed, 29 insertions(+), 149 deletions(-)
---
diff --git a/src/chatView.js b/src/chatView.js
index acce37e..a3f071a 100644
--- a/src/chatView.js
+++ b/src/chatView.js
@@ -289,8 +289,8 @@ const ChatView = new Lang.Class({
         this._pendingLogs = [];
         this._statusCount = { left: 0, joined: 0, total: 0 };
 
-        this._userTracker = new UserTracker.UserTracker({ room: this._room });
-        this._userTracker.connect('status-changed', Lang.bind(this, function(tracker, nick, room, status){
+        this._userTracker = new UserTracker.UserTracker(this._room);
+        this._userTracker.connect('status-changed', Lang.bind(this, function(tracker, nick, status){
             //log("status-changed signal received in chatView. User " + nick + " has status: " + status);
             this._updateTagStatus(nick);
         }));
@@ -818,8 +818,6 @@ const ChatView = new Lang.Class({
                 this._updateTagStatus(this._channel.connection.self_contact.alias);
                 this._updateTagStatus(this._channel.target_contact.alias);
             }
-        } else {
-            this._userTracker.resetTracker();
         }
 
         if (!this._channel)
diff --git a/src/userTracker.js b/src/userTracker.js
index 118ac9c..e29d863 100644
--- a/src/userTracker.js
+++ b/src/userTracker.js
@@ -2,64 +2,31 @@ const Polari = imports.gi.Polari;
 const Lang = imports.lang;
 const Tp = imports.gi.TelepathyGLib;
 const Signals = imports.signals;
-/* unused imports: */
-const GObject = imports.gi.GObject;
-const Gtk = imports.gi.Gtk;
 
 
 const UserTracker = new Lang.Class({
     Name: 'UserTracker',
 
-    _init: function(params) {
+    _init: function(room) {
         this._contactMapping = new Map();
 
-        /* not a widget, simply passing room */
-        if (params.room) {
-            this._room = params.room;
+        if (room) {
+            this._room = room;
+
             this._room.connect('notify::channel', Lang.bind(this, this._onChannelChanged));
+            this._room.connect('member-renamed', Lang.bind(this, this._onMemberRenamed));
+            this._room.connect('member-disconnected', Lang.bind(this, this._onMemberDisconnected));
+            this._room.connect('member-kicked', Lang.bind(this, this._onMemberKicked));
+            this._room.connect('member-banned', Lang.bind(this, this._onMemberBanned));
+            this._room.connect('member-joined', Lang.bind(this, this._onMemberJoined));
+            this._room.connect('member-left', Lang.bind(this, this._onMemberLeft));
 
             this._onChannelChanged();
         }
-        else {
-            /* we decided that separate local/global trackers probably doesn't work, so don't mention it for 
now */
-            //TODO: global user tracker
-        }
     },
 
     _onChannelChanged: function() {
         if (this._room.channel) {
-            if (this._room.type == Tp.HandleType.ROOM) {
-                let members = this._room.channel.group_dup_members_contacts();
-
-                //this._contactMapping = this._buildMapping(members);
-
-                /* let instead of var */
-                for (var i = 0; i < members.length; i++)
-                    this._trackMember(members[i]);
-
-                /* those can be done in _init() - otherwise we'd need to disconnect the signals to avoid 
adding contacts repeatedly */
-                this._room.connect('member-renamed', Lang.bind(this, this._onMemberRenamed));
-                this._room.connect('member-disconnected', Lang.bind(this, this._onMemberDisconnected));
-                this._room.connect('member-kicked', Lang.bind(this, this._onMemberKicked));
-                this._room.connect('member-banned', Lang.bind(this, this._onMemberBanned));
-                this._room.connect('member-joined', Lang.bind(this, this._onMemberJoined));
-                this._room.connect('member-left', Lang.bind(this, this._onMemberLeft));
-            } else {
-                let members = [this._room.channel.connection.self_contact, 
this._room.channel.target_contact];
-                /* ignored return value, this does nothing */
-                this._buildMapping(members);
-            }
-        } else {
-            /* unnecessary check since _contactMapping creation was moved to _init */
-            if(this._contactMapping) {
-                this._contactMapping.clear();
-
-                //this._room.disconnect('member-joined');
-            }
-        }
-
-        /* with this._contactMapping initialization and signal connections in _init(), this is cleaner:
-        if (this._room.channel) {
             let members;
             if (this._room.type == Tp.HandleType.ROOM)
                 members = this._room.channel.group_dup_members_contacts();
@@ -68,30 +35,17 @@ const UserTracker = new Lang.Class({
 
             members.forEach(m => { this._trackMember(m); });
         } else {
-            this._contactMapping.clear();
-        }
-        */
-    },
-
-    /* scrap that, using _trackMember() is cleaner */
-    _buildMapping: function(members) {
-        let map = new Map();
+            for ([, basenickContacts] of this._contactMapping) {
+                basenickContacts.forEach(Lang.bind(this, function(member) {
+                    this._untrackMember(member);
+                }));
+            }
 
-        for (var i = 0; i < members.length; i++) {
-            let currentBasenick = Polari.util_get_basenick(members[i].alias);
-            if (map.has(currentBasenick))
-                map.get(currentBasenick).push(members[i]);
-            else
-                map.set(currentBasenick, [members[i]]);
+            this._contactMapping.clear();
         }
-
-        //log(map.get("raresv").length);
-
-        return map;
     },
 
     _onMemberRenamed: function(room, oldMember, newMember) {
-        log("rename " + oldMember.alias + " to " + newMember.alias);
         this._untrackMember(oldMember);
         this._trackMember(newMember);
     },
@@ -119,19 +73,13 @@ const UserTracker = new Lang.Class({
     _trackMember: function(member) {
         let baseNick = Polari.util_get_basenick(member.alias);
 
-        /* nit: no braces */
-        if (this._contactMapping.has(baseNick)) {
+        if (this._contactMapping.has(baseNick))
             this._contactMapping.get(baseNick).push(member);
-        } else {
+        else
             this._contactMapping.set(baseNick, [member]);
-        }
 
-        /* This is equivalent to "this.emit('status-changed', ..., Tp.ConnectionPresenceType.AVAILABLE);",
-           which isn't what we want - we want something like:
-           if (this._contactMapping.size() == 1)
-               this.emit('status-changed', ..., Tp.ConnectionPresenceType.AVAILABLE);
-        */
-        this._updateStatus(member);
+        if (this._contactMapping.get(baseNick).length == 1)
+            this.emit('status-changed', member.alias, Tp.ConnectionPresenceType.AVAILABLE);
     },
 
     _untrackMember: function(member) {
@@ -143,8 +91,8 @@ const UserTracker = new Lang.Class({
             if (indexToDelete > -1) {
                 this._contactMapping.get(baseNick).splice(indexToDelete, 1);
 
-                /* I'd not split this into a separate function for now (see comment in _trackMember) */
-                this._updateStatus(member);
+                if (this._contactMapping.get(baseNick).length == 0)
+                    this.emit('status-changed', member.alias, Tp.ConnectionPresenceType.OFFLINE);
             }
         }
     },
@@ -152,85 +100,19 @@ const UserTracker = new Lang.Class({
     _updateStatus: function(member) {
         let baseNick = Polari.util_get_basenick(member.alias);
 
-        if (this._contactMapping.has(baseNick)) {
-            /* Nit: no braces */
-            if (this._contactMapping.get(baseNick).length == 0) {
-                /* some thoughts on parameters:
-                    - while we only implement the local tracker, the room parameter looks a bit pointless
-                    - I'm wondering whether member.alias or baseNick makes more sense
-                */
-                this.emit('status-changed', member.alias, this._room, Tp.ConnectionPresenceType.OFFLINE);
-            } else {
-                this.emit('status-changed', member.alias, this._room, Tp.ConnectionPresenceType.AVAILABLE);
-            }
-        }
+        if (this._contactMapping.has(baseNick))
+            if (this._contactMapping.get(baseNick).length == 0)
+                this.emit('status-changed', member.alias, Tp.ConnectionPresenceType.OFFLINE);
+            else
+                this.emit('status-changed', member.alias, Tp.ConnectionPresenceType.AVAILABLE);
     },
 
     getNickStatus: function(nickName) {
         let baseNick = Polari.util_get_basenick(nickName);
 
-        /* reads easier IMHO with less nesting:
-        if (this._contactMapping.has(baseNick) &&
-            this._contactMapping.get(baseNick).length > 0)
-            return Tp.ConnectionPresenceType.AVAILABLE;
-        return Tp.ConnectionPresenceType.OFFLINE;
-
-        Or maybe even:
         let contacts = this._contactMapping.get(baseNick) || [];
         return contacts.length == 0 ? Tp.ConnectionPresenceType.OFFLINE
                                     : Tp.ConnectionPresenceType.AVAILABLE;
-        */
-        if (this._contactMapping.has(baseNick)) {
-            if (this._contactMapping.get(baseNick).length == 0) {
-                return Tp.ConnectionPresenceType.OFFLINE;
-            } else {
-                return Tp.ConnectionPresenceType.AVAILABLE;
-            }
-        } else {
-            return Tp.ConnectionPresenceType.OFFLINE;
-        }
     },
-
-    /* I don't like this:
-       - all tracked contacts change status when we disconnect,
-         but with this we expect all tracker users to keep track
-         of the channel status to call this function themselves
-         (compare to the connect-case, where users don't need to
-          call a trackInitialMembers() function to get status-changed
-          signals for connected nicks)
-       - it relies on obscure implementation details:
-         - our own ::notify::channel handler wipes all mappings
-         - resetTracker called from some other ::notify::channel
-           handler uses the mapping to emit 'status-changed' signals
-         => if our own handler is called first, resetTracker() does not
-            work as expected
-
-        So no, this is bad API - the right place to do this is _onChannelChanged
-        */
-    resetTracker: function() {
-        if (this._contactMapping) {
-            this._contactMapping.forEach(function(value, key, map){
-                let basenickContacts = value;
-
-                basenickContacts.forEach(function(member){
-                    this._untrackMember(member);
-                });
-            });
-
-            this._contactMapping.clear();
-
-        }
-    },
-
-    /* unused */
-    resetBasenickMembers: function(basenick) {
-        if (this._contactMapping.has(basenick)) {
-            let basenickContacts = this._contactMapping.get(basenick);
-
-            basenickContacts.forEach(function(member){
-                    this._untrackMember(member);
-            });
-        }
-    }
 });
 Signals.addSignalMethods(UserTracker.prototype);