Closed Bug 617029 Opened 9 years ago Closed 9 years ago

API reference syntax should understand about events

Categories

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

defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: wbamberg, Assigned: wbamberg)

References

Details

Attachments

(1 file, 1 obsolete file)

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.
Attached patch Added event syntax (obsolete) — Splinter Review
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...
Attachment #521624 - Flags: review?(warner-bugzilla)
Priority: -- → P1
Hardware: x86 → All
Target Milestone: --- → 1.0
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.
Attachment #521624 - Attachment is obsolete: true
Attachment #527181 - Flags: review?(warner-bugzilla)
Attachment #521624 - Flags: review?(warner-bugzilla)
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!
Attachment #527181 - Flags: review?(warner-bugzilla) → review+
Thanks Brian! Fixed as: https://github.com/mozilla/addon-sdk/commit/c9a700e93d9e87aca2dd154e79b53b0a7bf46aeb
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.