search

prefix-dev / shell

The ultimate cross-platform, bash-like shell
MIT License
34 stars 4 forks source link

Implement `if / then / else / elif` support #3

Open wolfv opened 1 month ago

wolfv commented 1 month ago

deno task shell currently has no support for these constructs.

We should add a IfElse struct that looks something like: 

struct IfElse {
   condition: Expr,
   then: Expr,
   else: Optional<Expr>
}

And extend the parser to be able to parse if <expr>; then <expr>; else <expr> fi I am actually not sure if bash allows to write this on a single line.

prsabahrami commented 1 week ago

Just to confirm - we are planning to support both double brackets if [[ ... ]] and single brackets if [ ... ], correct? I believe double brackets are not POSIX compliant but are supported in bash.

baszalmstra commented 1 week ago

I would argue that we only support one option to not have multiple ways to do a single thing. So in that case single?

WDYT?

prsabahrami commented 1 week ago

IMO if shell might be used upon scripts written for Bash, we should support both. Also, considering that single brackets are much limited compared to double brackets in terms of comparison operators, I would say people mostly use double brackets for scripting and are more used to. Hence, if one of which is supported, I think it should be double?

baszalmstra commented 1 week ago

Under that assumption I agree with you. I was not aware this was a design goal.

wolfv commented 1 week ago

I think it's good to start with double brackets indeed!

prsabahrami commented 1 week ago

I think it's good to start with double brackets indeed!

Agreed!

Hofer-Julian commented 1 week ago

I believe double brackets are not POSIX compliant but are supported in bash.

If you are interested to making the syntax bash compatible, that would be even better :)

certik commented 1 week ago

The options are:

  1. Only [ ]
  2. Only [[ ]]
  3. Both [ ] and [[ ]]

Whichever option we go, it should be supported directly in the compiler and parser, so that we get good error messages. In Bash-style shells traditionally, the [ was an alias to a command called test, and the test command would just implement the "if" statement logic. The [[ was later added as a built-in syntax/semantics. I am proposing that both [ and [[ (if we support them) should be handled intrinsically.

It would be nice to just support one way, because this is a never-ending confusion which one should a new user use, the advice usually is to use [[.

So my further recommendation would be to understand both, but pick one to support and then give a nice error message to the user to user if the other one is used.

Given the historical recommendation, and better support in Bash for [[, I would start by using just [[, and give a beautiful error message for [. This leaves the door open to later relax an error message to just a warning and support [ as well without breaking backwards compatibility, or even not give a warning at all and support it directly. If however we support it right now, or just give a warning, it will be hard to later give an error message, it would break people's scripts. With the error message we can wait for the feedback of users and see if we need to relax it.

Summary: support [[ as the if statement, and give a nice error message when [ is used and recommend an equivalent syntax using [[.

certik commented 1 day ago

Initial support was implemented in https://github.com/prefix-dev/shell/pull/121.