Date: Thu, 24 Jun 2010 10:34:09 +0100 From: Gavin Atkinson <gavin@FreeBSD.org> To: Benjamin Fiedler <bfiedler@FreeBSD.org> Cc: Perforce Change Reviews <perforce@FreeBSD.org> Subject: Re: PERFORCE change 180136 for review Message-ID: <1277372050.14603.21.camel@buffy.york.ac.uk> In-Reply-To: <201006230056.o5N0urs3015191@repoman.freebsd.org> References: <201006230056.o5N0urs3015191@repoman.freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?1277372050.14603.21.camel>