Re: Using uncrustify for indent.

["Joseph S. Myers" <joseph at codesourcery dot com>]

On Fri, 31 May 2013, Ondrej Bilka wrote:

> > * If the arguments to a function call go only another line, then 
> > subsequent lines need their indentation adjusted.
> >
> This uncrustify does relatively well but it is choice between formatting
> entire file and nothing.

In general when fixing formatting issues I think we should just fix things 
that are genuine problems according to our style, rather than doing 
further reindentation that just changes one correct form to another.  (I 
don't know exactly what uncrustify does here.)

> > * The patch changes intl/plural.c, a generated file; you need to get the 
> > Bison template fixed in upstream Bison instead.
> 
> We could add comment that to disable is which is exactly /*INDENT-OFF*/.

We can't add any comments to plural.c; it's a generated file.  We can 
modify plural.y, but formatting issues present there *should* be fixed 
(and fixes carried through to plural.c by regenerating that file).  
(Though any issues in code imported from gettext, such as plural.y, or GNU 
MP, or GNU libidn, should really be fixed in those packages rather than 
just locally in glibc.)  Instead, I advise blacklisting generated files 
much like scripts/update-copyrights knows what files not to modify 
copyright dates in, or not to remove gaps in copyright year lists in.

> > It should be straightforward to make the code generating these changes 
> > exclude cases where the line would go to 80 columns or more or where 
> > parentheses are unbalanced at the end of the line, as well as excluding 
> > GLRO, GL and ElfW (at least).  (The cases where wrapping is involved could 
> > then be fixed separately with a patch that involved some manual editing of 
> > the results of initial automatic running of a script.)  I'd still suggest 
> > splitting up the resulting changes by directory for easier review.
> > 
> Patch above has 11116 lines. You need to test it automaticaly or you
> spend 3 hours at rate line checked per second.

Such a patch should be split up into smaller pieces for review - and one 
line of diff context rather than 3 should generally suffice for review.  
You need both testing and review, as for all patches.

(Smaller pieces could be arranged by area of the source tree, or by what 
function / macro is being called, or as I suggested separating those cases 
where only one line needs changing from those cases where wrapping is 
needed or reindentation of subsequent lines to avoid introducing one 
formatting problem while fixing another.)

-- 
Joseph S. Myers
joseph@codesourcery.com