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

direction? #2

Open
gabrielcsapo opened this issue Jul 7, 2017 · 22 comments
Open

direction? #2

gabrielcsapo opened this issue Jul 7, 2017 · 22 comments

Comments

@gabrielcsapo
Copy link
Contributor

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
Copy link
Collaborator

nfischer commented Jul 7, 2017

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
Copy link
Contributor Author

gabrielcsapo commented Jul 7, 2017

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
Copy link
Contributor

parro-it commented Jul 7, 2017

@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
Copy link
Contributor Author

@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
Copy link
Contributor

parro-it commented Jul 7, 2017

@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
Copy link
Contributor Author

@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
Copy link
Contributor

parro-it commented Jul 7, 2017

@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
Copy link
Contributor Author

gabrielcsapo commented Jul 8, 2017

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

@gabrielcsapo
Copy link
Contributor Author

@parro-it #3 😄

@gabrielcsapo
Copy link
Contributor Author

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

@nfischer
Copy link
Collaborator

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

@gabrielcsapo
Copy link
Contributor Author

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

@parro-it
Copy link
Contributor

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

@gabrielcsapo I would complete source code generation for all node types.
If you can take care of that, I'll try meanwhile to add source code info for all token. This change in bash-parser will allow to preserve source code formatting, comments, etc...

So, we have a source code parser, an AST traverser & transformer and finally sa source code generator. These toolkit start to resemble the babel toolchain;

What do you think of a compelte code transpiler for bash? I would love, for example, to have function with named arguments. That feature could be transpiled, like we do with js

@gabrielcsapo
Copy link
Contributor Author

I will try to allocate time to finish up the generation piece. The preservation of location will be very helpful.

Having a Babel like parser for bash will be incredibly helpful! I like the idea!

@nfischer
Copy link
Collaborator

What do you think of a compelte code transpiler for bash? I would love, for example, to have function with named arguments. That feature could be transpiled, like we do with js

@parro-it are you suggesting making a new bash-like language with extra features, and transpiling down to bash in order to run it?

@parro-it
Copy link
Contributor

Yes, I'm suggesting that. Does it sound crazy?

@gabrielcsapo
Copy link
Contributor Author

that would be awesome 😮

@nfischer
Copy link
Collaborator

Yes, I'm suggesting that. Does it sound crazy?

Could be a fun project. What new features do you have in mind?

@gabrielcsapo
Copy link
Contributor Author

Okay finally back working on this, I saw you made changes to bash-ast-parser @parro-it that allows us to get functions?! woot woot! Do we have an test data that we can deconstruct and reconstruct so I can write some tests?

@piranna
Copy link
Collaborator

piranna commented Sep 21, 2017

So, we have a source code parser, an AST traverser & transformer and finally sa source code generator. These toolkit start to resemble the babel toolchain;

What would be the best path for a bash2js transpiler? Run over the bash-ast and write Javscript code from it, or convert the bash-ast to some kind of Javascript-ast and use an already available Javascript code generator? What ones currently there are that accept a JSON object and are easy to use (and generate performant code, too :-D )?

@milahu
Copy link

milahu commented Dec 2, 2022

bash2js transpiler

hello from the future : )

https://github.com/nfischer/shelljs-transpiler

@dthree
Copy link
Collaborator

dthree commented Dec 20, 2022

Hello future!

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

No branches or pull requests

6 participants