[Toybox] [PATCH] Fix the operator precedence in expr.

[Andy Chu]

On Wed, Mar 16, 2016 at 3:00 PM, Rob Landley <rob at landley.net> wrote:
> I've already started cleanup on this command, but I'll integrate this
> with what I've got.
>
> Sounds like fun. I hope to have this done and promoted for the next
> release. (Ballpark end of April.)

OK great!  I'd like to see the diff for future reference -- I've
looked through your cleanup page.  Perhaps you can apply at least the
first 2 patches verbatim (as long as there are no egregious errors),
and then your cleanup commits, so I can just look at the history?

I wanted to bring up the one TODO in the patches... syntax_error()
defaults to printing "syntax error", but the caller passes a more
detailed error message, which can be enabled with "if (1)".

If it were my choice I would always use the verbose error message and
get rid of the if.  I added the option for the cryptic message because
I thought that was the style of the project -- a smaller vocabulary
and less space for constant strings (?).

But if I have to debug this code again, I much prefer each error
message to have a unique message, so you don't have to go into a
debugger to see what happened (and also for general usability).  In
fact I should have probably tested each of the syntax_error() calls
like this:

testing "error: missing )" "expr \( 5 + 5 2>&1" "expr: Expected )" ...

To illustrate the advantages, I think you wouldn't have been mystified
in your blog post about this behavior if GNU expr had better error
messages:

$ expr + 1
1

$ expr - 1
expr: syntax error

$ expr +
expr: syntax error

$ expr -
-

I would have written this as:

$ expr - 1
expr: expected atom, got operator -

$ expr +
expr: escape requires operand

That would make it clearer that + is both a unary and binary operator,
while - is only a binary operator.

thanks,
Andy







Andy

 1458171865.0