Opened 6 years ago

Closed 6 years ago

Last modified 6 years ago

#18002 closed defect (fixed)

dojo doc breaking js-doc-parse

Reported by: lee Owned by:
Priority: undecided Milestone: 1.10
Component: Documentation Version: 1.10.0-beta1
Keywords: Cc: Sebastien Pereira, Patrick Ruzand, Kris Zyp
Blocked By: Blocking:

Description

I've tried running js-doc-parse against dojo-release-1.10.0-beta1-src and 4 comment issues cause the doc parser to break.

They are minor fixes, indentation, block comments needed etc but will need to be fixed before release. If I get time I'll submit the PR's myself but if anyone else is currently working on these components, can you make the following changes? :


dojox/mobile/scrollable.js

ERR: dojox/mobile/scrollable.js:1288:49 TypeError?: Cannot read property '1' of null

(line 1281) So I fixed up locally the inline comments to a block comment e.g.

						/* #17822: when scrolling on one direction, avoid unnecessarily animating
							both top and left, because this leads to two transitionend events fired
							instead of one in some browsers (Safari/iOS7 at least). */


dojox/charting/action2d/TouchIndicator.js

ERR: dojox/charting/action2d/TouchIndicator.js:110:20 TypeError?: Cannot read property '1' of null

(Line 117) I fixed locally indentation to:

			// 		we should not have to do that, but dojo/touch is causing performances issue
			// 		we have to workaround here for now

dojox/store/db/IndexedDB.js ERR: dojox/store/db/IndexedDB.js:186:16 TypeError?: Cannot read property '1' of null

(Line 186). Not sure why the object's props are commented out, maybe should be in an example tag instead? I just locally added block comments to get it to parse e.g. :

		indices: {
			/* property: {
					preference: 1,
					multiEntry: true
			 } */
		},

dojox/mobile/migrationAssist.js

ERR: dojox/mobile/migrationAssist.js:323:27 TypeError?: Cannot call method 'mixinProperties' of undefined

I guess https://github.com/dojo/dojox/commit/65a4f1dd2abdd598f5476e2cf2131771914c5c3d#diff-fefdc9df5f71203b7e0916fc24b1ba1dR316 caused this? I undid this and it worked e.g. changing back to

	extendSelectFunction = function(obj) {

I don't think this should be in the API anyway so instead I added an ignore to js-doc-parse/config.js excludes e.g. :

		// not to be documented?
		/dojox\/mobile\/migrationAssist\.js/,

Change History (15)

comment:1 Changed 6 years ago by bill

Cc: Sebastien Pereira Patrick Ruzand Kris Zyp added

comment:2 Changed 6 years ago by Patrick Ruzand

I'll fix the dojox/mobile issues.

comment:3 Changed 6 years ago by Patrick Ruzand

Note that I don't understand why such multiple single-line comments need to be changed to block commnets:

	domStyle.set(node, css3.add({}, {
		// #17822: when scrolling on one direction, avoid unnecessarily animating 
		// both top and left, because this leads to two transitionend events fired 
		// instead of one in some browsers (Safari/iOS7 at least).
		transitionProperty: (to.x !== undefined && to.y !== undefined) ?

There are multiple cases in Dojo where successive single line comments is not an issue for js-doc-parse. Why this one ? There's obviously something I am missing...

comment:4 Changed 6 years ago by lee

I wanted js-doc-parse to run so I didn't look long at it, it's probably just indentation

comment:5 Changed 6 years ago by dylan

I would guess that it's looking at your #17822: and trying to parse it because of the : (that's the pattern that is used for things like summary, description, TODO, etc. So I would remove that and see if it parses.

comment:6 Changed 6 years ago by bill

Plus it's directly after a brace, it looks a lot like:

dojo.declare(..., {
     // summary: ...

comment:7 in reply to:  5 Changed 6 years ago by Patrick Ruzand

Replying to dylan:

I would guess that it's looking at your #17822: and trying to parse it because of the :

The ':' was indeed the issue. thx dylan !

comment:8 Changed 6 years ago by Patrick Ruzand

re: dojo/mobile/migrationAssist, it should indeed be excluded from the api doc.

comment:9 Changed 6 years ago by Patrick Ruzand <pruzand@…>

In 87ac7b3c5c2034d6e3c0baa17dcc14d459f1a5b2/dojox:

Error: Processor CommitTicketReference failed
Unsupported version control system "git": Can't find an appropriate component, maybe the corresponding plugin was not enabled? 

comment:10 Changed 6 years ago by Patrick Ruzand

The changeset above fixes dojox/charting/action2d/TouchIndicator.js and dojox/mobile/scrollable.js issues.

comment:11 in reply to:  8 Changed 6 years ago by bill

Replying to pruzand:

re: dojo/mobile/migrationAssist, it should indeed be excluded from the api doc.

Hmm, I thought we had done that, but it's not listed in https://github.com/wkeese/js-doc-parse/blob/master/config.js.

Last edited 6 years ago by bill (previous) (diff)

comment:12 Changed 6 years ago by Bill Keese <bill@…>

In 6799138338f33870d999fed20bc91be6527dd456/dojox:

Error: Processor CommitTicketReference failed
Unsupported version control system "git": Can't find an appropriate component, maybe the corresponding plugin was not enabled? 

comment:13 Changed 6 years ago by bill

Milestone: tbd1.10
Resolution: fixed
Status: newclosed

I excluded migrationAssist.js from scanning in https://github.com/wkeese/js-doc-parse/commit/acfdc99508e20ad7bb16ccdd2f1c5dbe6eff2f73. Parsing seems to work now.

comment:14 Changed 6 years ago by Bill Keese <bill@…>

In bedbf76db2d04766de8ad4fa26a48af3041128e0/dojox:

Error: Processor CommitTicketReference failed
Unsupported version control system "git": Can't find an appropriate component, maybe the corresponding plugin was not enabled? 

comment:15 Changed 6 years ago by Bill Keese <bill@…>

In fc11ab64523ca3eb08d5eedf82b979d086b06eff/dojox:

Error: Processor CommitTicketReference failed
Unsupported version control system "git": Can't find an appropriate component, maybe the corresponding plugin was not enabled? 
Note: See TracTickets for help on using tickets.