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

[Rob Landley]

On 03/16/2016 06:44 PM, Andy Chu wrote:
> 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

First pass was pushed to https://github.com/landley/toybox around
lunchtime: https://github.com/landley/toybox/commit/1af3bad8b63b

> -- 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 applied your two patches and I've done one round of cleanup on top of
them so far. Then I started replacing "TT.tok = toys.optargs++" with
directly incrementing TT.tok and I introduced a bug with parentheses
management and got myself confused and went to do something else for a
while and haven't gotten back to it yet.

> 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)".

This morning's cleanup pass replaced the calls to syntax_error() with
calls to error_exit(), making it always print the more detailed error
message.

> 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 (?).

It's not less space for constant strings, it's an intentionally
restricted vocabulary to be less hard on non-english speakers.

I don't have a direct jump-to#tag in
http://landley.net/toybox/design.html but it's "error messages and
internationalization". (And I probably should add an index with #tags to
that page...)

> 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 )" ...

By the way, "expr ( )" printed "missing )", I have a better error
message in the current one but as I said: buggy at present.

> 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.

So my help text is wrong and not _all_ operators are infix.

Sigh...

(I did a cleanup pass earlier, and often with cleanup I start with the
help text. What SHOULD this command be doing? Documentation is _not_ an
afterthought...)

Rob

 1458174461.0