Created attachment 181379 [details] [review]
telepathyClient: Fix two minor typos

Created attachment 181380 [details] [review]
messageTray: Add ability to suppress 'notify' signal

This allows clients to update the title without interrupting
the user for a minor update.

Created attachment 181381 [details] [review]
telepathyClient: Add alias change notifiers

Update the Source title when an contact's alias changes, and
also also add a minor meta message like the current timestamps.

Updating the alias of a 'presenced' contact will overwrite the
current title, and it will also not update the summary item title
right now due to limitations of the message tray.

Created attachment 181382 [details] [review]
telepathyClient: Re-open existing chat sources on Shell restart

This allows users to continue a chat they were having after
the shell is restarted. We don't hook up to TelepathyLogger, so
there's no chat history.

Created attachment 181383 [details] [review]
telepathyClient: Support the ACTION message type

This message type is commonly done in IM and IRC clients by the
command '/me'. Currently, we don't add the ALIAS feature to our
own contact, so we fall back to the identifier for messages with
the shell's user actions, which looks ugly.

Created attachment 181384 [details] [review]
history: Fix up HistoryManager.

Fix a few typos, and make gsettings saving support optional.

Created attachment 181385 [details] [review]
telepathyClient: Add history navigation to entry

This adds a non-saving history to each notification entry,
like runDialog and lookingGlass.

This is very incomplete, so tagging ui-review to see if it can or should be squeezed in for the freeze on Monday. I should be quite responsive to reviews today and tomorrow.

Review of attachment 181380 [details] [review]:

If you want to update the notification without actually notifying the user, you can call .update() on the Notification object. No need to patch .notify(), whose purpose is exactly to notify the user of something.

Created attachment 181521 [details] [review]
messageTray: Add ability to suppress 'notify' signal

This allows clients to update the title without interrupting
the user for a minor update.

Created attachment 181522 [details] [review]
messageTray: Add ability to suppress notification popup

This allows clients to update the title or add a source
without inconveniencing the user for a minor update.



Fixed commit message to more accurate.

Comment on attachment 181379 [details] [review]
telepathyClient: Fix two minor typos

Attachment 181379 [details] pushed as 97bb5b6 - telepathyClient: Fix two minor typos

Created attachment 181980 [details] [review]
lookingGlass: Add TelepathyGLib to the default set of imports

Created attachment 181981 [details] [review]
lookingGlass: Add TelepathyGLib to the default set of imports

Created attachment 181982 [details] [review]
messageTray: Add ability to suppress notification popup

This allows clients to update the title or add a source
without inconveniencing the user for a minor update.

Created attachment 181983 [details] [review]
telepathyClient: Add alias change notifiers

Update the Source title when an contact's alias changes, and
also also add a minor meta message like the current timestamps.

Updating the alias of a 'presenced' contact will overwrite the
current title, and it will also not update the summary item title
right now due to limitations of the message tray.

Created attachment 181984 [details] [review]
telepathyClient: Re-open existing chat sources on Shell restart

This allows users to continue a chat they were having after
the shell is restarted. We don't hook up to TelepathyLogger, so
there's no chat history.

Created attachment 181985 [details] [review]
history: Fix up HistoryManager

Make GSettings support optional, and fix a couple other typos
and bugs.

Created attachment 181986 [details] [review]
telepathyClient: Add history navigation to entry

This adds a non-saving history to each notification entry,
like runDialog and lookingGlass.

Created attachment 181987 [details] [review]
shell-global: Add shell_get_self_contact_features

This is another workaround for the lack of gjs supporting array
arguments, this time wrapping tp_connection_upgrade_contacts to
add new features to the connection's self contact.

Created attachment 181988 [details] [review]
telepathyClient: Add support for aliases on the self-contact

Upgrade the connection's self contact to allow aliases, so that we
don't show the internal telepathy identifier anywhere user-visible.

Created attachment 181989 [details] [review]
telepathyClient: Add support for the ACTION message type

This message type is usually supported by the '/me' command
in IRC and IM clients.

Comment on attachment 181981 [details] [review]
lookingGlass: Add TelepathyGLib to the default set of imports

it seems like it would be better to have lg automatically import everything that's imported anywhere else... but I guess this is fine

Comment on attachment 181982 [details] [review]
messageTray: Add ability to suppress notification popup

>This allows clients to update the title or add a source
>without inconveniencing the user for a minor update.

The former is what update() does, and the latter is not currently wanted in the design.

Comment on attachment 181983 [details] [review]
telepathyClient: Add alias change notifiers

>Updating the alias of a 'presenced' contact will overwrite the

not sure what you mean by 'presenced'

>current title, and it will also not update the summary item title
>right now due to limitations of the message tray.

you should fix that

>+        let message = _("<i>%s is now known as %s</i>").format(oldAlias, newAlias);

when possible, avoid putting markup in translatable strings. (ie, use '<i>' + _(...)

also, a comment for translators would be good here (explaining that the two strings are IM screen names).

Comment on attachment 181984 [details] [review]
telepathyClient: Re-open existing chat sources on Shell restart

>+        this.notify(true);

ah... So this recreates the source and puts it back into the summary without displaying the notification as a notification.

"this.notify(true)" is a bad way of expressing that idea though. Maybe something like "this._setNotification(notification)" (which Source.notify() would use internally.)

> We don't hook up to TelepathyLogger, so there's no chat history.

maybe this would be better as part of that bug then.

Comment on attachment 181985 [details] [review]
history: Fix up HistoryManager

>Make GSettings support optional, and fix a couple other typos
>and bugs.

the gsettings stuff is definitely right (and if you want to split that into a separate patch, it's accepted-commit_now)

Not sure about the fixes; even with the fixes, it seems that this._historyIndex can range from 0 to this._history.length, and that seems weird. (Eg, if there is 1 item in _history, _historyIndex can have 2 different values.) I am not familiar with this code though.

Comment on attachment 181986 [details] [review]
telepathyClient: Add history navigation to entry

>This adds a non-saving history to each notification entry,
>like runDialog and lookingGlass.

*suspiciously* like runDialog and lookingGlass... How about adding HistoryManager.connectToEntry() to get rid of the duplicated code between them?

Comment on attachment 181987 [details] [review]
shell-global: Add shell_get_self_contact_features

>This is another workaround for the lack of gjs supporting array
>arguments

"non-NULL-terminated array arguments in callbacks", I believe?

>+static void
>+shell_global_get_self_contact_features_cb (TpConnection *connection,
>+                                           guint n_contacts,
>+                                           TpContact * const *contacts,
>+                                           const GError *error,
>+                                           gpointer user_data,
>+                                           GObject *weak_object)
>+{
>+  TpContact *self_contact = *contacts;
>+  ((ShellGetSelfContactFeaturesCb)user_data)(connection, self_contact);
>+}

Presumably since there's a GError there, this can fail, so you should check n_contacts before dereferencing contacts.

>+ * Wrap tp_connection_upgrade_contacts due to the lack of support for
>+ * proper arrays arguments in GJS.

as in the commit message, please be more precise about exactly what gjs doesn't like about this function/callback, so that we can know when the wrapper can go away.

(In reply to comment #25)
> (From update of attachment 181983 [details] [review])
> >Updating the alias of a 'presenced' contact will overwrite the
> 
> not sure what you mean by 'presenced'

When a contact has certain presences (the ones that set shouldNotify), it also changes the title to something like: "Jasper is offline". I'm saying that this will override that status, because we don't keep the presence around to rebuild the messages. I'll fix that in the next update.

> >current title, and it will also not update the summary item title
> >right now due to limitations of the message tray.
> 
> you should fix that

I've looked into it, but I'd rather it be a separate bug, since the current system isn't really designed for it.

> >+        let message = _("<i>%s is now known as %s</i>").format(oldAlias, newAlias);

OK.

> when possible, avoid putting markup in translatable strings. (ie, use '<i>' +
> _(...)
> 
> also, a comment for translators would be good here (explaining that the two
> strings are IM screen names).

OK.

Comment on attachment 181988 [details] [review]
telepathyClient: Add support for aliases on the self-contact

>Upgrade the connection's self contact to allow aliases, so that we
>don't show the internal telepathy identifier anywhere user-visible.

Explain more? I don't understand how/why this code has that effect.

(In reply to comment #31)
> (From update of attachment 181988 [details] [review])
> >Upgrade the connection's self contact to allow aliases, so that we
> >don't show the internal telepathy identifier anywhere user-visible.
> 
> Explain more? I don't understand how/why this code has that effect.

This is an annoying bug. By default, the Telepathy 'self_contact' that's created doesn't have any features, which means that trying to get the alias will fall back to the internal Telepathy ID, which is completely ugly.

We upgrade the contact so we can add those features, and show the proper name instead of the ugly internal ID.

Comment on attachment 181989 [details] [review]
telepathyClient: Add support for the ACTION message type

>+        if (text.slice(0, 4) == "/me ") {
>+            type = Tp.ChannelTextMessageType.ACTION;
>+            text = text.slice(4);

Is there any way to test if the channel actually supports Type.ACTION? It seems like if it doesn't you might want to keep the "/me " there...

>+            messageBody = "<i>%s</i> %s".format(senderAlias, messageBody);

Hm... check the empathy source code (or ask the telepathy hackers) to see if that should be translatable? (There are definitely languages where it "should" be "action <i>name</i>", but probably IRC clients don't do that, so maybe people just don't use /me in those languages, or use it in ways that reflect the broken word order...)

>+            styles.push("chat-action");

single quotes here and above (assuming it doesn't get translated)

Comment on attachment 181982 [details] [review]
messageTray: Add ability to suppress notification popup

as discussed in the other patch, I think the right way to do this is to have a Source() method to set the notification, and not overload notify()

(In reply to comment #34)
> (From update of attachment 181982 [details] [review])
> as discussed in the other patch, I think the right way to do this is to have a
> Source() method to set the notification, and not overload notify()

I discussed this with Jasper some time ago in IRC, and in the end we agreed that you need an additional parameter to the 'notify' signal, because it is the signal handler for MessageTray that does all the work of building the source and summary notification (through _updateState()).
Else you could have another signal 'notification-set', which MessageTray connects to, skipping all the _notificationQueue stuff.

(In reply to comment #30)
> I've looked into it, but I'd rather it be a separate bug, since the current
> system isn't really designed for it.

ok, please at least file a bug for it before committing this.

(In reply to comment #32)
> We upgrade the contact so we can add those features, and show the proper name
> instead of the ugly internal ID.

OK, explain that in the commit, and make the comment in the code say something about that, rather than talking about "upgrading".

Also, the call is async, so it might be possible for the ugly ID to get used before the upgrade finishes? A way to fix that if it's really an issue would be to have _observeChannels() just call Shell.get_self_contact_features(), and have the callback from get_self_contact_features() do the rest of the stuff currently in _observeChannels.

Alternately, is it possible to use tp_base_client_add_connection_features() or something on this._observer to make it do this for us? (What does empathy do to avoid running into this problem?)

(In reply to comment #36)
> (In reply to comment #30)
> > I've looked into it, but I'd rather it be a separate bug, since the current
> > system isn't really designed for it.
> 
> ok, please at least file a bug for it before committing this.

filed as bug 643513

> (In reply to comment #32)
> > We upgrade the contact so we can add those features, and show the proper name
> > instead of the ugly internal ID.
> 
> OK, explain that in the commit, and make the comment in the code say something
> about that, rather than talking about "upgrading".

The operation in Telepathy to make an existing contact support more features is called "upgrading". Do you have a better name for it?

> Also, the call is async, so it might be possible for the ugly ID to get used
> before the upgrade finishes?

It is.

> A way to fix that if it's really an issue would be
> to have _observeChannels() just call Shell.get_self_contact_features(), and
> have the callback from get_self_contact_features() do the rest of the stuff
> currently in _observeChannels.

I'm curious if it's worth it to investigate that promise.js and async.js stuff that litl introduced.

> Alternately, is it possible to use tp_base_client_add_connection_features() or
> something on this._observer to make it do this for us? (What does empathy do to
> avoid running into this problem?)

This is a contact feature, not a connection feature.

Empathy uses their own contact factory and creates their own self_contact with the self_handle whenever it's updated.

(In reply to comment #37)
> The operation in Telepathy to make an existing contact support more features is
> called "upgrading". Do you have a better name for it?

You don't need to talk about the function names though.

    // Fetch the self contact's display name

or something

Created attachment 182125 [details] [review]
lookingGlass: Add TelepathyGLib to the default set of imports

Created attachment 182126 [details] [review]
messageTray: Add Source.update method

This allows clients to make minor adjustments to their notification
content without triggering a new popup.

Created attachment 182127 [details] [review]
telepathyClient: Re-open existing chat sources on Shell restart

This allows users to continue a chat they were having after
the shell is restarted.

Created attachment 182128 [details] [review]
telepathyClient: Add alias change notifiers

Update the Source title when an contact's alias changes, and
also also add a minor meta message like the current timestamps.

Updating the alias of a 'presenced' contact will overwrite the
current title, and it will also not update the summary item title
right now due to limitations of the message tray.

Created attachment 182129 [details] [review]
history: Fix up HistoryManager

Make GSettings support optional, and fix a couple other typos
and bugs.

Created attachment 182130 [details] [review]
history: Refactor out ClutterText entry event handling code

Add a new method, HistoryManager.connectToEntry that sets up signal
handlers for the common key-presses and changed events. This allows
a quick, easy way to get readline-like behavior for any ClutterText
entry.

Created attachment 182131 [details] [review]
telepathyClient: Add history navigation to entry

This adds a non-saving history to each notification entry,
like runDialog and lookingGlass.

Created attachment 182132 [details] [review]
shell-global: Add shell_get_self_contact_features

This is another workaround for the lack of gjs supporting array
arguments, this time wrapping tp_connection_upgrade_contacts to
add new features to the connection's self contact.

Created attachment 182133 [details] [review]
telepathyClient: Add support for aliases on the self-contact

Upgrade the connection's self contact to allow aliases, so that we
don't show the internal telepathy identifier anywhere user-visible.

Created attachment 182134 [details] [review]
telepathyClient: Add support for the ACTION message type

This message type is usually supported by the '/me' command
in IRC and IM clients.

(In reply to comment #33)
> (From update of attachment 181989 [details] [review])
> >+        if (text.slice(0, 4) == "/me ") {
> >+            type = Tp.ChannelTextMessageType.ACTION;
> >+            text = text.slice(4);
> 
> Is there any way to test if the channel actually supports Type.ACTION? It seems
> like if it doesn't you might want to keep the "/me " there...

There is, but gjs and/or gobject-introspection doesn't support it.

What will happen is that the message will error out, but we don't supply a callback to 'send_message_async' nor handle 'delivery-report' messages properly.

Review of attachment 182132 [details] [review]:

Why not just use tp_connection_get_self_handle() and the existing shell_get_tp_contacts() ?

(In reply to comment #29)
> >+ * Wrap tp_connection_upgrade_contacts due to the lack of support for
> >+ * proper arrays arguments in GJS.
> 
> as in the commit message, please be more precise about exactly what gjs doesn't
> like about this function/callback, so that we can know when the wrapper can go
> away.

Ideally we should have a gjs bug opened for each of those and add a FIXME linking to it in the code.

(In reply to comment #32)
> (In reply to comment #31)
> > (From update of attachment 181988 [details] [review] [details])
> > >Upgrade the connection's self contact to allow aliases, so that we
> > >don't show the internal telepathy identifier anywhere user-visible.
> > 
> > Explain more? I don't understand how/why this code has that effect.
> 
> This is an annoying bug. By default, the Telepathy 'self_contact' that's
> created doesn't have any features, which means that trying to get the alias
> will fall back to the internal Telepathy ID, which is completely ugly.
> 
> We upgrade the contact so we can add those features, and show the proper name
> instead of the ugly internal ID.

I'm starting to think that telepathy-glib should provide contact factory which does some preparation on all the TpContact created. telepathy-qt4 has something like that and it seems to be pretty handy for users.

(In reply to comment #49)
> (In reply to comment #33)
> > (From update of attachment 181989 [details] [review] [details])
> > >+        if (text.slice(0, 4) == "/me ") {
> > >+            type = Tp.ChannelTextMessageType.ACTION;
> > >+            text = text.slice(4);
> > 
> > Is there any way to test if the channel actually supports Type.ACTION? It seems
> > like if it doesn't you might want to keep the "/me " there...
> 
> There is, but gjs and/or gobject-introspection doesn't support it.

Actually I just noticed that we don't have high level API for that. I opened https://bugs.freedesktop.org/show_bug.cgi?id=34907 and will fix it soonish.

(In reply to comment #52)
> (In reply to comment #49)
> > (In reply to comment #33)
> > > (From update of attachment 181989 [details] [review] [details] [details])
> > > >+        if (text.slice(0, 4) == "/me ") {
> > > >+            type = Tp.ChannelTextMessageType.ACTION;
> > > >+            text = text.slice(4);
> > > 
> > > Is there any way to test if the channel actually supports Type.ACTION? It seems
> > > like if it doesn't you might want to keep the "/me " there...
> > 
> > There is, but gjs and/or gobject-introspection doesn't support it.
> 
> Actually I just noticed that we don't have high level API for that. I opened
> https://bugs.freedesktop.org/show_bug.cgi?id=34907 and will fix it soonish.

Done; this API will be in tp-glib 0.13.16.

Note that while implementing this, I discovered a bug in all our CMs. I'm fixing it everywhere, but that means you'll need a recent version of CMs to be able to use this API.

Comment on attachment 182125 [details] [review]
lookingGlass: Add TelepathyGLib to the default set of imports

still acn

Comment on attachment 182126 [details] [review]
messageTray: Add Source.update method

>This allows clients to make minor adjustments to their notification
>content without triggering a new popup.

Again, that's what notification.update() does. At this point pushNotification is really only for the case of "make the source show up in the tray without popping up a notification first" isn't it?

>+    pushNotification: function(notification) {

>+    update: function(notification) {
>+        this.pushNotification(notification);
>+    },

You have two public functions that do exactly the same thing. I'd get rid of update() and keep pushNotification().

Comment on attachment 182127 [details] [review]
telepathyClient: Re-open existing chat sources on Shell restart

good, but needs an s/update/pushNotification/ if you go with that.

Comment on attachment 182128 [details] [review]
telepathyClient: Add alias change notifiers

>Updating the alias of a 'presenced' contact will overwrite the
>current title

I thought you were going to fix that in the next revision?

(If it's annoying to fix the bug now, at least fix the commit message. "Updating the alias of a contact whose source title currently contains presence information will overwrite that information." or something)

>     _updateAlias: function() {
>+        let oldAlias = this.title;
>         this.title = this._contact.get_alias();
>+        this._notification.appendAliasChange(oldAlias, this.title);
>+        this.update();

The this.update() would be a no-op; it will just disconnect the signal handlers on this._notification, and then reconnect them.

(In reply to comment #27)
> (From update of attachment 181985 [details] [review])
> >Make GSettings support optional, and fix a couple other typos
> >and bugs.
> 
> the gsettings stuff is definitely right (and if you want to split that into a
> separate patch, it's accepted-commit_now)
> 
> Not sure about the fixes; even with the fixes, it seems that this._historyIndex
> can range from 0 to this._history.length, and that seems weird. (Eg, if there
> is 1 item in _history, _historyIndex can have 2 different values.) I am not
> familiar with this code though.

The HistoryManager here was largely stolen from runDialog.js. The best explanation I can give is that the 0 index is the slot where you currently are typing. If you notice, before the HistoryManager stuff I did, using the looking glass was painful if you hit 'up' and then 'down', which would take you up a slot in history and then wouldn't go back down.

+ this._historyIndex = this._history.length;

That was happening before, but only because calling this._save() then triggered the 'change' handler, which did exactly that.

Comment on attachment 182129 [details] [review]
history: Fix up HistoryManager

OK, I read through and figured this out. Your fixes all look right, but I think there's still one (pre-existing) bug left, which is that if you press Down (ie, call nextItem()) when _historyIndex == _history.length, then it will append the current text to _history[], but *not* update _historyIndex. (nextItem will increment it, but then _setHistory will clamp it back into bounds, but then still use the original unclamped value to update _history). (Likewise, if you call prevItem when _historyIndex is 0, it will create _history[-1], but that doesn't change _history.length so it has no real effect.)

The way nextItem and prevItem interact with _setHistory (with respect to the value of _historyIndex) is kind of bizarre. it would be better if prevItem() no-opped if _historyIndex was 0, and nextItem() no-opped if _historyIndex was _history.length, and then _setHistory() wouldn't need to re-adjust it. In fact, _setHistory wouldn't really be needed at all:

    nextItem: function(text) {
        if (this._historyIndex < this._history.length) {
            this._history[this._historyIndex++] = text;
            return this._indexChanged();
        } else
            return text;
    }

(In reply to comment #57)
> (From update of attachment 182128 [details] [review])
> >Updating the alias of a 'presenced' contact will overwrite the
> >current title
> 
> I thought you were going to fix that in the next revision?

I will, eventually. It's just annoying having to keep the presence around. We're also depending on the behavior bug 643513 is doing to make sure the summary item title doesn't have "is offline" or anything, so I'll fix it when I fix 643513, which should be soon.

> (If it's annoying to fix the bug now, at least fix the commit message.
> "Updating the alias of a contact whose source title currently contains presence
> information will overwrite that information." or something)
> 
> >     _updateAlias: function() {
> >+        let oldAlias = this.title;
> >         this.title = this._contact.get_alias();
> >+        this._notification.appendAliasChange(oldAlias, this.title);
> >+        this.update();

(Doing some funky comment replies)
> You have two public functions that do exactly the same thing. I'd get rid of update() and keep pushNotification().

pushNotification should have been private, my bad. The way I wanted it was 'update' is silent and 'notify' is noisy.

Additionally, right now we have a 'notify' signal which things can connect to to mean that the notification was updated. Should we add a new 'updated' signal and only have 'notify' to mean it was noisily shown, so anything that tracks properties of the notification, like the SummaryItem does, would have to connect to 'updated', not 'notify'?

> The this.update() would be a no-op; it will just disconnect the signal handlers
> on this._notification, and then reconnect them.

I forgot. I wanted the 'update' override to call 'this.notification.update(this.title, null, { customContent: true });'.

Comment on attachment 182130 [details] [review]
history: Refactor out ClutterText entry event handling code

>+    connectToEntry: function(entry) {
>+        this.connect('changed', function(manager, text) {

connecting to your own signals is considered bad style.

I'm thinking now it would be better to just pass the entry to HistoryManager's constructor, and then you could store it as this._entry, and just update it directly from _indexChanged.

>+            if (symbol == Clutter.Up) {
>+            } else if (symbol == Clutter.Down) {

That style of keysym name is deprecated (though still widely used in gnome-shell). Use Clutter.KEY_Up, Clutter.KEY_Down.

Comment on attachment 182131 [details] [review]
telepathyClient: Add history navigation to entry

>+const Clutter = imports.gi.Clutter;

no longer needed. otherwise good

Created attachment 182346 [details] [review]
history: Fix HistoryManager

Make GSettings support optional, refactor text entry handling,
fix some off-by-one bugs in the management itself, use Params
for parsing, fix other typos and bugs.

(In reply to comment #61)
> (From update of attachment 182130 [details] [review])
> >+    connectToEntry: function(entry) {
> >+        this.connect('changed', function(manager, text) {
> 
> connecting to your own signals is considered bad style.
> 
> I'm thinking now it would be better to just pass the entry to HistoryManager's
> constructor, and then you could store it as this._entry, and just update it
> directly from _indexChanged.

I was a bit hesitant to do that sort of stuff in a file in js/misc, but OK. I doubt there's going to be any need for HistoryManager outside of Clutter.Text, and I doubt we're going to need more than one hooked up.

I squashed the two HistoryManager commits into a big one: it's hard to split them out, making each one an atomic non-build-breaking commit, along with a easily identifiable purpose.

> >+            if (symbol == Clutter.Up) {
> >+            } else if (symbol == Clutter.Down) {
> 
> That style of keysym name is deprecated (though still widely used in
> gnome-shell). Use Clutter.KEY_Up, Clutter.KEY_Down.

Aha! I always thought that the 'KEY_' was due to an introspection/gjs limitation, and that 'Clutter.Up' was the new style. Fixed.

Comment on attachment 182346 [details] [review]
history: Fix HistoryManager

> Aha! I always thought that the 'KEY_' was due to an introspection/gjs
> limitation, and that 'Clutter.Up' was the new style.

No, the problem is that the old names pollute the namespace when used via introspection. Eg, they wouldn't be able to create a "ClutterMenu" class, because it would conflict with the CLUTTER_Menu keysym.

>+        if (this._entry)
>+            this._entry.connect('key-press-event', 
>+                                Lang.bind(this, this._onEntryKeyPress));

{}s around that for consistency

>+        this._historyIndex --;

no space before -- or ++

good to commit with those changes. The switch to params is definitely the right thing.

Attachment 182125 [details] pushed as 8ce9796 - lookingGlass: Add TelepathyGLib to the default set of imports
Attachment 182346 [details] pushed as 75ae209 - history: Fix HistoryManager

Created attachment 182733 [details] [review]
messageTray: Add Source.pushNotification method

This allows clients to make minor adjustments to their notification
content without triggering a new popup.

Created attachment 182734 [details] [review]
telepathyClient: Re-open existing chat sources on Shell restart

This allows users to continue a chat they were having after
the shell is restarted.

Created attachment 182735 [details] [review]
telepathyClient: Add alias change notifiers

Update the Source title when an contact's alias changes, and
also also add a minor meta message like the current timestamps.

Updating the alias of a 'presenced' contact will overwrite the
current title, and it will also not update the summary item title
right now due to limitations of the message tray.

Created attachment 182736 [details] [review]
telepathyClient: Add history navigation to entry

This adds a non-saving history to each notification entry,
like runDialog and lookingGlass.

Created attachment 182737 [details] [review]
shell-global: Add shell_get_self_contact_features

This is another workaround for the lack of gjs supporting array
arguments, this time wrapping tp_connection_upgrade_contacts to
add new features to the connection's self contact.

Created attachment 182738 [details] [review]
telepathyClient: Add support for aliases on the self-contact

Upgrade the connection's self contact to allow aliases, so that we
don't show the internal telepathy identifier anywhere user-visible.

Created attachment 182739 [details] [review]
telepathyClient: Add support for the ACTION message type

This message type is usually supported by the '/me' command
in IRC and IM clients.

Created attachment 182778 [details] [review]
telepathyClient: Re-open existing chat sources on Shell restart

This allows users to continue a chat they were having after
the shell is restarted.

Created attachment 183017 [details] [review]
telepathyClient: Add history navigation to entry

This adds a non-saving history to each notification entry,
like runDialog and lookingGlass.



rebased to master after drago01's scrollbar fixes

Created attachment 183018 [details] [review]
telepathyClient: Add support for the ACTION message type

This message type is usually supported by the '/me' command
in IRC and IM clients.



rebased to master after drago01's scrollbar fixes

Comment on attachment 182737 [details] [review]
shell-global: Add shell_get_self_contact_features

>+  TpContact *self_contact = *contacts;
>+  if (error != NULL) {

You need to check error before dereferencing contacts. OK to commit after that.

Comment on attachment 183017 [details] [review]
telepathyClient: Add history navigation to entry

>+const Clutter = imports.gi.Clutter;

not used any more. ok to commit after removing that

Attachment 182733 [details] pushed as 4029202 - messageTray: Add Source.pushNotification method
Attachment 182735 [details] pushed as 8c40c20 - telepathyClient: Add alias change notifiers
Attachment 182737 [details] pushed as ebcb87c - shell-global: Add shell_get_self_contact_features
Attachment 182738 [details] pushed as 6ff69da - telepathyClient: Add support for aliases on the self-contact
Attachment 182778 [details] pushed as f4a000c - telepathyClient: Re-open existing chat sources on Shell restart
Attachment 183017 [details] pushed as 79d9df9 - telepathyClient: Add history navigation to entry
Attachment 183018 [details] pushed as e6aee5d - telepathyClient: Add support for the ACTION message type

Jasper: http://git.gnome.org/browse/gnome-shell/commit/?id=8c40c2086aac9d252c51150a9ac5f4760ba352dc broke the string freeze by adding the translatable string "%s is now known as %s".
See http://live.gnome.org/TranslationProject/HandlingStringFreezes

Created attachment 183428 [details] [review]
telepathyClient: remove alias-change messages, to unbreak string freeze

Created attachment 183429 [details] [review]
Revert "telepathyClient: remove alias-change messages, to unbreak string freeze"

to be committed post-freeze

Comment on attachment 183428 [details] [review]
telepathyClient: remove alias-change messages, to unbreak string freeze

Attachment 183428 [details] pushed as 4d804c2 - telepathyClient: remove alias-change messages, to unbreak string freeze

Comment on attachment 183429 [details] [review]
Revert "telepathyClient: remove alias-change messages, to unbreak string freeze"

tagging this to help us not forget...

[Minor nitpick about "// FIXME: uncomment this after 3.0 string freeze ends": 3.0 string freeze NEVER ends => "Commit to master after branching for gnome-3-0"]

Attachment 183429 [details] pushed as 72bee6d - Revert "telepathyClient: remove alias-change messages, to unbreak string freeze"