Skip to content

Add basic comments support to the AST #15

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

Merged
merged 1 commit into from
Jul 15, 2017
Merged

Add basic comments support to the AST #15

merged 1 commit into from
Jul 15, 2017

Conversation

xmo-odoo
Copy link
Contributor

Cf #14

Follows the Recast comment-attachment structure[0], though only implements a small subset in a straightforwardly hacky manner, and does not come close to implementing the entire comment-attachment structure or algorithm:

  • comments preceding (some) nodes are extracted and set on the
    comments key of the host node
  • does not extract the location
  • only does leading comments (I don't think it'll mis-attach trailing
    comments but that's not impossible)
  • may not work for all node types
  • the comments key may be missing entirely
  • uses esprima's type (Block/Line) rather than Babel's

[0] dev's reasoning about some comments being ambiguous in Esprima's structure/extension makes sense to me: jquery/esprima#1024 (comment) benjamn/recast#159 (comment)

Follows the Recast comment-attachment structure[0], though only
implements a small subset in a straightforwardly hacky manner, and
does not come close to implementing the entire comment-attachment
structure or algorithm:

* comments preceding (some) nodes are extracted and set on the
  `comments` key of the host node
* does not extract the location
* only does leading comments (I don't think it'll mis-attach trailing
  comments but that's not impossible)
* may not work for all node types
* the comments key may be missing entirely
* uses esprima's type (Block/Line) rather than Babel's

[0] dev's reasoning about some comments being ambiguous in Esprima's
    structure/extension makes sense to me:
    jquery/esprima#1024 (comment)
    benjamn/recast#159 (comment)
@PiotrDabkowski
Copy link
Owner

Thank you very much! Seems to work for now. Maybe I will update the pyesprima according to the newest esprima.js version if I find some time.

@PiotrDabkowski PiotrDabkowski merged commit ff57386 into PiotrDabkowski:master Jul 15, 2017
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

Successfully merging this pull request may close these issues.

2 participants