Opened 11 years ago

Closed 11 years ago

Last modified 11 years ago

#5264 closed defect (fixed)

blank line does not terminate keyword doc comment

Reported by: simonjb Owned by: Neil Roberts
Priority: high Milestone: 1.1
Component: Doc parser Version: 1.0
Keywords: Cc: dante
Blocked By: Blocking:

Description

In dijit.wai.onload (dijit/trunk/_base/wai.js) the blank line following the "description" keyword text does not terminate the text. In util/docscripts/_browse.php the description for dijit.wai.onload contains the text of the comment following the blank line.

dijit.wai = {
    onload: function(){
      // summary:
      //   Function that detects if we are in high-contrast mode or not,
      //   and sets up a timer to periodically confirm the value.
      //   figure out the background-image style property
      //   and apply that to the image.src property.
      // description:
      //   This must be a named function and not an anonymous
      //   function, so that the widget parsing code can make sure it
      //   registers its onload function after this function.
      //   DO NOT USE "this" within this function.

      // create div for testing if high contrast mode is on or images are turned off
      var div = document.createElement("div");

In util/docscripts/_browse.php the description includes "create div for..."

Attachments (2)

screenshot.jpeg (28.9 KB) - added by Neil Roberts 11 years ago.
Results from API Preview tool
screenshot1.jpeg (26.1 KB) - added by Neil Roberts 11 years ago.
Source code showing invisible characters

Download all attachments as: .zip

Change History (12)

comment:1 Changed 11 years ago by dante

Milestone: 1.1

comment:2 Changed 11 years ago by simonjb

Summary: blank line does terminate keyword doc commentblank line does not terminate keyword doc comment

comment:3 Changed 11 years ago by Neil Roberts

Resolution: invalid
Status: newclosed

There was likely a whitespace character on that line.

comment:4 Changed 11 years ago by Adam Peller

do the docs clearly state that the syntax is whitespace sensitive? Does it have to be? I'd argue that it shouldn't be, since JavaScript? isn't.

comment:5 Changed 11 years ago by simonjb

Resolution: invalid
Status: closedreopened

Maybe you could test it before asserting that the ticket is invalid?

Current doc for dijit.wai.onload:

dijit.wai = {
	onload: function(){
		// summary:
		//		Detects if we are in high-contrast mode or not,
		//		and sets up a timer to periodically check.

		// This must be a named function and not an anonymous
		// function, so that the widget parsing code can make sure it
		// registers its onload function after this function.
		// DO NOT USE "this" within this function.

util/docscripts/_browse.php?ns=dijit&file=_base/wai.js gives summary as:

"Detects if we are in high-contrast mode or not, and sets up a timer to periodically check.

This must be a named function and not an anonymous function, so that the widget parsing code can make sure it registers its onload function after this function. DO NOT USE "this" within this function."

There is no whitespace on the blank line separating the summary text from the comment following.

Changed 11 years ago by Neil Roberts

Attachment: screenshot.jpeg added

Results from API Preview tool

Changed 11 years ago by Neil Roberts

Attachment: screenshot1.jpeg added

Source code showing invisible characters

comment:6 Changed 11 years ago by Neil Roberts

Resolution: fixed
Status: reopenedclosed

To definitively make my point, I did test it before asserting the ticket was invalid. I'm not an idiot.

I've gone through the trouble of attaching screenshots showing that this does, in fact, parse properly and that wai.js does, in fact, have no whitespace on that line.

comment:7 Changed 11 years ago by Adam Peller

Resolution: fixed
Status: closedreopened

Neil, we discussed this yesterday. It's just not going to be obvious to any programmer that this is how the parser works. Can we restate this very clearly in the docs about the difference between a blank line and a really blank line? According to the current docs, I don't think it's fair to tell Simon he did anything wrong.

Thinking this over, I'd really rather see no distinction between blank and whitespace. Is this necessary? Could we just require a comment to make the blocks contiguous?

comment:8 Changed 11 years ago by Neil Roberts

The Skill of Comprehension:

simonjb starts by saying: "the blank line following the 'description' keyword text does not terminate the text" pottedmeat closes it as invalid, since it was invalid: a blank line does terminate the text (this was tested, of course) simonjb accuses pottedmeat of not testing the bug before it was marked invalid simonjb reiterates that the blank line is not terminating, saying that when he tested it in _browse.php, the blank line didn't stop the first comment block from joining with the second pottedmeat, using the same _browse.php file that simonjb used, posts screenshots, showing that the blank line does terminate the comment block

Conclusion

The bug as filed was that the blank-line terminator is not working. Except that the blank-line terminator is working, and the results of this exact comment block can be seen in our API viewer.

simonjb did not expect the 2 blocks to be joined together. In fact, that would have been the opposite of simonjb's desires. simonjb did absolutely nothing incorrect anywhere in this ticket. He expected the parser to do something that it should do. I don't know why his experiences don't line up with testing locally, on the nightlies, or on the API viewer, but either way it's not a parser bug.

comment:9 Changed 11 years ago by Neil Roberts

Resolution: fixed
Status: reopenedclosed

(In [12693]) Fixes #5264. Handle carriage returns when splitting lines.

comment:10 Changed 11 years ago by simonjb

I confirm that [12693] fixes the issue for me. Thanks :-)

Note: See TracTickets for help on using tickets.