617029 - API reference syntax should understand about events

Status: RESOLVED FIXED

(Add-on SDK Graveyard :: Documentation, defect, P1)

Description

[Will Bamberg [:wbamberg]]

Events are a fundamental idiom in the SDK, and the API reference syntax should represent them explicitly to make sure they are documented consistently across all modules.

The syntax needs to capture:
- the name of the event (e.g. 'message', 'show')
- conditions in which the event is emitted
- arguments with which the event listener is called

Although it's possible to implement this without the fix for bug 612721 being in, it would be a lot easier and safer to implement after bug 612721, so I'm marking it as dependent.

Comment 1

[Will Bamberg [:wbamberg]]

Attachment: Added event syntax

Here's a patch that adds event syntax to the API docs. An event is documented like this:

<api name="my_event">
@event
Description of my event.
@argument {Datatype}
Description of this argument, which will be supplied to any listener function.
</api>

Events can be at module level, or nested within other objects.

I decided to create a new 'argument' type as well, on the basis that while these things are a lot like parameters, parameter must have names, whereas the names for arguments passed to event listeners is up to the event listener itself to define, it seems to me. Otherwise event arguments will all look like "worker : Worker" which seems silly and redundant. But I'm not sure.

I also migrated three of the module docs, to see what they look like:
- page-worker
- private-browsing
- windows

Sorry this patch is quite big. Although the change is pretty tiny I took the chance to rewrite some of the test code to make it easier to maintain. But if you like I could revert that.

This code is also up here: https://github.com/wbamberg/jetpack-sdk/tree/617029 if that's easier to review...

Comment 2

[Will Bamberg [:wbamberg]]

Attachment: Reverted refactoring changes to test code

This patch includes only:

- the new event syntax
- new tests for it
- updated a few module docs to show what it looks like

But doesn't change any of the existing test structure.

Comment 3

[Brian Warner [:warner :bwarner]]

Comment on attachment 527181 [details] [diff] [review]
Reverted refactoring changes to test code

Thanks for simplifying it.. this looks good! One suggestion for the future (don't change it in this patch), in _assemble_api_element(), you can use 'if working_set["methods"]' instead of 'if len(working_set["methods"]) > 0', unless you need to distinguish between None and an empty list.

ship it!

Comment 4

[Will Bamberg [:wbamberg]]

Thanks Brian! Fixed as: https://github.com/mozilla/addon-sdk/commit/c9a700e93d9e87aca2dd154e79b53b0a7bf46aeb