neugram / ng

scripting language integrated with Go
https://neugram.io
BSD 2-Clause "Simplified" License
916 stars 43 forks source link

parser: swap own shell parser with mvdan.cc/sh #223

Closed mvdan closed 2 years ago

mvdan commented 6 years ago

For now, simply use the parser and translate the AST into what the rest of neugram expects. This is the smallest change that can be made without breaking neugram.

Once the interpreter in mvdan.cc/sh/interp also starts being used, we can then get rid of the code translating ASTs and neugram's own shell AST code.

We can keep the semantics of ShellList and parseShellList thanks to sh/syntax.Parser.Stmts, which lets us control how to parse a program statement by statement.

The only complication comes from handing the underlying bytes from one lexer to another. The neugram lexer has to be careful to not tokenize any shell bytes, and vice versa.

mvdan commented 6 years ago

This is for https://github.com/neugram/ng/issues/9.

There are a couple of TODO(mvdan) comments that I added for slightly tricky things. Other than that, it works fine.

If shell code is used that neugram doesn't like (such as an if clause), this will for now panic. That can be replaced with a user-friendly error, of course.

sbinet commented 6 years ago

FYI, travis is failing because there is a non-gofmt'd file :)

mvdan commented 6 years ago

Yep - it's my bad for posting a PR right before an international flight, without checking CI before leaving :)

mvdan commented 6 years ago

Ah, forgot about a test in another package. Will have a look tomorrow.

codecov-io commented 6 years ago

Codecov Report

Merging #223 into master will decrease coverage by 0.33%. The diff coverage is 91.22%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #223      +/-   ##
==========================================
- Coverage   49.99%   49.65%   -0.34%     
==========================================
  Files          22       22              
  Lines       10406    10289     -117     
==========================================
- Hits         5202     5109      -93     
+ Misses       4702     4686      -16     
+ Partials      502      494       -8
Impacted Files Coverage Δ
parser/scanner.go 66.84% <100%> (-4.28%) :arrow_down:
parser/parser.go 70.26% <75%> (ø) :arrow_up:
parser/shell.go 91.89% <91.74%> (+5.65%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 89480e5...9c29751. Read the comment docs.

crawshaw commented 6 years ago

Apologies for my slowness on this. I need to find enough time to work on neugram to work out what to do with the shell implementation. (I have a big piece of unfinished code on methods which distracts me each time I come back to the project.) There's a lot to be said for reducing the number of lines of code in this repository, I just need to take the time to think through adding a dependency.

mvdan commented 6 years ago

Of course, feel free to take your time. This PR doesn't bring much to the table, as most of the wins would come once the interpreter is also swapped.