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

Allow preserving parenthesis. #113

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Conversation

ravener
Copy link

@ravener ravener commented Dec 12, 2021

This is very useful for when converting the tree back to source code to know where parenthesis have been used by the user.

This is inspired by acornjs and is exactly how they did it.

Example: print((5 + 2) * 3)

{
  "type": "Chunk",
  "body": [
    {
      "type": "CallStatement",
      "expression": {
        "type": "CallExpression",
        "base": {
          "type": "Identifier",
          "name": "print"
        },
        "arguments": [
          {
            "type": "BinaryExpression",
            "operator": "*",
            "left": {
              "type": "ParenthesizedExpression",
              "expression": {
                "type": "BinaryExpression",
                "operator": "+",
                "left": {
                  "type": "NumericLiteral",
                  "value": 5,
                  "raw": "5"
                },
                "right": {
                  "type": "NumericLiteral",
                  "value": 2,
                  "raw": "2"
                }
              }
            },
            "right": {
              "type": "NumericLiteral",
              "value": 3,
              "raw": "3"
            }
          }
        ]
      }
    }
  ],
  "comments": []
}

README.md Outdated Show resolved Hide resolved
@fstirlitz
Copy link
Owner

First of all, the test suite is failing. I will not be merging anything without that fixed.

Second, this may conflict with #98 when it is finally merged.

And finally, I am not necessarily sold on preserving parentheses where they are not semantically meaningful. This is an abstract syntax tree after all.

@ravener
Copy link
Author

ravener commented Dec 13, 2021

@fstirlitz

  1. not sure why tests failed, it wasn't really a major change, i'll take a look.
  2. not sure what to say about this, i think this change is still compatible with it.
  3. well as I said it helps when transforming the tree back to source code, it's hidden behind an option anyway.

Edit: the tests was due to a typo, just fixed it.

@ravener
Copy link
Author

ravener commented Dec 13, 2021

Now CI is failing due to coverage, I have no idea what to do from here on but I hope you will consider the idea, it might be really useful for some.

If not then there must atleast be some way to achieve this from the user side.

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