[WebODF] Documenting configuration options

10 Apr 2013

      Hi,

A recent patch I submitted (https://gitorious.org/webodf/webodf/commit/259b203d2198ce457c85994cbda5eaac1..., https://gitorious.org/webodf/webodf/commit/b5ebf1452b495d0cd2143e7425e6f57a5...) had some difficulties with the documentation of the SessionViewOptions type. I didn't want to enforce a particular class, as these options are basically a dictionary of optional entries that influence how the view is initialised, and is most conveniently passed in using plain old object literals (i.e., { } notation).

A few ways that have been attempted:

Anonymous types

Looks like:
    /**
      * @constructor
     * @param {{editInfoMarkersInitiallyVisible:boolean=true, caretAvatarsInitiallyVisible:boolean=true}} viewOptions
     * @param {ops.Session} session
     * @param {gui.CaretFactory} caretFactory
      */
   function SessionView(viewOptions, session, caretFactory) {
… }

I didn't like these because it meant that the options had to be copied and pasted into every place that passed them around. Anonymous types also don't have any syntax for allowing documentation of members (from what I could tell)

Interface type

Looks like:
/**
 * @interface
 */
gui.SessionViewOptions = function() {
    "use strict";

    /**
     * Set the initial edit information marker visibility
     * @type {boolean}
     */
    this.editInfoMarkersInitiallyVisible = true;
    ...
};

This was the submitted code. There is no clear way to document pure properties involving primitive types however. All combinations attempted (e.g., this.X, SessionViewOptions.prototype.X) complained in either closure or jslint.

Typedef
Looks like:
/*
 * @typedef {{editInfoMarkersInitiallyVisible:boolean,
 *               caretAvatarsInitiallyVisible:boolean}}
 */
gui.SessionViewOptions;

This was the chosen approach for now, as at least it allows the SessionViewOptions properties to be defined in only one place. However, it still doesn't allow these individual members to be documented. Also, jslint doesn't really like the non-method, non-assign statement that defines the type name

Proposed approach - Struct type

Looks like:
/**
 * @constructor
 * @struct
 */
gui.SessionViewOptions = function() {
    "use strict";

    /**
     * Set the initial edit information marker visibility
     * @type {boolean}
     */
    this.editInfoMarkersInitiallyVisible = true;
    ...
};

This method was discovered after submission of the patch. It allows object literals to be cast to that type (see example at https://developers.google.com/closure/compiler/docs/js-for-compiler#tag-stru...). I think this is perhaps the best option so far, and suggest potentially it be adopted as "the standard" way of documenting these configuration objects where they are likely to see re-use or passage between classes.

Does anyone else have other thoughts or preferences?

Cheers,

Philip Peitsch