From owner-p4-projects@FreeBSD.ORG Thu Jun 24 10:06:15 2010 Return-Path: Delivered-To: p4-projects@freebsd.org Received: by hub.freebsd.org (Postfix, from userid 32767) id 35717106566C; Thu, 24 Jun 2010 10:06:14 +0000 (UTC) Delivered-To: perforce@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 85246106566B; Thu, 24 Jun 2010 10:06:14 +0000 (UTC) (envelope-from gavin@FreeBSD.org) Received: from mail-gw1.york.ac.uk (mail-gw1.york.ac.uk [144.32.128.246]) by mx1.freebsd.org (Postfix) with ESMTP id 1DAA38FC1C; Thu, 24 Jun 2010 10:06:13 +0000 (UTC) Received: from mail-gw7.york.ac.uk (mail-gw7.york.ac.uk [144.32.129.30]) by mail-gw1.york.ac.uk (8.13.6/8.13.6) with ESMTP id o5O9YBuk001850; Thu, 24 Jun 2010 10:34:11 +0100 (BST) Received: from buffy-128.york.ac.uk ([144.32.128.160] helo=buffy.york.ac.uk) by mail-gw7.york.ac.uk with esmtps (TLSv1:AES256-SHA:256) (Exim 4.68) (envelope-from ) id 1ORip1-0001Vn-2k; Thu, 24 Jun 2010 10:34:11 +0100 Received: from buffy.york.ac.uk (localhost [127.0.0.1]) by buffy.york.ac.uk (8.14.3/8.14.3) with ESMTP id o5O9YAYf015113; Thu, 24 Jun 2010 10:34:10 +0100 (BST) (envelope-from gavin@FreeBSD.org) Received: (from ga9@localhost) by buffy.york.ac.uk (8.14.3/8.14.3/Submit) id o5O9YATt015112; Thu, 24 Jun 2010 10:34:10 +0100 (BST) (envelope-from gavin@FreeBSD.org) X-Authentication-Warning: buffy.york.ac.uk: ga9 set sender to gavin@FreeBSD.org using -f From: Gavin Atkinson To: Benjamin Fiedler In-Reply-To: <201006230056.o5N0urs3015191@repoman.freebsd.org> References: <201006230056.o5N0urs3015191@repoman.freebsd.org> Content-Type: text/plain; charset="ASCII" Content-Transfer-Encoding: quoted-printable Date: Thu, 24 Jun 2010 10:34:09 +0100 Message-ID: <1277372050.14603.21.camel@buffy.york.ac.uk> Mime-Version: 1.0 X-Mailer: Evolution 2.28.1 FreeBSD GNOME Team Port X-York-MailScanner: Found to be clean X-York-MailScanner-From: gavin@freebsd.org Cc: Perforce Change Reviews Subject: Re: PERFORCE change 180136 for review X-BeenThere: p4-projects@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: p4 projects tree changes List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 24 Jun 2010 10:06:15 -0000 On Wed, 2010-06-23 at 00:56 +0000, Benjamin Fiedler wrote: > http://p4web.freebsd.org/@@180136?ac=3D10 Just a couple of small style issues... > Change 180136 by bfiedler@freebsd-7803 on 2010/06/23 00:56:51 >=20 > Add Eflag >=20 > =3D=3D=3D=3D //depot/projects/soc2010/bsdtextproc/gabor_diff/diff.c#4 (te= xt+ko) =3D=3D=3D=3D >=20 > @@ -48,7 +48,7 @@ > =20 > int aflag, bflag, Bflag, dflag, iflag, lflag, Nflag, Pflag, pflag, rfla= g; > int sflag, tflag, Tflag, wflag, uniflag, yflag, strip_cr, tabsize=3D8; > -int horizon; > +int horizon, Eflag; Try to keep variable defines sorted into alphabetical order. Given the number of changes these lines are seeing at the moment, it's probably best refactoring them in one go, to make this easier. Something like: int aflag, bflag, Bflag, dflag, Eflag, iflag; int lflag, Nflag, Pflag, pflag, rflag, sflag; int tflag, Tflag, wflag, uniflag, yflag; int strip_cr, horizon; int tabsize =3D 8; > -/* XXX: UNIMPLEMENTED > - { "ignore-tab-expansion", no_argument, NULL, 'E' }, */ > +/* XXX: UNIMPLEMENTED */ > + { "ignore-tab-expansion", no_argument, NULL, 'E' },=20 I'm guessing this comment is now wrong? > =3D=3D=3D=3D //depot/projects/soc2010/bsdtextproc/gabor_diff/diff.h#4 (te= xt+ko) =3D=3D=3D=3D >=20 > @@ -83,7 +83,7 @@ > }; > =20 > extern int aflag, bflag, Bflag, dflag, iflag, lflag, Nflag, Pflag, pfla= g, rflag, > - sflag, tflag, Tflag, wflag, uniflag, strip_cr, tabsize; > + sflag, tflag, Tflag, wflag, uniflag, strip_cr, tabsize, Eflag; > extern int format, status, horizon; > extern int fcase_behave; > extern unsigned long long context; Same here, re variable ordering. > =3D=3D=3D=3D //depot/projects/soc2010/bsdtextproc/gabor_diff/diffreg.c#4 = (text+ko) =3D=3D=3D=3D >=20 > + newcol =3D ((b/8)+1)*8; > + while ((Eflag) && (c =3D=3D L'\t') && (d =3D=3D L' ') && b <=3D newco= l ) > + d =3D strd[++b]; > + > + newcol =3D ((a/8)+1)*8; > + while ((Eflag) && (d =3D=3D L'\t') && (c =3D=3D = L' ') && a <=3D newcol ) > + c =3D strc[++a]; This looks good, although there is too much indentation on three of the lines. It might be worth having another read over the style(9) man page, but most of the issues really are minor. It's the sort of issue that we'd want to fix up before the work ends up in the tree though, so it's worth getting it right first time. Overall, I'm pretty impressed with the work that you're committing at the moment :) Thanks, Gavin