Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Provide information about original source formatting #93

Closed
syavorsky opened this issue Sep 23, 2020 · 16 comments
Closed

Provide information about original source formatting #93

syavorsky opened this issue Sep 23, 2020 · 16 comments
Milestone

Comments

@syavorsky
Copy link
Owner

Supplying meta about white spaces would address #92, #79, and #51.

As per discussion in #92, following spec has emerged

 0|   /**
 1|    * Overall description
 2|    * 
 3|   *  another line
 4|    * 
 5|    * @param {Object} options
 6|    * @example default
 7|   this line has no *
 8|   * doSomething(options)
 9|     * doSomething(options)
10|    */
{
  description: '\nOverall description\n\n another line\n',
  line: 1,
  source: '\nOverall description\n\n another line\n\n@param {Object} options\n@example default\nthis line has no marker\ndoSomething(options)\ndoSomething(options)\n',
  tokens: {
    descr: [{ // line 1
      start:       '     ',
      delim:       '*',
      postdelim:   ' ',
      descr:      'Overall description'
    }, { // line 2, empty
      start:       '     ',
      delim:       '*',
    }, { // line 3, shifted
      start:       '    ',
      delim:       '*',
      postdelim:   '    ',
      descr:       'Overall description'
    }, { // line 4, empty
      start:       '     ',
      delim:       '*',
    }]
  },
  tags: [{
    description: '',
    line: 6,
    name: 'options',
    optional: false,
    source: '@param {Object} options',
    tag: 'param',
    type: 'Object',
    tokens: {
      start:     '     ',
      delim:     '*',
      postdelim: ' ',
      tag:       '@param',
      posttag:   ' ',
      type:      '{Object}',
      posttype:  ' ',
      name:      'options'
    }
  }, {
    description: 'this line has no marker\ndoSomething(options)\ndoSomething(options)\n',
    line: 7,
    name: 'default',
    optional: false,
    source: '@example default\nthis line has no marker\ndoSomething(options)\ndoSomething(options)\n',
    tag: 'example',
    type: '',
    tokens: {
      start:     '     ',
      delim:     '*',
      postdelim: ' ',
      tag:       '@example',
      posttag:   ' ',
      name:      'default',
      descr: [{
        // nothing, line break
      }, {
        start:     '    ',
        descr:     'this line has no marker'
      }, {
        start:     '    ',
        delim:     '*',
        postdelim: ' ',   
        descr:     'doSomething(options)'
      }, {
        start:     '      ',
        delim:     '*',
        postdelim: ' ',   
        descr:     'doSomething(options)'
      }]
    }
  }]
}
@syavorsky syavorsky mentioned this issue Sep 23, 2020
1 task
@syavorsky
Copy link
Owner Author

Just to give an update on this story. I am actively working on implementation and will have it available as different branch/version soon. Apparently, token approach greatly simplifies some flows layered over last few of years. There will be some breaking changes, not much, but major version would have to be bumped for compatibility safety.

@renatho
Copy link

renatho commented Nov 7, 2020

Hey @syavorsky!
Do you have an update on that? If I can help with any part, let me know!
Looking forward to this feature! 😀

@syavorsky
Copy link
Owner Author

@brettz9 @renatho yeah, sorry, it took forever. But seems like I finally put something together. Drifted away a bit from original idea as implementing, but mostly it is what was discussed above

Would appreciate if you give it a try. Please see the 1.0 branch. I have updated the README and added examples. Also, a bunch of unit tests added, so you can refer those too.

[email protected]

@syavorsky
Copy link
Owner Author

@renatho I took a pick on alignment, didn't mean to undermine what you are already doing, just wanted to understand if the new stringifier can be used for the purpose. Would be happy to take in a PR with improvements, I believe you put more thoughts into the problem and would suggest some tweaks

@brettz9
Copy link
Contributor

brettz9 commented Nov 16, 2020

This is great to see moving forward. Maybe a simple migration doc could help?

I see parsers may now be available as tokenizers, trim is no longer an option, and for the stringifier, there is no more indent option. Do you have plans to continue supporting these latter two options?

syavorsky added a commit that referenced this issue Nov 17, 2020
syavorsky added a commit that referenced this issue Nov 17, 2020
@syavorsky
Copy link
Owner Author

@brettz9 added some notes to put more light on this. I hope to make it better by having interactive examples page on github.io

@syavorsky
Copy link
Owner Author

syavorsky commented Nov 17, 2020

@brettz9 let me know what cases you have in mind that can't be handled with 1.0 API

I see parsers may now be available as tokenizers, trim is no longer an option, and for the stringifier, there is no more indent option. Do you have plans to continue supporting these latter two options?

I want to keep API compact but sufficient, and rather provide recipes for all possible customization on top

@renatho
Copy link

renatho commented Nov 18, 2020

Awesome! Thank you for work on that @syavorsky! I did some tests and it's great.

I'm just wondering about some different tags, like the @return. See the following example:

/**
 * Description may go 
 * over few lines followed by @tags
 * @param {string} name the name parameter
 * @param {any} value the value parameter
 * @return {string} Lorem ipsum dolor sit amet
 */

It's parsing the "Lorem" as the name, and "ipsum dolor sit amet" as the description. It also happens for any other different tag (@todo, @see, and so on). It also makes the stringify give some weird results. I thought that we could create some exceptions for a version 1, maybe only parsing the tags we're sure that works, otherwise, I think we should handle all possible exceptions. WDYT?

just wanted to understand if the new stringifier can be used for the purpose

Totally! Probably we can parse and compare with the comment being validated. If it's not equal is because it's wrong. And if it's wrong we can also use it as the fixer. 😊 And a prettier plugin can also be created using this.

I see parsers may now be available as tokenizers, trim is no longer an option, and for the stringifier, there is no more indent option. Do you have plans to continue supporting these latter two options?

And out of curiosity. @brettz9, could these items that you commented to be a barrier to update it in the eslint-plugin-jsdoc?

@brettz9
Copy link
Contributor

brettz9 commented Nov 18, 2020

I see parsers may now be available as tokenizers, trim is no longer an option, and for the stringifier, there is no more indent option. Do you have plans to continue supporting these latter two options?

And out of curiosity. @brettz9, could these items that you commented to be a barrier to update it in the eslint-plugin-jsdoc?

Yes, we need them all for eslint-plugin-jsdoc. As I may find the energy, I hope to see about updating the API, reapplying the behavior in our package, and see if any basic hooks might be missing/useful. I personally would think that at least the indent feature would be a pretty common use case for stringifying, but I can understand the rationale for offloading features to consumers.

@syavorsky
Copy link
Owner Author

@brettz9 let me think this over, indent does seems like a generic enough to be available somehow.

Right now stringify({format: "align"}) is just a shortcut and could be written as {format: alignFormatter()}. Maybe, it should turn into alignFormatter(options).

@jaydenseric
Copy link
Contributor

Please, for code locations use the Babel AST structure, e.g.

{
  "start": {
    "line": 1,
    "column": 0
  },
  "end": {
    "line": 1,
    "column": 6
  }
}

Screen Shot 2020-12-15 at 10 54 51 am

Having the same format as the Babel AST is useful; it's not only familiar, but you can also pass location data straight into @babel/code-frame to create error messages with the code location marked.

I really need a way to get this start and end line and column information from comment-parser regarding JSDoc tag type and name parts to be able to make error messages like this:

Screen Shot 2020-12-15 at 10 52 14 am

Like with the Babel AST (or most AST's), all the JSDoc parts (tag, name, type, description) should have start and end location data capable of describing an exact range across potentially multiple lines. Ideally, it's not just the char positions because figuring out later what lines certain char positions are on is fiddly.

@jaydenseric
Copy link
Contributor

Also, it would be nice to be able to pass a start location line and column to comment-parser so that all the location data of the JSDoc parts could be adjusted accordingly, to reflect their real positions in the source code of the whole file.

@syavorsky
Copy link
Owner Author

@jaydenseric I see your point about aligning with Babel AST, that makes sense indeed. However, I would rather bundle a mapper function allowing you to convert one to another.

And start line parameter is already in

@jaydenseric
Copy link
Contributor

@syavorsky

And start line parameter is already in

Step in the right direction, although it's not documented yet and a start column is also necessary. Comments can come from somewhere indented or with code to the left.

I need the features I described earlier right now for features coming to the next major jsdoc-md release, so I have begun work on a new comment parsing lib. Some time ago I started a similar effort, but that attempt was regex based and while at first it was looking super elegant it reached some technical dead ends. This time I'm looping all the characters.

@syavorsky
Copy link
Owner Author

@jaydenseric column information should also be available through the mapper.

1.0 is being worked on, it does better job on describing the input source, hopefully that would fit better with what you are building. I am trying to finish documentation and usage examples.

syavorsky added a commit that referenced this issue Jan 6, 2021
@syavorsky
Copy link
Owner Author

1.0 went to the master and is current stable version now. Please see updated README and find data types discussed above in src/primitives.ts

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

No branches or pull requests

4 participants