parameter types as global variables confuse parser about js-doc-parse

[csnover]

8116fc9 expands the valid characters for a type annotation to include . and / which fixes this issue.

[wkeese]

Cool, they are showing up now, although there's a stray question mark:

<parameter name="plot" type="String?|dojox.charting.plot2d.Base" usage="optional">
<summary>
Optional target plot for this action. Default is "default".
</summary>
</parameter>

I guess the source code should be modified to only have a question mark at the end of the list of possible types?

[cjolif]

I can modify the source code for removing the question mark is if needed. That said I have another problem afterwards. See for example dojox/charting/Chart.setTheme. Now the parameter of type dojox.charting.SimpleTheme is correctly set. However it also automatically pulls into the description the parameter description the dojox.charting.SimpleTheme class description and I don't really want that in that case? (this is only useful for kwarg type of parameters?). So we need something to specify when we want that vs when we don't? Or do we really want the every time?

<method type="function" from="dojox/charting/Chart" scope="prototype" name="setTheme">
<parameters>
<parameter type="Object" name="theme" usage="required">
<summary>The theme to be used for visual rendering.</summary>
<description><p>An object with the following properties: </p> <ul> <li>shapeSpaces</li> <li>clone: Clone the current theme.</li> <li>clear: Clear and reset the internal pointer to start fresh.</li> <li>next: Get the next color or series theme.</li> <li>skip: Skip the next internal color.</li> <li>addMixin: Add a mixin object to the passed theme and process.</li> <li>post: Process any post-shape fills.</li> <li>getTick: Calculates and merges tick parameters.</li> <li>inspectObjects</li> <li>reverseFills</li> <li>addMarker: Add a custom marker to this theme.</li> <li><p>setMarkers: Set all the markers of this theme at once. obj should be a dictionary of keys and path segments.</p> </li> <li><p>_buildMarkerArray</p> </li> <li>defaultMarkers</li> <li>defaultColors</li> <li>defaultTheme</li> </ul> </description>
</parameter>
</parameters>
<return-types>
<return-type type="dojox.charting.Chart"/>
<return-type type="function"/>
</return-types>
<return-description>A reference to the current chart for functional chaining.</return-description>
<summary>Set a theme of the chart.</summary></method>

[cjolif]

Actually this is even worst than I thought not only all the properties of the classes are put in the description but the class type is entirely removed. Just like for a kwArgs. I don't seem to have the permission to re-open that one but I think it should be. @wkeese as you created maybe you can re-open it?

[wkeese]

I can't reopen, only Colin can.

But I think the problem is that in the above case you want the parser to just list the type as dojox.charting.plot2d.Base, rather than inlining it, whereas for something like below, you want the definition of dojox.gfx.Point to be inlined, because otherwise the definition is lost.

declare("dojox.gfx.Point", null, {
    // summary:
    //      a hypothetical 2D point to be used for drawings - {x, y}
    // description:
    //      This object is defined for documentation purposes.
    //      You should use the naked object instead: {x: 1, y: 2}.
});
...
// returns: dojox.gfx.Point
//      the midpoint

How can the parser tell the difference between those two cases?

[wkeese]

Talked to Colin on IRC about this. You can avoid the inlining by using slashes:

// theme: dojox/charting/SimpleTheme

or to access an attribute in a module return value:

// rect: dojox/gfx/shape.Rect

Colin could also theoretically put in a hack to figure that out automatically, but only if SimpleTheme was a dependency of Chart, so you would need to add /*==== , "dojox/charting/SimpleTheme" =====*/ to Chart.js' dependency list. Not sure that's any easier than just changing dots to slashes.

A regex to find all the places to change is: //[\s]*[\S]+:[\s]*(dojo|dijit|dojox)\. and /\* *(dojo|dijit|dojox)\.

[cjolif]

Two things:

1/ SimpleTheme is already a dependency for Chart. But that might not be indeed the case for all of the cases where I have this problem.

2/ I think the number of cases were we want inlining (basically kwArgs use-case) is far lower than the number where we don't want? (regular parameters). So it sounds strange to me to request changing the doc for the majority of the cases to deal with a minority use-case (inlining)? Can't we keep the doc as is and request kwArgs type parameters to put a specific marker in the doc to trigger the inlining?

[wkeese]

Sorry, I was confused about the dependencies... the problem actually is triggered by having SimpleTheme as a dependency of Chart... if it weren't a dependency, the parser wouldn't be able to inline SimpleTheme's definition since it would have no idea what it is.

Anyway, the *main" reason to use slashes rather than dots is that the current charting code still hasn't been fully converted to baseless AMD. Specifically, it's still referencing and depending on global variables for lines like:

// chart: dojox.charting.Chart

Working around the parser bug is a secondary reason for using slashes.

[cjolif]

Ok, I will move to slashes. That said I won't do that on the entire dojox code base so I'm afraid we will get problems somewhere else.

[wkeese]

Colin says he has a fix where it won't inline the data if there's a relatedModule, and plans to check it in this weekend.