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

More user-controllable verbatim property #173

Closed
azu opened this issue Mar 9, 2014 · 5 comments
Closed

More user-controllable verbatim property #173

azu opened this issue Mar 9, 2014 · 5 comments

Comments

@azu
Copy link
Member

azu commented Mar 9, 2014

Hi,

escodegen 1.2.0 generate unnecessary ( ) around verbatim property value.

var code = "fn([1,2,3])";
var ast = esprima.parse(code, {range: true});
// Actually, edit AST ...
function embedVerbatim(node) {
    // embed code
    node["x-verbatim-property"] = code.substring(node.range[0], node.range[1])
}
estraverse.traverse(ast, {
    enter: function enter(node, parent) {
        var fn = {
            "ArrayExpression": embedVerbatim
        }[node.type];
        if (fn) {
            fn(node, parent);
        }
    }
});

escodegen.generate(ast, {
    verbatim: "x-verbatim-property"
}); // => "fn(([1,2,3]));"

I expected result "fn([1,2,3])"

More detail in that project.

I want to control the parenthesize.

Thanks.

@Constellation
Copy link
Member

What do you think of exposing Precendence?
For example,

node['x-verbatim-property'] = {
    content: 'string',
    precedence: escodegen.Precedence.XXX
};

Pros

  • Provides fine grained control over parentheses.

Cons

  • Depends on unstable Precedence. Precedence contents may be changed at some time.

@azu
Copy link
Member Author

azu commented Mar 9, 2014

@Constellation I think it is a good idea to set a option object.
(We can make each object difference Precedence. )

I'm worried about Cons, too.
However, It seems the impact is small.

@Constellation
Copy link
Member

Agreed. So I'll expose Precedence.

@Constellation
Copy link
Member

I've published version 1.3.0 with this feature.

@azu
Copy link
Member Author

azu commented Mar 9, 2014

@Constellation Thnaks! great job!

Test passed on example project 💚

azu/escodegen_array@66ccc61

--- a/index.js
+++ b/index.js
@@ -23,7 +23,10 @@ function main(code) {
     function embedVerbatim(node) {
         var embed = helper.getCodeFromRange(node.range);
         if (embed) {
-            node["x-verbatim-property"] = embed
+            node["x-verbatim-property"] = {
+                content : embed,
+                precedence : escodegen.Precedence.Primary
+            }
         }
     }

azu added a commit to azu/remove-use-strict that referenced this issue Mar 9, 2014
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

2 participants