Skip to content
This repository has been archived by the owner on Dec 31, 2017. It is now read-only.

EDIT: provide raw metadata to exporter #80

Open
wants to merge 5 commits into
base: master
Choose a base branch
from

Conversation

neonstalwart
Copy link

many return descriptions contain markdown - some even contain examples

EDIT: this PR now encompasses a number of issues that are somewhat interdependent. the end goal was to provide the raw metadata to the exporter and along the way i found a few bugs that have been fixed and folded into this PR.

@wkeese
Copy link
Collaborator

wkeese commented Mar 23, 2013

This is already part of #59, specifically e75fed6.

@neonstalwart
Copy link
Author

even though this PR was presented as a way to parse return descriptions, one of the key pieces of this PR is breaking out processMetadata so that it can be reused. i'm planning to attach a raw property to the metadata so that the unparsed summary, description, examples, etc can be stashed somewhere that an exporter can get to it.

regarding 58cfcf6, in practice, it's currently not possible that a property in metadata.properties is going to have anything other than a summary that needs to be parsed but recursing with parseMetadata slightly simplifies the code for stashing the raw values for the metadata. i think i'll just add that to this PR too.

by resolving this in the exporter, we can avoid a race condtion where
sometimes relatedModule had not been indicated when it should have been.
@wkeese
Copy link
Collaborator

wkeese commented Apr 29, 2013

This and #84 seem to be an improvement. I started to diff the output details.xml, and although my eyes quickly glazed over, I did notice that:

  • start, end, obj parameters to dojo/_base/Color::blendColors() listed as type dojo/_base/Color rather than having the description inlined. Likewise for fromHex() (improvement)
  • dojo/_base/fx::anim() listed as type "function" rather than "Function". API viewer works either way (indifferent)
  • description of dojo/_base/fx::anim() is the same, but summary changed from "Alias of dojo.anim - the shorthand dojo.animateProperty with auto-play" to "A simpler interface to animateProperty(), also returns an instance of Animation but begins the animation immediately, unlike nearly every other Dojo animation API." Not sure where the first one came from, but the second one is correct. (improvement)
  • some spacing changes in HTML around dojo/_base/xhr; presumably no effect on output.

So I merged this pull request and #84 into my code (https://github.com/wkeese/js-doc-parse) and regenerated the 1.8 and 1.9 details.xml and tree.json used on the website.

As a side note, I just did the normal merge command:

$ git checkout -b neonstalwart-process-return-description master
$ git pull git://github.com/neonstalwart/js-doc-parse.git process-return-description
$ git commit

... but it squashed all the commits into one. Maybe because I had to manually resolve conflicts. Not sure if it's better to keep them separate or not.

@wkeese
Copy link
Collaborator

wkeese commented Apr 29, 2013

One alternative would be to call processTypeAnnotation() after dojodoc completes, but before the exporter(s) are called.

Note though how processTypeAnnotation() in your submitted patch generates HTML but how processTypeAnnotation() in your markdown-exporter branch generates markdown. So if processTypeAnnotation() was in some common code path before the exporters, it would need to generate both HTML and markdown.

@wkeese
Copy link
Collaborator

wkeese commented Apr 29, 2013

PS: A more radical approach would be that every data type gets exported into details.xml as an object. For example, that CurrencyTextBox's __Constraints datatype would appear as it's own object in the details.xml, the same way as NumberTextBox's __Constraints does, as opposed to having it's definition inlined into the page for CurrencyTextBox.

That's probably a better solution because of the consistency, and because then you don't need any special HTML-generating or markdown-generating javascript code.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants