From: Tomi Ollila <tomi.ollila@iki.fi>
To: David Bremner <david@tethera.net>, <notmuch@notmuchmail.org>
Subject: Re: [PATCH 1/2] uncrustify.cfg: initial support for notmuch coding
	style
In-Reply-To: <1326197228-10356-2-git-send-email-david@tethera.net>
References: <1324135695-15487-1-git-send-email-david@tethera.net>
	<1326197228-10356-1-git-send-email-david@tethera.net>
	<1326197228-10356-2-git-send-email-david@tethera.net>
User-Agent: Notmuch/0.11+71~gefa5d6c (http://notmuchmail.org) Emacs/23.3.1
	(i686-pc-linux-gnu)
Date: Tue, 17 Jan 2012 17:36:53 +0200
Message-ID: <yf6d3ai5oiy.fsf@taco2.nixu.fi>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Precedence: list

On Tue, 10 Jan 2012 08:07:07 -0400, David Bremner <david@tethera.net> wrote:
> From: David Bremner <bremner@debian.org>
> 
> Uncrustify is a free (as in GPL2+) tool that indents and beautifies
> C/C++ code. It is similar to GNU indent in functionality although
> probably more configurable (in fairness, indent has better
> documentation).  Uncrustify does not have the indent mis-feature of
> needing to have every typedef'ed type defined in the
> configuration (even standard types like size_t).
> 
> This configuration starts with the linux-kernel style from the
> uncrustify config, disables aggressive re-indenting of structs,
> and fine tunes the handling 'else' and braces.
> 
> In an ideal situation, running uncrustify on notmuch code would be
> NOP; currently this is not true for all files because 1) the
> configuration is not perfect 2) the coding style of notmuch is not
> completely consistent; in particular the treatment of braces after
> e.g. for (_) is not consistent.
> ---
>  uncrustify.cfg |   99 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++

Good stuff: while testing I did these changes:

--- uncrustify.cfg0	2012-01-17 16:20:18.686489325 +0200
+++ uncrustify.cfg	2012-01-17 17:02:39.664512588 +0200
@@ -21 +20,0 @@
-
@@ -60,0 +60,4 @@
+sp_before_ptr_star	= force
+sp_between_ptr_star	= remove
+sp_after_ptr_star	= remove
+
@@ -62 +65 @@
-sp_sizeof_paren		= remove	# "sizeof (int)" vs "sizeof(int)"
+sp_sizeof_paren		= force		# "sizeof (int)" vs "sizeof(int)"
@@ -93 +96 @@
-align_right_cmt_span	= 0
+align_right_cmt_span	= 1		# 0 -> 1 -- just to fix some alignments


The first three:

+sp_before_ptr_star	= force
+sp_between_ptr_star	= remove
+sp_after_ptr_star	= remove

are important and pretty uncontroversial; fixes more than breaks (if
anything)


+sp_sizeof_paren	= force		# "sizeof (int)" vs "sizeof(int)"

adds space after sizeof; do 'grep sizeof *.c' and you see this is
prevalent in the code. I think this is the right way (especially) as 
sizeof is an operator...


+align_right_cmt_span	= 1		# 0 -> 1 -- just to fix some alignments

This fixes, for example, comment alignment in g_mime_filter_headers_get_type()
(in gmime-filter-headers.c).


I also tested using this change:

@@ -66 +69 @@
-sp_after_cast		= force		# "(int) a" vs "(int)a"
+sp_after_cast		= remove	# "(int) a" vs "(int)a"

But this changes basically all the casts in code not to have space
after cast. I'd prefer this but it is not the prevalent style.

Note that just running 
uncrustify --replace -c uncrustify.cfg notmuch-show.c
does this:

format_part_start_json (unused (GMimeObject * part),

I.e. there is space after '*' and 'part'. This is probably
the old known problem: uncrustify thinks unused is function 
which takes int parameter and there is multiplication to be 
done. If there is way to tell uncrustify that GMimeObject is 
type then this change would not take place.


I run for f in *.c; do uncrustify --replace -c uncrustify.cfg $f; done

in notmuch source root directory and the output is pretty good, in addition
to the above problem there are these format_* structure initialization
changes: 
Quoting Austin: "The good news is that that structure is on its way out."

So, it would be good to get devel/uncrustify.cfg in place so developers
can start playing with it...

Tomi