[Insight-developers] uncrustify git hook

[Hans Johnson]

Matthew,

Your change the the uncrustify configuration is going to cause changes to a
large portion of the code base.  This type of change may cause huge pushback
to the use of uncrustify if the style is changing a lot.  It could be
changed to ³ignore², and that way it will not cause files to changes.

-- personally, I disagree in your contention that it increases readability
or that it makes work with vim/emacs easier, but that is just one opinion,
and many needed to be vetted before such a change is put into place.

I don¹t have real strong feelings either way about the style, but I do have
VERY strong feelings about making evolutionary changes to the uncrustify
configuration one at a time.

The current proposal is to only run uncrustify on the code base before
releases.   At that point all the vetted and approved style changes can be
applied at once.

Regards,
Hans

PS:  Thanks for starting to put together the optional git-hooks scripts for
this process.

PPS:  uncrustify still has a small set of bugs (discussed elsewhere) that
need to be fixed before it is too widely used or promoted.

On 9/9/10 4:10 PM, "Matthew McCormick (thewtex)" <matt at mmmccormick.com>
wrote:

> Hi Hans, Brad, Luis, et al.,
> 
> I drafted a git pre-commit hook to run uncrustify and KWStyle checks on
> changes to the code.
> 
> uncrustify (uncrustify.sourceforge.net <http://uncrustify.sourceforge.net> )
> is applied to changed files prior to
> commit.
> 
> To enable this behavior, set
>   git config hooks.uncrustify true
> 
> By default, the behavior of git-mergetool is used to review the changes
> uncrustify makes before they are added to the commit.  For more information on
> this behavior, see
>  git help mergetool
> To disable review with mergetool
>   git config hooks.uncrustify.mergetool false
> All changes suggested by uncrustify will automatically be added to the
> commit (dangerous?).
> 
> Hans, I took the uncrustify.conf from the wiki page here:
> http://www.itk.org/Wiki/ITKv4_StyleChangeProposal
> 
> I made one modification:
> --- .git/hooks/uncrustify.conf 2010-09-09 15:40:13.000000000 -0500
> +++ /tmp/Uncrustify_itk.txt 2010-09-09 15:52:52.945218676 -0500
> @@ -388,7 +388,7 @@
>  sp_inside_fparens                        = remove   # ignore/add/remove/force
>  
>  # Add or remove space inside function '(' and ')'
> -sp_inside_fparen                         = add   # ignore/add/remove/force
> +sp_inside_fparen                         = remove   # ignore/add/remove/force
>  
>  # Add or remove space between ']' and '(' when part of a function call.
>  sp_square_fparen                         = remove   # ignore/add/remove/force
> @@ -1197,3 +1197,4 @@
>  #
>  # You can assign any keyword to any type with the set option.
>  # set func_call_user _ N_
> +
> 
> I think this is more consistent with the codebase, increases readability, and
> it is easier to work with an advanced editor like emacs/vim.
> 
> 
> The KWStyle check on changed files is run by default.
> 
> KWStyle is run on the changed files and the commit is aborted if the files do
> not pass the test.  A file similar to the original is saved with a '*.kws'
> extension so that line numbers referenced in the error message can be
> examined.
> 
> The test is on by default.  To avoid running the KWStyle tests
>   git config hooks.KWStyle false
> 
> The configuration file for KWStyle was taken from master at
> Utilities/KWStyle/ITK.kws.xml.in <http://ITK.kws.xml.in> .
> 
> 
> There is one problem I have encountered that I was unable to find a good
> solution too.  I use vimdiff as my mergetool, and it will complain about the
> input not being a terminal, then the terminal will be corrupt after completing
> the commit message.  As a workaround for now, 'reset' is called after running
> the mergetool in the script.  Googling suggests this can happen when running
> vim in a pipe, but I am not sure what is going on.  Any pointers would be
> appreciated.
> 
> To give the hook a try,
> 
> cd ${ITK_SOURCE}/.git/hooks
> git pull git://github.com/thewtex/ITK.git <http://github.com/thewtex/ITK.git>
> hooks_uncrustify
> cd ../..
> git config hooks.uncrustify true
> 
> 
> Matt
> 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.itk.org/mailman/private/insight-developers/attachments/20100909/380f8cbf/attachment.htm>