Include comments if available? #10

Open
papandreou opened this Issue May 4, 2012 · 19 comments

Comments

Projects
None yet
8 participants

I don't know if it's even possible based on the output of require('esprima').parse(str, {comment: true}), but just in case.

I have more use cases for comment-and-whitespace-preserving JavaScript source manipulation than I can count :)

Owner

Constellation commented May 4, 2012

In esprima, now we're discussing about attaching comments to AST.
http://code.google.com/p/esprima/issues/detail?id=197

So if it is available, escodegen can attach comments to its output!

@constellation: Ah, I didn't notice that issue, thanks for the link. Great news!

I'd love to see support for comments as well. I just wrote a script to convert our AMD codebase into CommonJS modules, but just when I was done, I realized that I can't use it because the comments will be stripped out : /.

Owner

Constellation commented May 20, 2012

OK, we'll implement experimental version.
But take care that this is experimental.
If esprima is changed the direction of comment, we maybe follow to it.
So if you use it, we suggest check escodegen and esprima version.

Constellation reopened this May 20, 2012

Owner

Constellation commented May 20, 2012

Oops, sorry, I pushed "Close & comment" button incorrectly.

Owner

Constellation commented May 20, 2012

Above commit makes statement level comment possible.

Currently, you can use it

var tree = esprima.parse(code, { range: true, token: true, comment: true });
tree = escodegen.attachComments(tree, tree.comments, tree.tokens);
var output = escodegen.generate(tree);

Of cource, this is still development version, so if you have any idea about API, format, multiline comment, expression comment, indentation adjustment of multiline comment and others, I appreciate if you comment your opinion or create pull requests.

Wow, thanks a lot! I ran it on my project and it seems to work great. Maybe I'll have more comments later.

Owner

Constellation commented May 20, 2012

I fixed some bugs in above commit and add new option, 'format.indent.adjustMultilineComment'.

If you provide this option, for example,

var tree = esprima.parse(code, { range: true, token: true, comment: true });
tree = escodegen.attachComments(tree, tree.comments, tree.tokens);
var output = escodegen.generate(tree, { format: { indent: { adjustMultilineComment: true } } });

When you convert such a 2-space script to 4-space script,

function test() {
  /*
   * comment
   */
  var i = 20;
}

escodegen automatically adjusts multiline comment base indentation(2-space to 4-space) so output is below.

function test() {
    /*
     * comment
     */
    var i = 20;
}

not following.

function test() {
    /*
   * comment
   */
    var i = 20;
}
Owner

Constellation commented May 20, 2012

And demonstration. :-)
This is 2-space converted jQuery source code from jQuery development version.

https://gist.github.com/2757675

Constellation referenced this issue in jashkenas/coffeescript Jun 3, 2012

Closed

Single-line comments in output #2365

Owner

Constellation commented Jun 3, 2012

Now, escodegen requires option.comment,
If option.comment provided and comment attached, escodegen generate comment attached code.

Owner

Constellation commented Jun 7, 2012

We released 0.0.4, but this function is still experimental, so API will change.
Probably, for more precise and good comment attaching, we will require source code text.

So if we change this API, we should write new one to this issue.

Owner

Constellation commented Jul 17, 2012

We should add line terminator check to attachComments.

For example,

var i = 10;    /*
                * this is trailing comment to i = 10
                */
var i2 = 20;

But

var i = 10;
/*
 * this is leading comment to var i2 = 20
 */
var i2 = 20;
Contributor

RGustBardon commented Jul 19, 2012

What follows might be helpful when generating expression-level comments.

var singleLineComments = [
    '//A\n{//B\n}//C\n//D\n',
    '//A\n{//B\n42//C\n}//D\n//E\n',
    '//A\n[//B\n]//C\n//D\n',
    '//A\n[//B\n,//C\n42//D\n]//E\n//F\n',
    '//A\n!//B\n{//C\n}//D\n//E\n',
    '//A\n!//B\n{//C\n42//D\n://E\n42//F\n,//G\nfoo//H\n://I\n42//J\n}//K\n//L\n',
    '//A\nswitch//B\n(//C\n42//D\n)//E\n{//F\n}//G\n//H\n',
    '//A\nswitch//B\n(//C\n42//D\n)//E\n{//F\ndefault//G\n://H\n42//I\n}//J\n//K\n',
    '//A\n42//B\n;//C\n42//D\n//E\n',

    '//A\n!//B\nfunction//C\n(//D\nfoo//E\n,//F\nbar//G\n)//H\n{//I\n}//J\n//K\n',
    '//A\n42//B\n,//C\n42//D\n//E\n',
    '//A\nfoo//B\n=//C\n42//D\n//E\n',
    '//A\n42//B\n?//C\n42//D\n://E\n42//F\n//G\n',
    '//A\nfoo//B\n(//C\n42//D\n,//E\n42//F\n)//G\n//H\n',
    '//A\nnew//B\nFoo(//C\n42//D\n,//E\n42//F\n)//G\n//H\n',
    '//A\n//B\n!//C\n{//D\n42//E\n://F\n42//G\n}//H\n//I\n',
    '//A\ndo//B\n{//C\n}//D\nwhile//E\n(//F\n42//G\n)//H\n;//I\n//J\n',
    '//A\ntry//B\n{//C\n}//D\ncatch//E\n(//F\nfoo//G\n)//H\n{//I\n}//J\n//K\n',
    '//A\nvar//B\nfoo//C\n=//D\n42//E\n//F\n',
    '//A\nvar//B\nfoo//C\n,//D\nbar//E\n//F\n',
    '//A\ntry//B\n{//C\n}//D\nfinally//E\n{//F\n}//G\n//H\n',
    '//A\nswitch//B\n(//C\nfoo//D\n)//E\n{//F\n}//G\n//H\n',
    '//A\nif//B\n(//C\n42//D\n)//E\n{//F\n}//G\n//H\n',
    '//A\nif//B\n(//C\n42//D\n)//E\n{//F\n}//G\nelse//H\nif//I\n(//J\n42//K\n)//L\n{//M\n}//N\n//O\n',
    '//A\nif//B\n(//C\n42//D\n)//E\n{//F\n}//G\nelse//H\n{//I\n}//J\n//K\n',
    '//A\nfor//B\n(//C\n;//D\n42//E\n;//F\n42//G\n)//H\n;//I\n//J\n',
    '//A\nfor//B\n(//C\nfoo//D\nin//E\n42//F\n)//G\n;//H\n//I\n',
    '//A\nwhile//B\n(//C\n42//D\n)//E\n;//F\n//G\n',
    '//A\nwith//B\n(//C\n{//D\n}//E\n)//F\n;//G\n//H\n',

    '//A\nfoo//B\nin//C\nbar//D\n,//E\n[//F\n]//G\nin//H\n[//I\n]//J\n//K\n',
    '//A\nnew//B\nFoo//C\n(//D\n)//E\n,//F\nnew//G\n[//H\n]//I\n(//J\n)//K\n//L\n',
    '//A\ntypeof//B\n42//C\n,//D\ntypeof//E\n[//F\n]//G\n//H\n',
    '//A\nthrow 42//B\n;//C\nthrow[//D\n]//E\n//F\n',
    '//A\nswitch//B\n(//C\n42//D\n)//E\n{//F\ncase//G\n42//H\n://I\ncase//J\n[//K\n]//L\n://M\n}//N\n//O\n',
    '//A\nif//B\n(//C\n42//D\n)//E\n{//F\n}//G\nelse//H\n42//I\n;//J\nif//K\n(//L\n42//M\n)//N\n{//O\n}//P\nelse//Q\n[//R\n]//S\n;//T\n//U\n',
    '//A\nfor//B\n(foo//C\nin//D\n42//E\n)//F\n;//G\nfor//H\n(//I\nfoo//J\nin//K\n[//L\n]//M\n)//N\n;//O\n',
    '//A\n!//B\nfunction//C\n(//D\n)//E\n{//F\nreturn 42//H\n}//I\n,//J\nfunction//K\n(//L\n)//M\n{//N\nreturn[//O\n]//P\n}//Q\n//R\n',

    '//A\n!//B\nfunction//C\nfoo//D\n(//E\n)//F\n{//G\n}//H\n//I\n',
    '//A\n!//B\n{//C\nget//D\n42//E\n(//F\n)//G\n{//H\n}//I\n,//J\nset//K\n42//L\n(//M\nbar//N\n)//O\n{//P\n}//Q\n}//R\n//S\n',
    '//A\nfoo//B\n://C\nwhile//D\n(//E\n42//F\n)//G\nbreak foo//H\n;//I\n//J\n',
    '//A\nfoo//B\n://C\nwhile//D\n(//E\n42//F\n)//G\ncontinue foo//H\n;//I\n//J\n',
    '//A\nvar//B\nfoo//C\n//D\n',
    '//A\nfor//B\n(//C\nvar//D\nfoo//E\nin//F\n42)//G\n;//H\n//I\n',
    '//A\nfunction//B\nfoo//C\n(//D\n)//E\n{//F\n}//G\n//H\n',

    '//A\n(//B\n{//C\n}//D\n)//E\n//F\n',
    '//A\n(//B\nfunction//C\n(//D\n)//E\n{//F\n}//G\n)//H\n//I\n',
    '//A\n{//B\n42//C\n}//D\n//E\n',
    '//A\n[//B\n]//C\n//D\n',
    '//A\n[//B\n,//C\n42//D\n]//E\n//F\n',
    '//A\n!//B\n{//C\n}//D\n//E\n',
    '//A\n!//B\n{//C\n42//D\n://E\n42//F\n,//G\nfoo//H\n://I\n42//J\n}//K\n//L\n',
    '//A\nswitch//B\n(//C\n42//D\n)//E\n{//F\n}//G\n//H\n',
    '//A\nswitch//B\n(//C\n42//D\n)//E\n{//F\ndefault//G\n://H\n42//I\n}//J\n//K\n',
    '//A\n42//B\n;//C\n42//D\n//E\n',

    '//A\n!//B\nfunction//C\n(//D\nfoo//E\n,//F\nbar//G\n)//H\n{//I\n}//J\n//K\n',
    '//A\n42//B\n,//C\n42//D\n//E\n',
    '//A\nfoo//B\n=//C\n42//D\n//E\n',
    '//A\n42//B\n?//C\n42//D\n://E\n42//F\n//G\n',
    '//A\nfoo//B\n(//C\n42//D\n,//E\n42//F\n)//G\n//H\n',
    '//A\nnew//B\nFoo(//C\n42//D\n,//E\n42//F\n)//G\n//H\n',
    '//A\n//B\n!//C\n{//D\n42//E\n://F\n42//G\n}//H\n//I\n',
    '//A\ndo//B\n{//C\n}//D\nwhile//E\n(//F\n42//G\n)//H\n;//I\n//J\n',
    '//A\ntry//B\n{//C\n}//D\ncatch//E\n(//F\nfoo//G\n)//H\n{//I\n}//J\n//K\n',
    '//A\nvar//B\nfoo//C\n=//D\n42//E\n//F\n',
    '//A\nvar//B\nfoo//C\n,//D\nbar//E\n//F\n',
    '//A\ntry//B\n{//C\n}//D\nfinally//E\n{//F\n}//G\n//H\n',
    '//A\nswitch//B\n(//C\nfoo//D\n)//E\n{//F\n}//G\n//H\n',
    '//A\nif//B\n(//C\n42//D\n)//E\n{//F\n}//G\n//H\n',
    '//A\nif//B\n(//C\n42//D\n)//E\n{//F\n}//G\nelse//H\nif//I\n(//J\n42//K\n)//L\n{//M\n}//N\n//O\n',
    '//A\nif//B\n(//C\n42//D\n)//E\n{//F\n}//G\nelse//H\n{//I\n}//J\n//K\n',
    '//A\nfor//B\n(//C\n;//D\n42//E\n;//F\n42//G\n)//H\n;//I\n//J\n',
    '//A\nfor//B\n(//C\nfoo//D\nin//E\n42//F\n)//G\n;//H\n//I\n',
    '//A\nwhile//B\n(//C\n42//D\n)//E\n;//F\n//G\n',
    '//A\nwith//B\n(//C\n{//D\n}//E\n)//F\n;//G\n//H\n',

    '//A\nfoo//B\nin//C\nbar//D\n,//E\n[//F\n]//G\nin//H\n[//I\n]//J\n//K\n',
    '//A\nnew//B\nFoo//C\n(//D\n)//E\n,//F\nnew//G\n[//H\n]//I\n(//J\n)//K\n//L\n',
    '//A\ntypeof//B\n42//C\n,//D\ntypeof//E\n[//F\n]//G\n//H\n',
    '//A\nthrow 42//B\n;//C\nthrow[//D\n]//E\n//F\n',
    '//A\nswitch//B\n(//C\n42//D\n)//E\n{//F\ncase//G\n42//H\n://I\ncase//J\n[//K\n]//L\n://M\n}//N\n//O\n',
    '//A\nif//B\n(//C\n42//D\n)//E\n{//F\n}//G\nelse//H\n42//I\n;//J\nif//K\n(//L\n42//M\n)//N\n{//O\n}//P\nelse//Q\n[//R\n]//S\n;//T\n//U\n',
    '//A\nfor//B\n(foo//C\nin//D\n42//E\n)//F\n;//G\nfor//H\n(//I\nfoo//J\nin//K\n[//L\n]//M\n)//N\n;//O\n',
    '//A\n!//B\nfunction//C\n(//D\n)//E\n{//F\nreturn 42//H\n}//I\n,//J\nfunction//K\n(//L\n)//M\n{//N\nreturn[//O\n]//P\n}//Q\n//R\n',

    '//A\n!//B\nfunction//C\nfoo//D\n(//E\n)//F\n{//G\n}//H\n//I\n',
    '//A\n!//B\n{//C\nget//D\n42//E\n(//F\n)//G\n{//H\n}//I\n,//J\nset//K\n42//L\n(//M\nbar//N\n)//O\n{//P\n}//Q\n}//R\n//S\n',
    '//A\nfoo//B\n://C\nwhile//D\n(//E\n42//F\n)//G\nbreak foo//H\n;//I\n//J\n',
    '//A\nfoo//B\n://C\nwhile//D\n(//E\n42//F\n)//G\ncontinue foo//H\n;//I\n//J\n',
    '//A\nvar//B\nfoo//C\n//D\n',
    '//A\nfor//B\n(//C\nvar//D\nfoo//E\nin//F\n42)//G\n;//H\n//I\n',
    '//A\nfunction//B\nfoo//C\n(//D\n)//E\n{//F\n}//G\n//H\n',

    '//A\n(//B\n{//C\n}//D\n)//E\n//F\n',
    '//A\n(//B\nfunction//C\n(//D\n)//E\n{//F\n}//G\n)//H\n//I\n'
];

It might be reasonable to defer testing options.format.compact with comments until expression-level comments are supported and issues #20, #21, and #22 are resolved.

It might be useful to follow the progress on Esprima issue 276.

It would be useful to have a function that returns true when a statement or an expression for which the code is being generated is preceded by a line separator. For instance:

false/**///
/**/true

would lead to false for false and true for true. Similarly:

false/**//*
*//**/true

However,

false/**/,/**/false

Is there a plan to have this change in for 1.1.0, or is it blocked by the AST change?

Owner

Constellation commented Oct 31, 2012

Statement level comment is already supported by Escodegen :-)

Contributor

mathiasbynens commented Mar 31, 2013

It would be useful to be able to only preserve /*! … */-style license header comments. Should I file a new issue for that?

tschaub commented Jun 4, 2013

var tree = esprima.parse(code, { range: true, token: true, comment: true });

I'm guessing this should be tokens instead:

var tree = esprima.parse(code, { range: true, tokens: true, comment: true });

Any changes to the syntax for attaching comments or generating code with comments? I'm using escodegen@0.0.22 and esprima@1.0.3. It looks like leadingComments are attached to the tree, but I don't see them in the generated source.

sample code.js:

var foo;

/**
 * @constructor
 * @param {string} name Bar.
 */
function Bar(name) {
  this.name = name;
}

and parsing/generating:

var fs = require('fs');
var esprima = require('esprima');
var escodegen = require('escodegen');

var code = fs.readFileSync('code.js', 'utf8')
var tree = esprima.parse(code, {range: true, tokens: true, comment: true});
tree = escodegen.attachComments(tree, tree.comments, tree.tokens);
var output = escodegen.generate(tree);

This yields the output: 'var foo;\nfunction Bar(name) {\n this.name = name;\n}'

Also, without the var foo; before the first comment, the comment is attached as a leadingComment of the Program instead of the FunctionDeclaration.

Thanks for any updates on the status of attaching comments to nodes.

tschaub commented Jun 4, 2013

Ah, should have read the tests first.

I see the parsing/generating bit should be:

var tree = esprima.parse(code, {range: true, tokens: true, comment: true});
tree = escodegen.attachComments(tree, tree.comments, tree.tokens);
var output = escodegen.generate(tree, {comment: true});

I think it is still a valid issue that without the var foo; at the top of the source, the first comment is not attached as a leadingComment of the FunctionDeclaration - I'll search through more recent issues to see if that is mentioned elsewhere.

goatslacker referenced this issue in jshint/fixmyjs Aug 6, 2013

Closed

Why does it remove comments #70

getify commented Sep 22, 2013

FYI: I've submitted a PR which I think would provide a mechanism to address this issue more broadly across all places of the code where they would be valid: Constellation#133

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment