From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 4593 invoked from network); 26 Jun 2009 21:58:14 -0000 X-Spam-Checker-Version: SpamAssassin 3.2.5 (2008-06-10) on f.primenet.com.au X-Spam-Level: X-Spam-Status: No, score=-2.6 required=5.0 tests=BAYES_00 autolearn=ham version=3.2.5 Received: from new-brage.dotsrc.org (HELO a.mx.sunsite.dk) (130.225.254.104) by ns1.primenet.com.au with SMTP; 26 Jun 2009 21:58:14 -0000 Received-SPF: none (ns1.primenet.com.au: domain at sunsite.dk does not designate permitted sender hosts) Received: (qmail 61567 invoked from network); 26 Jun 2009 21:58:04 -0000 Received: from sunsite.dk (130.225.247.90) by a.mx.sunsite.dk with SMTP; 26 Jun 2009 21:58:04 -0000 Received: (qmail 12760 invoked by alias); 26 Jun 2009 21:57:57 -0000 Mailing-List: contact zsh-workers-help@sunsite.dk; run by ezmlm Precedence: bulk X-No-Archive: yes X-Seq: 27059 Received: (qmail 12746 invoked from network); 26 Jun 2009 21:57:56 -0000 Received: from bifrost.dotsrc.org (130.225.254.106) by sunsite.dk with SMTP; 26 Jun 2009 21:57:56 -0000 Received: from QMTA06.westchester.pa.mail.comcast.net (qmta06.westchester.pa.mail.comcast.net [76.96.62.56]) by bifrost.dotsrc.org (Postfix) with ESMTP id 5036480307FA for ; Fri, 26 Jun 2009 23:57:39 +0200 (CEST) Received: from OMTA21.westchester.pa.mail.comcast.net ([76.96.62.72]) by QMTA06.westchester.pa.mail.comcast.net with comcast id 8gxy1c0081ZXKqc56lxgEf; Fri, 26 Jun 2009 21:57:40 +0000 Received: from smtp.klanderman.net ([98.217.254.247]) by OMTA21.westchester.pa.mail.comcast.net with comcast id 8lyW1c0035M2Np63hlyWa0; Fri, 26 Jun 2009 21:58:30 +0000 Received: from lwm.klanderman.net (unknown [192.168.100.50]) by smtp.klanderman.net (Postfix) with ESMTP id D3927B3014A for ; Fri, 26 Jun 2009 17:57:38 -0400 (EDT) Received: by lwm.klanderman.net (Postfix, from userid 500) id ACD4F9FC61E; Fri, 26 Jun 2009 17:57:38 -0400 (EDT) From: Greg Klanderman To: zsh-workers@sunsite.dk Subject: Re: bug in ztrftime(): '%e' and '%f' specifiers swapped Reply-To: gak@klanderman.net Date: Fri, 26 Jun 2009 17:57:38 -0400 In-Reply-To: <200906262123.n5QLNqJU008820@pws-pc.ntlworld.com> (Peter Stephenson's message of "Fri, 26 Jun 2009 22:23:52 +0100") Message-ID: User-Agent: Gnus/5.1007 (Gnus v5.10.7) XEmacs/21.4.17 (linux) References: <200906262123.n5QLNqJU008820@pws-pc.ntlworld.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Virus-Scanned: ClamAV 0.94.2/9510/Fri Jun 26 17:46:23 2009 on bifrost X-Virus-Status: Clean > Interesting how subjective this is, I find it significantly less so Yep! Not to try to sway you, but I'll give you my reasons just for the fun of it.. I guess the biggest is that I really don't like cases that do something then fall through, it's just very hard to reason about, and usually makes further modifications difficult and error prone. Also, this style very rarely is able to extend beyond two cases - case in point when you added the 'H' format you had to violate your first three points below. > - there is no immediate visual cue that the formats do different things I would always assume if there are multiple cases sharing the same logic that the logic itself will differentiate between them, not that they all function identically. > - the test for the format letter is repeated without it being obvious it > is the same test > - it's made even less obvious because the second test has to advance > back down the format string since the key letter isn't saved in a > variable [we do this sort of thing all the time elsewhere, though] I certainly would have saved the switch'd on character in a variable if I were writing this, and probably if I'd had a working build system I might have done more significant cleanup. On the other hand, when submitting patches to a project like this I usually go for the minimal change because otherwise it's a larger burden on whoever reviews and commits the change. Seeing that pattern (fmt[-1] == 'x') already in use in that function I figured it was considered not so bad. > - the fact that 'f' sets the "strip" feature unconditionally is obscured strip = strip || (fmtchar == 'f') is pretty clear to me :-) > - I'm not that keen on "||" and "&&" in assignments anyway [again, the > shell is full of stuff I'm not that keen on] > so I've committed the first one. thank you! greg