PERFORCE change 180136 for review

[Gavin Atkinson]

On Wed, 2010-06-23 at 00:56 +0000, Benjamin Fiedler wrote:
> http://p4web.freebsd.org/@@180136?ac=10

Just a couple of small style issues...

> Change 180136 by bfiedler at freebsd-7803 on 2010/06/23 00:56:51
> 
> 	Add Eflag

> 
> ==== //depot/projects/soc2010/bsdtextproc/gabor_diff/diff.c#4 (text+ko) ====
> 
> @@ -48,7 +48,7 @@
>  
>  int	 aflag, bflag, Bflag, dflag, iflag, lflag, Nflag, Pflag, pflag, rflag;
>  int	 sflag, tflag, Tflag, wflag, uniflag, yflag, strip_cr, tabsize=8;
> -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 = 8;


> -/* XXX: UNIMPLEMENTED
> -	{ "ignore-tab-expansion",	no_argument,		NULL,	'E' }, */
> +/* XXX: UNIMPLEMENTED */
> +	{ "ignore-tab-expansion",	no_argument,		NULL,	'E' }, 

I'm guessing this comment is now wrong?



> ==== //depot/projects/soc2010/bsdtextproc/gabor_diff/diff.h#4 (text+ko) ====
> 
> @@ -83,7 +83,7 @@
>  };
>  
>  extern int	 aflag, bflag, Bflag, dflag, iflag, lflag, Nflag, Pflag, pflag, 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.

> ==== //depot/projects/soc2010/bsdtextproc/gabor_diff/diffreg.c#4 (text+ko) ====
> 

> +			newcol = ((b/8)+1)*8;
> +			while ((Eflag) && (c == L'\t') && (d == L' ') && b <= newcol )
> +                                        d = strd[++b];
> +
> +			newcol = ((a/8)+1)*8;
> +                        while ((Eflag) && (d == L'\t') && (c == L' ') && a <= newcol )
> +                                        c = 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