954125 - jsProtoHelper could help registering commands

Status: RESOLVED FIXED

(Chat Core :: General, enhancement)

Description

[Florian Quèze [:florian]]

*** Original post on bio 690 at 2011-02-10 21:48:00 UTC ***

Now that it's possible to register commands implemented in JS (bug 953569 (bio 118) is fixed), it would be nice to provide a helper for that in jsProtoHelper.

In bug 953569 (bio 118) comment 4, I wrote:
If we assume protocol plugins want to register their commands at the
PRIORITY_PRPL priority, we can make it as simple as this for protocols:

 commands: {
   nick: function() { ...},
   op: function() { ...}
 },

Of course, we would need to add in jsProtoHelper the code that registers these
commands when the protocol is first used.

Comment 1

[Patrick Cloke [:clokep]]

Attachment: v1.0

*** Original post on bio 690 as attmnt 521 at 2011-02-15 00:58:00 UTC ***

This adds a function that will automatically register commands from an array in the protocol object, unfortunately there's no init function for the protocol implementation (that I saw at least), so I'm calling this manually in my IRC code (http://hg.instantbird.org/experiments/file/803aa6539a1b/components/ircProtocol.js#l364)

For the actual commands, you can see: http://hg.instantbird.org/experiments/file/803aa6539a1b/components/ircProtocol.js#l387 although I haven't really implemented any yet, since I want a review before I start typing out lots of code.

Comment 2

[Florian Quèze [:florian]]

Comment on attachment 8352262 [details] [diff] [review]
v1.0

*** Original change on bio 690 attmnt 521 at 2011-02-16 12:09:27 UTC ***

I'm a bit sad that we can't make it as simple for protocols as just:
commands: {
  nick: function() {},
  foo: function() {}
}
but if we want all protocols to specify the helpString so that the /help command can be actually useful, we can't simplify that much.

I thought we could workaround the lack of init method in protocols by registering the commands only when a first account of the protocol is initialized, but that wouldn't work with the second proposed approach to fix bug 954132 (bio 697).

So, even though I'm not really enthusiast about this patch, I guess it's the best we can do.

r=me.

Comment 3

[Patrick Cloke [:clokep]]

*** Original post on bio 690 at 2011-02-16 13:11:46 UTC ***

(In reply to comment #2)
> (From update of attachment 8352262 [details] [diff] [review] (bio-attmnt 521) [details])
> I'm a bit sad that we can't make it as simple for protocols as just:
> commands: {
>   nick: function() {},
>   foo: function() {}
> }
We could do that, but then you can't change the help string and priority (which probably doesn't matter) if you want to. The way I was doing the /help command it would have some default "This command has no help associated with it" string for ones that don't give it.

I figured the command objects were so simple anyway that it'd be fine to just put the whole object in and fill in the missing bits.

> but if we want all protocols to specify the helpString so that the /help
> command can be actually useful, we can't simplify that much.
> I thought we could workaround the lack of init method in protocols by
> registering the commands only when a first account of the protocol is
> initialized, but that wouldn't work with the second proposed approach to fix
> bug 954132 (bio 697).
> So, even though I'm not really enthusiast about this patch, I guess it's the
> best we can do.
> r=me.
I don't particularly like having to call the register method ourself.

Comment 4

[Florian Quèze [:florian]]

*** Original post on bio 690 at 2011-02-17 12:19:44 UTC ***

https://hg.instantbird.org/instantbird/rev/ea20178e93a4