csnover / js-doc-parse

An experimental library for parsing JavaScript files and extracting inline documentation.
31 stars 7 forks source link

property's summary gets assigned to the property's value #61

Open wkeese opened 11 years ago

wkeese commented 11 years ago

dijit/form/TextBox's module level summary is:

var TextBox = declare("dijit.form.TextBox", [_FormValueWidget, _TextBoxMixin], {
    // summary:
    //      A base class for textbox form inputs

However, it's appearing in the API viewer (and presumably the details.xml) as the summary from a property of InlineEditBox whose value is TextBox:

    // editor: String|Function
    //      MID (ex: "dijit/form/TextBox") or constructor for editor widget
    editor: TextBox,
neonstalwart commented 11 years ago

i just came across this same issue... when generateMetaData handles 'Property' types, if the property happens to be for a value that already has metatdata then we mixin/override the summary of the existing metadata with the one from the property.

as an example, the following snippet wrongly redefines the summary for SimpleQueryEngine because the value is a reference to SimpleQueryEngine.

// queryEngine: Function
//      Defines the query engine to use for querying the data store
queryEngine: SimpleQueryEngine,

i traced it to propertyValue = read(property.value); which will return a reference to the SimpleQueryEngine value that then gets passed to generateMetadata which then updates the summary. i wonder if it would hurt anything if we changed it to propertyValue = read(property.value, { asIdentifier: true });.

neonstalwart commented 11 years ago

i wonder if it would hurt anything if we changed it to propertyValue = read(property.value, { asIdentifier: true });.

it does hurt... very badly - so that might not be the answer :smiley:

neonstalwart commented 11 years ago

by default, a property's metadata is a copy of the value's metadata but in generateMetadata we have enough context to know when a property's metadata should override the value's metadata. so that's what i've done with #84.

@wkeese this really makes a big difference to dojo's docs - you might want to try it out. i think you'll like it :smiley:

wkeese commented 11 years ago

@neonstalwart Yes I'd love to integrate all your fixes, that's why I'm disappointed that you're working against csnover's branch and forcing me to merge.