apidocs need a way to document object properties

RESOLVED FIXED in 0.5

Status

RESOLVED FIXED
9 years ago
8 years ago

People

(Reporter: adw, Assigned: adw)

Tracking

unspecified

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 2 obsolete attachments)

(Assignee)

Description

9 years ago
It looks like the only "top-level" @foo that apiparser.py supports is @method.  There should be a way to document object members that are not functions.  For example, TextReader objects in the text-streams module have a boolean member `closed` that's true if the stream is closed:

  var stream = require("file").open(filename);
  var isClosed = stream.closed;
  // isClosed == false

There's no way to document `closed`.  A suggestion:

<api name="closed">
@member {boolean}
  True if the stream is closed.
</api>
(Assignee)

Comment 1

9 years ago
Created attachment 447199 [details] [diff] [review]
patch

This seems to work.  No idea if I got everything though.  It uses "property" instead of "member" since "property" is more precise, but there's potential to cause confusion with @prop.

  <api name="closed">
  @property {boolean}
    True if the stream is closed.
  </api>

Produces in the HTML:

  closed is boolean
  True if the stream is closed.

Similar to how methods look like "foo() returns bar".
Attachment #447199 - Flags: review?(warner-bugzilla)
(Assignee)

Comment 2

9 years ago
(In reply to comment #1)
> Produces in the HTML:
> 
>   closed is boolean
>   True if the stream is closed.

"closed is a boolean" sounds better.  I'll batch that change with others during review.
Comment on attachment 447199 [details] [diff] [review]
patch

looks good to me. Some nits:

* in _createPropertyTitle(), should "title" be "$title", to reflect that it's a jquery object? Also, I didn't realize that "is" isn't reserved.. that threw me for a moment, but if it's standard JS style, I'm cool with it.
 * this could use some tests. I'll see if I can code something up quickly.
Attachment #447199 - Flags: review?(warner-bugzilla) → review+
(Assignee)

Comment 4

9 years ago
Created attachment 447637 [details] [diff] [review]
patch v2

This addresses comment 3 and adds tests.
Assignee: nobody → adw
Attachment #447199 - Attachment is obsolete: true
Status: NEW → ASSIGNED
Attachment #447637 - Flags: review?(warner-bugzilla)
(Assignee)

Comment 5

9 years ago
Created attachment 447638 [details]
screenshot

Noelle, what do you think about this style and syntax?
(Assignee)

Comment 6

9 years ago
Created attachment 447727 [details] [diff] [review]
patch v3

Improves indefinite article detection and extends it to "returns" for functions, like:

  foo() returns a string
  bar() returns an object
  baz() returns a URL

I can remove the articles if you guys want. :D
Attachment #447637 - Attachment is obsolete: true
Attachment #447727 - Flags: review?(warner-bugzilla)
Attachment #447637 - Flags: review?(warner-bugzilla)

Comment 7

9 years ago
looks good to me!
Comment on attachment 447727 [details] [diff] [review]
patch v3

looks great. One question (perhaps for a different bug): should we handle sub-properties via "@prop" lines on the @property APIs? The "self" example doesn't happen to need it, and perhaps the best approach is to follow the dot-separated 'api name="data.load"' approach (i.e. name the sub-properties with "absolute" paths, instead of relative ones, in separate API clauses). But it's probably worth thinking about briefly.
Attachment #447727 - Flags: review?(warner-bugzilla) → review+
(Assignee)

Comment 9

9 years ago
(In reply to comment #8)
> looks great. One question (perhaps for a different bug): should we handle
> sub-properties via "@prop" lines on the @property APIs?

Good point.  Noelle, what do you think?  We should pick one of the styles that Brian mentions (or maybe some other) and add it to our list of doc conventions.  Since we use the @prop syntax for both object parameters and returned objects, seems like we should use it here too.
(Assignee)

Comment 10

9 years ago
Went ahead and pushed.  If we want to add @prop lines for @property, we can do that in a new bug.
http://hg.mozilla.org/labs/jetpack-sdk/rev/8fbf341270a0

Noelle, I'd still like to hear your thoughts re: Brian's comment 8.
Status: ASSIGNED → RESOLVED
Last Resolved: 9 years ago
Resolution: --- → FIXED
Summary: apidocs need a way to document non-function object members → apidocs need a way to document object properties
Target Milestone: -- → 0.5

Comment 11

9 years ago
I think it makes sense to think about; definitely. At the current status of docs I don't know that it should be a high priority, but, if you wanted to implement it now/sooner-rather-than-later in order to avoid headache later - that would be awesome!
(Assignee)

Comment 12

9 years ago
Thanks Noelle.  In that case I'll leave it alone for now.  If/when it becomes a higher priority or I get bored, we can come back.
The Add-on SDK is no longer a Mozilla Labs experiment and has become a big enough project to warrant its own Bugzilla product, so the "Add-on SDK" product has been created for it, and I am moving its bugs to that product.

To filter bugmail related to this change, filter on the word "looptid".
Component: Jetpack SDK → General
Product: Mozilla Labs → Add-on SDK
QA Contact: jetpack-sdk → general
Version: Trunk → unspecified
You need to log in before you can comment on or make changes to this bug.