fullDocs option for doc_comment

[Sevin777]

Closes #411

Updated demo with default value for fullDocs to make it easy for users
to understand. Added documentation to manual.txt (I don't know how to run the Makefile to regenerate the manual.html file - I'm using Windows).

[Sevin777]

@p-bakker you are right! I didn't think about this before but it makes a lot of sense.

I made a few changes including changes to parsing when not using fullDocs (see code comments).

[Sevin777]

This removes all asterisks that are part of the JSDoc structure except the first one (which is removed later). This is important as it keeps asterisks that are part of the documentation; example of asterisk in documentation:

 /**
  * Solves equations of the form a * x = b
  */

This also preserves the newlines inside of the comments.

[p-bakker]

Did a quick test and for me the code is now failing in some scenario's:

• The first regex you use (/\n {0,}* {0,}/g) doesn't take into account tabs (\t's) being used to indent the entire JSDoc comment (which happens for nested code)
• The first regex you use (/\n {0,}* {0,}/g) strips out all leading spaces after the *, which shouldn't happen IMHO, because the content might intentionally be indented (for example the value of an @example tag, which can be spread out over multiple lines)

There might be a good reason why you deviated from using the regex I suggested (/^(\ |\t)\ ?\r?\n?/gm), but at least that one got above two points right

I tried to figure out what the last regex is for (/^\s{0,}*{1,}\s{0,}/), but couldn't figure it out. And I didn't check the changes made to non-fullDocs variant

Maybe it would be good to have some tests for this. Some of the testcases could be:

/**
 * @param {String} name 
 * @return {String}
 */

    /**
     * @param {String} name 
     * @return {String}
     */

/**
 * @example <pre> //Example of extending the base class
 * /**
 *  * Extended Event class that also exposes getPosition
 *  * @private
 *  * @constructor 
 *  * @extends {EventManager.Event}
 *  *
 *  * @param {String} type
 *  * @param {*} source
 *  * @param {{
 *  *   x: Number, 
 *  *   y: Number
 *  * }} [position]
 *  * @param {Object} [data]
 *  */
 *  function Event(type, source, position, data) {
 *      EventManager.Event.call(this, type, source, data); //Applying the arguments to the base class constructor
 *      
 *      this.getPosition = function() {
 *          return position||null;
 *      }
 *  }
 *
 */

[Sevin777]

fullDocs Purpose

I think the purpose of the fullDocs option should be to allow the editor's tern plugin to decide how to handle the comments. This means the returned comments from tern should contain the original comments (including all spacing) minus the comment structure. Example:

/**
 * sentence one. sentence two.
 *      Indented sentence.
 * @param {bool} [p1=false] - param 1
 */
function test1() {}

should return:

sentence one. sentence two.
     Indented sentence.
@param {bool} [p1=false] - param 1

Updates

• The first regex you use (/\n {0,}* {0,}/g) doesn't take into account tabs (\t's) being used to indent the entire JSDoc comment (which happens for nested code)

@p-bakker thankyou for pointing this out. I just updated it to remove any combination of tabs and spaces before the jsdoc newline comment start.

• The first regex you use (/\n {0,}* {0,}/g) strips out all leading spaces after the *, which shouldn't happen IMHO, because the content might intentionally be indented (for example the value of an @example tag, which can be spread out over multiple lines)

@p-bakker You are correct again. The updated regex only removes the first whitespace after the jsdoc newline comment start.

I tried to figure out what the last regex is for (/^\s{0,}*{1,}\s{0,}/), but couldn't figure it out. And I didn't check the changes made to non-fullDocs variant

@p-bakker scroll up in this pull request and read the comments on the changed line. It simply removes any preceding asterisks from the result.

Maybe it would be good to have some tests for this. Some of the testcases could be:

@p-bakker the test case you presented with the nested example is invalid javascript (it has block comment end inside of block comment which causes premature comment ending). Tests would be ideal but I'm not exactly sure how to write them for tern as its not well documented. I use the code below for testing (manually):

//paste this code in tern demo with fullDocs=true


test //start auto complete here


/**
 * sentence one. sentence two.
 *      Indented sentence.
 * @param {bool} [p1=false] - param 1
 */
function test1() {}
/**
 * @param {String} name 
 * @return {String}
 * @example
 *      test2(1) * 1 = undefined
 */
function test2() {}
/** test function 4*/
function test4() {}
//test function 5
//   line two of test 5.
//line 3 (not indented)
function test5() {}

[p-bakker]

Ah, right, I did something wrong there. The nested blockcomment should be done like this:

 * @example <pre> //Example of extending the base class
 * &#47;**
 *  * Extended Event class that also exposes getPosition
 *  * @private
 *  * @constructor 
 *  * @extends {EventManager.Event}
 *  *
 *  * @param {String} type
 *  * @param {*} source
 *  * @param {{
 *  *   x: Number, 
 *  *   y: Number
 *  * }} [position]
 *  * @param {Object} [data]
 *  *&#47;
 *  function Event(type, source, position, data) {
 *      EventManager.Event.call(this, type, source, data); //Applying the arguments to the base class constructor
 *      
 *      this.getPosition = function() {
 *          return position||null;
 *      }
 *  }
 *
 */

[Sevin777]

https://github.com/sevin7676
@p-bakker your example works for me (you are missing the start of the jsdoc comment)

test code:

//paste this code in tern demo with fullDocs=true


test //start auto complete here


     /**
      * This comment block is indented with both tabs and spaces.
      *     @param {bool} [p1=false] - param 1
      */
function test1() {}
/**
 * @param {String} name 
 * @return {String}
 * @example
 *      test2(1) * 1 = undefined
 */
function test2() {}
/** test function 4*/
function test4() {}
//test function 5
//   line two of test 5.
//line 3 (not indented)
function test5() {}
/**
 * @example <pre> //Example of extending the base class
 * &#47;**
 *  * Extended Event class that also exposes getPosition
 *  * @private
 *  * @constructor 
 *  * @extends {EventManager.Event}
 *  *
 *  * @param {String} type
 *  * @param {*} source
 *  * @param {{
 *  *   x: Number, 
 *  *   y: Number
 *  * }} [position]
 *  * @param {Object} [data]
 *  *&#47;
 *  function Event(type, source, position, data) {
 *      EventManager.Event.call(this, type, source, data); //Applying the arguments to the base class constructor
 *      
 *      this.getPosition = function() {
 *          return position||null;
 *      }
 *  }
 *
 */
function test6() {}

The tern plugin is responsible for converting / back to / (CodeMirror's tern plugin doesn't do this)

[p-bakker]

The latest regex doesn't work if the whitespace preceding the * is "\t \t". This regex would work:

/\n[ \t]{0,}\* ?/g

Or simpler:

/\n[ \t]+\* ?/g

[Sevin777]

@p-bakker You are right again (I'm relatively new to JavaScript regex). I changed it to /\n[ \t]{0,}\* ?/g.

Or simpler: /\n[ \t]+\* ?/g

@p-bakker this one requires at least one space before the asterisk which could cause issues.

[p-bakker]

Think we're onto a winner now :-)

You're right about the simpler one not working all the time, forgot to test that scenario.

FYI: a handy tool to build and test regex: http://regex101.com/#javascript

[p-bakker]

This would be the correct simpler version:

/\n[ \t]*\* ?/g

[Sevin777]

@p-bakker I updated it with the even simpler regex. I use regex101 (its awesome!). Thanks for your help!

[marijnh]

Thanks. Merged as f9b553f and cleaned up in ec16bc7. The choice of using the last comment above a statement might cause some regressions (see example below, where the description of a function is spread over several comments), but in general it is probably a win.

// This is the function to frobnify a zork.

// See http://github.net/frob/zork/issue/2031 for background

function frobnify(zork) {}

[marijnh]

Just a heads-up: I've removed this option again in fd7a999 and made it the default behavior.

[marijnh]

And another heads-up, attached patch makes the default behavior to strip off long text again, and allows you to add a docFormat: "full" field to your queries when you want the full docstring (including newlines). This will require a change to your client if you were relying on these full strings.

[Sevin777]

Why? Is the full documentation causing performance issues? If not, shouldn't the client decide to trim the string as it sees fit? I'm not attacking your decision, just hoping for a little bit of an explanation as to why...

[marijnh]

The client is deciding to trim the string, by not passing that query parameter. The string trimming logic is involved enough (and likely to be refined in the future) that I don't want to duplicate it in every client. Also, it seems a good idea not to change the old default behavior. There's no performance issues, just that people were suddenly seeing @-jsdoc mess in their tooltips, and not happy about it.