[Insight-developers] Re: KWStyle run on Code/IO and Code/Numerics

Julien Jomier julien.jomier at kitware.com
Mon Mar 26 10:39:25 EST 2007


Hi Bill,

Thanks a lot for taking the time to report these errors. I really 
appreciate it.

> 1) KWStyle wants enums in a .h file to have m_ prefixes. This is 
> incorrect. Lots of examples of this in Code/IO.
>  

This is fixed now.

> 2) In switch statements, if "default": is not the last switch state, 
> KWStyle gets confused about indentation. Examples: lines 476, 1048 and 
> 1113 in itkNrrdImageIO.cxx and lines 191, 980 and 1095 in 
> itkMetaImageIO.cxx .
>  

This is fixed now.

> 3) itkGDCMImageIO.cxx has some #ifdef's that mess up KWStyle's 
> indentation. Run KWStyle on this file and you'll see the challenges.
>

This is a difficult one. I'll try to look at it later this week.

> 4) itkIPLFileNameLists.h has some classs definitions inside a class. 
> This seems to confuse KWStyle. Run KWStyle on this file.

This is fixed now. The main issue is when the comments are not defined 
correctly then KWStyle is lost. I have put a warning when the comments 
are not defined correctly and now KWStyle behaves correctly

>  
> 5) itkMetaArrayReader.h line 127 and 211 indentations suggested by 
> KWStyle are not correct.
>  

This is fixed. This was due to the fact that KWStyle tried to minimize 
the amount of code on a line.

Let's say you have:
   test(foo,
        bar);

then KWStyle will detect that test(foo,bar); is fitting onto one line 
and will report an indentation error. I've put an optional argument 
(true by default) to not report this kind of error (only for commas).

> 6) for(;;) KWStyle reports a repeated ";". Numerics/itkLBFGSBOptimizer.cxx
>  

Fixed in cvs. Thanks for the report!

> There are a few other scattered examples. You should run KWStyle on all 
> the files in IO and Numerics (only top level).
>  

I will try to look at them soon.

>  
> It would be neat if KWStyle could optionally correct the style errors 
> and write a corrected file to stdout. I spent a lot of time removing 
> extra white space and replacing " ;" with ";".
>  

This is on my todo list.

> Finally, KWStyle is riot quite ready to be used as a commit check.

I agree with you on this.

Thanks again for the report,

Julien



More information about the Insight-developers mailing list