knrafto / language-bash

Parse and pretty-print Bash shell scripts
BSD 3-Clause "New" or "Revised" License
35 stars 9 forks source link

[ is not treated as a builtin #11

Closed pbiggar closed 8 years ago

pbiggar commented 8 years ago

[ is not parsed into Cond, Binary, etc, as [[ is. Based on [1], [2], and the code for test and [[, I'm guessing this is an oversight rather than a design decision.

Happy to code up a fix, which would reuse the test code, but I wanted to check that it would be accepted before I did.

[1] https://www.gnu.org/software/bash/manual/html_node/Bourne-Shell-Builtins.html#Bourne-Shell-Builtins [2] https://www.gnu.org/software/bash/manual/html_node/Bash-Conditional-Expressions.html#Bash-Conditional-Expressions,

pbiggar commented 8 years ago

Also, I think test doesn't work - it doesn't appear to be called from anywhere and empirically doesn't work.

knrafto commented 8 years ago

It is a design decision, but I'm open to reevaluate it. I originally created this package for a "safe" Bash interpreter (on Github as bash-config, now dead) that didn't actually execute any commands, so I tried to separate Bash syntax and semantics and have this package be only the syntax. Some builtins (e.g. declare) require special syntax, since they can take assignments as arguments, but the rest of the builtins have the same syntax as normal commands. In fact, in other shells test and [ are not builtins, but actual executables that are forked. The [[ on the other hand is special Bash syntax and should not treated like a command.

That said, I eventually did need to parse conditional expressions for test and [, so I added parseTestExpr. The idea is after substitutions and all that jazz, you pass the arguments of the test command to this function to get the AST of the conditional. For [, you have to check for and strip off the trailing ] first. You can see how this is done in bash-config here.

You might want to do things differently though. What's your use case here?

Also, what test function are you referring to in your last comment? Is it a Haskell function?

pbiggar commented 8 years ago

My use case is that I'm making a language (called Rash - the Rebourne again shell) intended to be a more sane and safe language than bash for the same niche (5-500 line programs comprised largely of chaining unix commands together). As such, I'm using language-bash in a program to translate Bash programs into Rash.

For my use case, I'll definitely be treating [[ and test and [ all the same. I see now that I can use parseTestExpr to do that - awesome! (that's what I meant by test not working - it isn't parsed automatically by parseTestExpr, and now I see why).

Judging from what I linked to before, I would certainly argue that it would be correct to parse [ directly in a bash parser. It is a builtin in Bash, as you mention, so there isn't any shelling out to use it, and it's a major part of the "language". Treating it like test and including it in the AST by default would seem "correct" to me.

That said, I can just call parseTestExpr directly if that's what you think is right for the library. I suspect that my use case will make me bend the semantics in a number of different ways, so even if we agree here, I'll no doubt have other places which need hacks. So happy to go either way (with a definite preference of parsing [ by default).

knrafto commented 8 years ago

One big issue is that for [ you have to perform globbing, command substitution, etc. before being able to parse the expression. [[ on the other hand is purely syntactic. Here's a pathological example:

if [ $(echo 1 = 1) ]; then echo 'true'; else echo 'false'; fi # prints true
if [ $(echo 1 = 2) ]; then echo 'true'; else echo 'false'; fi # prints false

so it's not even possible to know that the [ expression is when parsing the script.

Here's a StackOverflow post illustrating some other differences: http://stackoverflow.com/questions/3427872/whats-the-difference-between-and-in-bash

knrafto commented 8 years ago

I think for your case you could use Language.Bash.Word.unquote to turn the arguments of [ into strings and then call parseTestExpr on that. It wouldn't be exactly Bash-compatible, but I think it's a sane way to go.

pbiggar commented 8 years ago

OK, thanks. In that case, I definitely agree you don't want this in parser, in order to get the semantics right. This solution should work great for me. Thanks again!