Bug GCLI: ash math operators outside of parenthesis return an unclear error

katyarn

New member
In the Graphical CLI:
Code:
> ash (100 - elemental_resistance($element[spooky])) / 100.0
Expected ;, found / ()
Returned: void

Code:
> ash ((100 - elemental_resistance($element[spooky])) / 100.0)
Returned: 0.46666666666666673

Code:
> ash 1+1
Expected ;, found + ()
Returned: void

Code:
> ash (1+1)
Returned: 2

Expected behavior is to always return the result of calculation, or at least explain why ash is looking for a semicolon instead of the plus/minus/multiplication/division operator. Otherwise I think it could be very confusing.
 

zarqon

Active member
I believe you'll find that behavior is unique to the "ash" command. In a script, no parentheses works as expected.
 

heeheehee

Developer
Staff member
I believe you'll find that behavior is unique to the "ash" command. In a script, no parentheses works as expected.
Not quite.

Code:
1+1;
is not a valid ASH script.
Expected ;, found + (test.ash, line 1)

This is a quirk of our current parser logic, wherein we distinguish between valid commands (which can be used in this standalone fashion) and expressions. On one hand, it makes sense to change in the interest of consistency with other languages (and also if people want to use the CLI command as a calculator...). On the other, within a script, it isn't very useful because there are no side effects associated with any of ASH's binary non-assignment operators.
 

zarqon

Active member
Aha, thanks for that. I meant to test my claim above but by the time I uninstalled old Java/installed new Java I had to get back to work and then promptly forgot to follow up.
 

heeheehee

Developer
Staff member
Expressions can stand alone in JS (regardless of whether your engine is Rhino or V8 or SpiderMonkey or whatever), yeah.
 

Veracity

Developer
Staff member
This is a quirk of our current parser logic, wherein we distinguish between valid commands (which can be used in this standalone fashion) and expressions. On one hand, it makes sense to change in the interest of consistency with other languages (and also if people want to use the CLI command as a calculator...). On the other, within a script, it isn't very useful because there are no side effects associated with any of ASH's binary non-assignment operators.
Considering that "1+1" is an expression and "(1+1)" is an expression, and the latter can be used as a standalone command - and presumably as the last statement in a function without using a "return" - I see no reason to not allow it.

Looking at Parser, I see ParseCommand has "} else if ((result = this.parseEvaluable(scope)) != null) {" as one of the allowed "commands". That used to be ParseValue. I wonder why not ParseExpression? In any case, it is ParseEvaluable which calls ParseExpression iff it sees "(".

I suspect a close look at ParseCommand, ParseEvaluable, and ParseExpression will be necessary.
And ideally a whole slew of unit tests. :)
 

heeheehee

Developer
Staff member
Yeah. I don't think it'll be a super complicated fix.

Previously, parseEvaluable was parseValue, as you noted. We recently (as in earlier today) merged in a change to remove Value from the parse tree, since it had been overloaded to refer to a concrete value as well as a whole category of parse tree elements. Said change extracted the parse tree functionality into a new abstract Evaluable class in its place (something that has a meaningful value that it can be evaluated to).

I would like to merge Evaluable and Expression (as part of a broader reworking of Parser), but introducing Evaluable was the simplest fix at the time.
 
Top