Closed Bug 954125 Opened 7 years ago Closed 7 years ago

jsProtoHelper could help registering commands

Categories

(Chat Core :: General, enhancement)

enhancement
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: florian, Assigned: clokep)

References

Details

Attachments

(1 file)

*** 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.
Depends on: 953569
Assignee: nobody → clokep
Status: NEW → ASSIGNED
Attached patch v1.0Splinter Review
*** 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.
Attachment #8352262 - Flags: review?(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.
Attachment #8352262 - Flags: review?(florian) → review+
*** 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.
*** Original post on bio 690 at 2011-02-17 12:19:44 UTC ***

https://hg.instantbird.org/instantbird/rev/ea20178e93a4
Status: ASSIGNED → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → 0.3a2
You need to log in before you can comment on or make changes to this bug.