direction?

[gabrielcsapo]

I added a basic initial commit with some working code and a test case.

What do you think about the direction?

@parro-it @dthree

[nfischer]

It looks like you're modifying the original AST, but that isn't obvious to users. I'd recommend leaving the original AST untouched and returning the fully-formed shell code as a string.

I get nasty errors when trying to call codegen(ast) twice:

Error: While traversing property commands:While traversing property name:Cannot redefine property: source
    at propertiesNames.map.err (/home/nate/fake/node_modules/bash-ast-traverser/descend-visitor.js:23:11)
    at Array.map (native)
    at /home/nate/fake/node_modules/bash-ast-traverser/descend-visitor.js:15:47
    at visit (/home/nate/fake/node_modules/bash-ast-traverser/index.js:27:17)
    at visitor.reduce (/home/nate/fake/node_modules/bash-ast-traverser/index.js:20:21)
    at Array.reduce (native)
    at visit (/home/nate/fake/node_modules/bash-ast-traverser/index.js:19:5)
    at node (/home/nate/fake/node_modules/bash-ast-traverser/index.js:40:3)
    at traverse (/home/nate/fake/node_modules/bash-ast-traverser/index.js:48:41)
    at module.exports (/home/nate/fake/node_modules/bash-codegen/index.js:4:12)

[gabrielcsapo]

I put it in the README and the test cases that ensure the user knows they are getting back and AST with an attribute to retrieve the source. I can submit a PR to do what you are asking, instead of sending back the AST or maybe have two methods? One for altering the AST, another for actually returning the values?

[parro-it]

@gabrielcsapo you can clone the original node and add the source method to the cloned node.

This could anyway have a big impact on the performance of the transform, especially for very big tree.

Do you know how babel work in this regard?

[gabrielcsapo]

@parro-it looking at https://github.com/babel/babel/tree/7.0/packages/babel-generator it seems like they are accessing an object and updating it via functions like this.token('[') as seen here https://github.com/babel/babel/blob/7.0/packages/babel-generator/src/generators/expressions.js#L211 I would be up to see what other people would rather have done, I was thinking that just returning a string would be a little too dramatic from a AST to String. I now see that might be a better idea.

[parro-it]

@gabrielcsapo yes, it seems babel mutate AST nodes.

It looks like you're modifying the original AST, but that isn't obvious to users

According to me, you can go ahead mutating input objects, we just have to make it clear what we are doing for the users, using clever names for API functions...

It could be better to check if source property is already present on the node, to avoid the exception above.

[gabrielcsapo]

@parro-it would you be able to specify where in the readme we should add this?

So I had put code to block adding the property for nodes if they had a source property already, but with the recent release to the AST traverser it had fixed the issue of not being recursive.

[parro-it]

@parro-it would you be able to specify where in the readme we should add this?

It could be in the example section:

const enableCodeGen = require('bash-codegen');
enableCodeGen(ast);

const parsed = ast.commands.map((code) => {
    return code.source + '\n';
}).join('');

[gabrielcsapo]

okay, to point out that the AST is being transformed? @parro-it I will submit a PR for that then

[gabrielcsapo]

@parro-it #3 😄

[gabrielcsapo]

@parro-it @nfischer any thoughts on functionality moving forward?

[nfischer]

Why does .source use a getter instead of being a method?

[gabrielcsapo]

@nfischer because I didn't want to call a function to get source, since it is really just a dynamic property.