oils-for-unix / oils

Oils is our upgrade path from bash to a better language and runtime. It's also for Python and JavaScript users who avoid shell!
http://www.oilshell.org/
Other
2.85k stars 159 forks source link

[ysh] Lex '&&', '||' for better errors in expr mode #2101

Closed PossiblyAShrub closed 1 month ago

PossiblyAShrub commented 1 month ago

Lex '&&', '||' in expression mode for better error messages:

  = a && b
      ^~
[ -c flag ]:1: Use 'and' in expression mode

  = a || b
      ^~
[ -c flag ]:1: Use 'or' in expression mode

Eggex has Expr_Bang which means we cannot check this case the same way:

  = !a
    ^
[ -c flag ]:1: Syntax error in expression (near Id.Expr_Bang)

(I don't think it's possible to cleanly check this specific error unless we moved away from pgen2.)

andychu commented 1 month ago

OK looks great!

And yeah too bad about ! ... we can probably check it in arithmetic, but it would have to use a different strategy, not this "over-lexing" one. It would have to be aware of parsing

How about adding a OILS-ERR- code in doc/error-catalog.md ? Because not everyone will know what "expression mode" is

I think they could use a format like:

No:
   if (a || b) {
     echo yes
   }

Yes:
  if (a or b) {
    echo yes
  }

and then say that Python-like expressions generally occur within ()

Actually it can link to doc/command-vs-expression-mode.html


It can just be 1 code for both errors

andychu commented 1 month ago

I guess we can also have something like:

No:
  if test --dir d1 and test --dir d2 { ... }
  if (test --dir d1 and test --dir d2) { ... }

Yes:
  if test --dir d2 && test --dir d2 { ... }
PossiblyAShrub commented 1 month ago

This is ready for another review.

andychu commented 1 month ago

Looks good! one of the jobs failed with a Docker error, so I kicked it