Closed Bug 953569 Opened 10 years ago Closed 10 years ago

Extensions should be able to register commands.

Categories

(Chat Core :: General, defect)

defect
Not set
minor

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: florian, Assigned: florian)

References

Details

(Whiteboard: [0.3-wanted])

Attachments

(2 files, 1 obsolete file)

*** Original post on bio 118 at 2008-09-23 00:56:00 UTC ***

We should add a scriptable interface to register additional commands.
It should also possible to list existing commands and get info about them (name, priority, related to a specific prpl, ...)
Blocks: 953550
Blocks: 953944
Whiteboard: [0.3-wanted]
*** Original post on bio 118 at 2011-02-03 15:32:20 UTC ***

Some notes about this from flo:

> 17:46:12 <flo> cmd.h/cmd.c files in purple/libpurple/
> 17:46:42 <flo> and some crap at the end of
> purple/purplexpcom/src/purpleInit.cpp for the commands we currently register
> (= those that are not from the libpurple core or the libpurple protocols)
Attached patch Patch v1 (js part only) (obsolete) — Splinter Review
*** Original post on bio 118 as attmnt 517 at 2011-02-09 23:43:00 UTC ***

This is the part adding a JS XPCOM component to handle the commands. It works, I tested it by registering a "dump" command with this code in the error console:

Components.classes["@instantbird.org/purple/commands-service;1"].getService(Components.interfaces.imICommandsService).registerCommand({name: "dump", priority: 0, run: function(aMsg) {dump(aMsg + "\n"); return true; } });

@clokep: I think you can understand most of this code and I would like some feedback especially w.r.t how useful this API is for JS protocol plugins :). The C/C++ part to make libpurple use this new system may be harder for you to look at.
Attachment #8352258 - Flags: review?(clokep)
Assignee: nobody → florian
Status: NEW → ASSIGNED
Comment on attachment 8352258 [details] [diff] [review]
Patch v1 (js part only)

*** Original change on bio 118 attmnt 517 at 2011-02-10 01:12:33 UTC ***

(In reply to comment #2)
> Created an attachment (id=517) [details]
> @clokep: I think you can understand most of this code and I would like some
> feedback especially w.r.t how useful this API is for JS protocol plugins :).
> The C/C++ part to make libpurple use this new system may be harder for you to
> look at.

Yeah, it looks fine. The API looks easy enough, in terms of JS plugins...we'd need to register them for each protocol (probably as part of the initialization code) I'm guessing? That's not a big deal (and we could even have it be done automatically if we wanted).

The C/C++ part I don't think I'll be much help on.
Attachment #8352258 - Flags: review?(clokep) → review+
*** Original post on bio 118 at 2011-02-10 09:29:32 UTC ***

(In reply to comment #3)

> [...] we'd
> need to register them for each protocol (probably as part of the initialization
> code) I'm guessing? That's not a big deal (and we could even have it be done
> automatically if we wanted).

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.
Attached patch Patch v2Splinter Review
*** Original post on bio 118 as attmnt 519 at 2011-02-10 19:14:00 UTC ***

I think it's ready. Whoever is interested should feel free to look and comment :).
Comment on attachment 8352258 [details] [diff] [review]
Patch v1 (js part only)

*** Original change on bio 118 attmnt 517 at 2011-02-10 19:14:13 UTC was without comment, so any subsequent comment numbers will be shifted ***
Attachment #8352258 - Attachment is obsolete: true
*** Original post on bio 118 at 2011-02-10 19:29:35 UTC ***

flo: It looks good from what I can understand of it (which unfortunately is no where near all of it).
*** Original post on bio 118 as attmnt 520 at 2011-02-10 21:21:00 UTC ***

Additional changes above attachment 8352260 [details] [diff] [review] (bio-attmnt 519) (nits noticed during a self review).
*** Original post on bio 118 at 2011-02-10 21:35:09 UTC ***

https://hg.instantbird.org/instantbird/rev/4e8ee81dc102
Status: ASSIGNED → RESOLVED
Closed: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → 0.3a2
Blocks: 954125
*** Original post on bio 118 at 2011-02-11 14:03:02 UTC ***

Follow up to fix the packaging on all OSes and the compilation on Linux: https://hg.instantbird.org/instantbird/rev/2a7327c35427
Depends on: 954132
You need to log in before you can comment on or make changes to this bug.