From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 26460 invoked by alias); 29 Oct 2013 12:49:04 -0000 Mailing-List: contact zsh-workers-help@zsh.org; run by ezmlm Precedence: bulk X-No-Archive: yes List-Id: Zsh Workers List List-Post: List-Help: X-Seq: 31927 Received: (qmail 18819 invoked from network); 29 Oct 2013 12:48:49 -0000 X-Spam-Checker-Version: SpamAssassin 3.3.2 (2011-06-06) on f.primenet.com.au X-Spam-Level: X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_NONE autolearn=ham version=3.3.2 From: Greg Klanderman To: zsh-workers@zsh.org Subject: Re: PATCH: math error messages Reply-to: gak@klanderman.net Date: Tue, 29 Oct 2013 08:31:14 -0400 In-reply-to: <20131029101639.0007bc11@pwslap01u.europe.root.pri> (Peter Stephenson's message of "Tue, 29 Oct 2013 10:16:39 +0000") Message-id: <87iowgi7hp.fsf@lwm.klanderman.net> User-Agent: Gnus/5.1008 (Gnus v5.10.8) XEmacs/21.4.22 (linux) References: <877gcxe1to.fsf@ixian.com> <20131028175505.584149cc@pwslap01u.europe.root.pri> <20131029101639.0007bc11@pwslap01u.europe.root.pri> MIME-version: 1.0 Content-type: text/plain; charset=us-ascii I don't think most users would realize that "bad math expression" might be referring to an array indexing expression. Seems like ideally, if you can tell you're in an array index context, it would say "empty array index". "math expression" is good for $((...)). not sure what other contexts we could be in here. Maybe even better, have the error prefix reflect the context: "bad array index: empty string". Not sure you even need "string", seems you could just drop it in the two cases below. Greg >>>>> On October 29, 2013 Peter Stephenson wrote: > On Mon, 28 Oct 2013 17:55:05 +0000 > Peter Stephenson wrote: >> > zsh: bad math expression: operand expected at `' >> >> The error you got comes from it being empty, so there's no >> number, which is what it's complaining about, slightly cryptically. > Is anyone particularly attached to the current error messages? I think > the following are a bit clearer for the two specific cases, although I'm > not sure about the word "string". I used it in order to keep the > standard math error prefix and avoid repeating "expression". I could > say "argument" but that means a lot of possible things, most commonly a > command line argument which would be inaccurate. > % print $foo[] > zsh: bad math expression: empty string > % print $foo[3+] > zsh: bad math expression: operand expected at end of string > I have not changed any behaviour, only messages. I've noted that while > $(()) returns 0, $foo[$unset] is an error. I'm happy with the latter > and I think the former was forced on us by standards. > diff --git a/Src/math.c b/Src/math.c > index eae283d..b21a3ad 100644 > --- a/Src/math.c > +++ b/Src/math.c > @@ -1374,6 +1374,19 @@ mathevalarg(char *s, char **ss) > mnumber x; > int xmtok = mtok; > + /* > + * At this entry point we don't allow an empty expression, > + * whereas we do with matheval(). I'm not sure if this > + * difference is deliberate, but it does mean that e.g. > + * $array[$ind] where ind hasn't been set produces an error, > + * which is probably safe. > + * > + * To avoid a more opaque error further in, bail out here. > + */ > + if (!*s) { > + zerr("bad math expression: empty string"); > + return (zlong)0; > + } > x = mathevall(s, MPREC_ARG, ss); > if (mtok == COMMA) > (*ss)--; > @@ -1401,6 +1414,7 @@ checkunary(int mtokc, char *mptr) > } > if (errmsg) { > int len, over = 0; > + char *errtype = errmsg == 2 ? "operator" : "operand"; > while (inblank(*mptr)) > mptr++; > len = ztrlen(mptr); > @@ -1408,9 +1422,12 @@ checkunary(int mtokc, char *mptr) > len = 10; > over = 1; > } > - zerr("bad math expression: %s expected at `%l%s'", > - errmsg == 2 ? "operator" : "operand", > - mptr, len, over ? "..." : ""); > + if (!*mptr) > + zerr("bad math expression: %s expected at end of string", > + errtype); > + else > + zerr("bad math expression: %s expected at `%l%s'", > + errtype, mptr, len, over ? "..." : ""); > } > unary = !(tp & OP_OPF); > } > diff --git a/Test/C01arith.ztst b/Test/C01arith.ztst > index 71c8a19..c19135c 100644 > --- a/Test/C01arith.ztst > +++ b/Test/C01arith.ztst > @@ -134,11 +134,11 @@ > print $(( a = )) > 1:empty assignment > -?(eval):1: bad math expression: operand expected at `' > +?(eval):1: bad math expression: operand expected at end of string > print $(( 3, )) > 1:empty right hand of comma > -?(eval):1: bad math expression: operand expected at `' > +?(eval):1: bad math expression: operand expected at end of string > print $(( 3,,4 )) > 1:empty middle of comma > pws