unofficial mirror of notmuch@notmuchmail.org
 help / color / mirror / code / Atom feed
* [PATCH] uncrustify.cfg: initial support for notmuch coding style
@ 2011-12-17 15:28 David Bremner
  2011-12-17 22:50 ` David Bremner
  2012-01-10 12:07 ` David Bremner
  0 siblings, 2 replies; 23+ messages in thread
From: David Bremner @ 2011-12-17 15:28 UTC (permalink / raw)
  To: notmuch; +Cc: David Bremner

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).

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 |   98 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 98 insertions(+), 0 deletions(-)
 create mode 100644 uncrustify.cfg

diff --git a/uncrustify.cfg b/uncrustify.cfg
new file mode 100644
index 0000000..7eab7c4
--- /dev/null
+++ b/uncrustify.cfg
@@ -0,0 +1,98 @@
+#
+# uncrustify config file for the linux kernel
+#
+# $Id: linux-indent.cfg 488 2006-09-09 12:44:38Z bengardner $
+# Taken from the uncrustify distribution under license (GPL2+)
+#
+# sample usage:
+#        uncrustify --replace -c uncrustify.cfg foo.c
+#
+#
+
+indent_with_tabs	= 2		# 1=indent to level only, 2=indent with tabs
+align_with_tabs		= TRUE		# use tabs to align
+align_on_tabstop	= TRUE          # align on tabstops
+input_tab_size		= 8		# original tab size
+output_tab_size		= 8		# new tab size
+indent_columns		= 4
+
+indent_label		= 2		# pos: absolute col, neg: relative column
+
+
+#
+# inter-symbol newlines
+#
+
+nl_enum_brace		= remove	# "enum {" vs "enum \n {"
+nl_union_brace		= remove	# "union {" vs "union \n {"
+nl_struct_brace		= remove	# "struct {" vs "struct \n {"
+nl_do_brace             = remove	# "do {" vs "do \n {"
+nl_if_brace             = remove	# "if () {" vs "if () \n {"
+nl_for_brace            = remove	# "for () {" vs "for () \n {"
+nl_else_brace           = remove	# "else {" vs "else \n {"
+nl_while_brace          = remove	# "while () {" vs "while () \n {"
+nl_switch_brace         = remove	# "switch () {" vs "switch () \n {"
+nl_brace_while		= remove	# "} while" vs "} \n while" - cuddle while
+nl_brace_else		= remove	# "} else" vs "} \n else" - cuddle else
+nl_func_var_def_blk	= 1
+nl_fcall_brace		= remove	# "list_for_each() {" vs "list_for_each()\n{"
+nl_fdef_brace		= force		# "int foo() {" vs "int foo()\n{"
+# nl_after_return		= TRUE;
+# nl_before_case	= 1
+
+# Add or remove newline between return type and function name in definition
+nl_func_type_name	= force
+
+#
+# Source code modifications
+#
+
+# mod_paren_on_return	= remove	# "return 1;" vs "return (1);"
+# mod_full_brace_if	= remove	# "if (a) a--;" vs "if (a) { a--; }"
+# mod_full_brace_for	= remove	# "for () a--;" vs "for () { a--; }"
+# mod_full_brace_do	= remove	# "do a--; while ();" vs "do { a--; } while ();"
+# mod_full_brace_while	= remove	# "while (a) a--;" vs "while (a) { a--; }"
+
+
+#
+# inter-character spacing options
+#
+
+# sp _return_paren	= force		# "return (1);" vs "return(1);"
+sp_sizeof_paren		= remove	# "sizeof (int)" vs "sizeof(int)"
+sp_before_sparen	= force		# "if (" vs "if("
+sp_after_sparen		= force		# "if () {" vs "if (){"
+sp_sparen_brace		= force
+sp_after_cast		= force		# "(int) a" vs "(int)a"
+sp_inside_braces	= add		# "{ 1 }" vs "{1}"
+sp_inside_braces_struct	= add		# "{ 1 }" vs "{1}"
+sp_inside_braces_enum	= add		# "{ 1 }" vs "{1}"
+sp_assign		= force
+sp_arith		= force
+sp_bool			= add
+sp_compare		= add
+sp_assign		= add
+sp_after_comma		= add
+sp_func_def_paren	= force		# "int foo (){" vs "int foo(){"
+sp_func_call_paren	= force         # "foo (" vs "foo("
+sp_func_proto_paren	= force		# "int foo ();" vs "int foo();"
+
+#
+# Aligning stuff
+#
+
+align_enum_equ_span	= 4		# '=' in enum definition
+# align_nl_cont		= TRUE
+# align_var_def_span	= 2
+# align_var_def_inline	= TRUE
+# align_var_def_star	= FALSE
+# align_var_def_colon	= TRUE
+# align_assign_span	= 1
+align_struct_init_span	= 3		# align stuff in a structure init '= { }'
+align_right_cmt_span	= 3
+# align_pp_define_span	= 8;
+# align_pp_define_gap	= 4;
+
+# cmt_star_cont		= FALSE
+
+# indent_brace		= 0
-- 
1.7.7.3

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [PATCH] uncrustify.cfg: initial support for notmuch coding style
  2011-12-17 15:28 [PATCH] uncrustify.cfg: initial support for notmuch coding style David Bremner
@ 2011-12-17 22:50 ` David Bremner
  2011-12-17 23:57   ` Austin Clements
  2012-01-10 12:07 ` David Bremner
  1 sibling, 1 reply; 23+ messages in thread
From: David Bremner @ 2011-12-17 22:50 UTC (permalink / raw)
  To: notmuch

On Sat, 17 Dec 2011 11:28:15 -0400, David Bremner <david@tethera.net> wrote:
> +#
> +# sample usage:
> +#        uncrustify --replace -c uncrustify.cfg foo.c

There are several different possible workflows, here is what I have
found convenient.  To simplify the example, suppose we have a single
patch, and no uncommitted changes.

I like to do this one file at a time; I'm not sure there is really any
good reason.

1) $ uncrustify --replace -c uncrustify.cfg a.c

2) Now carefully look at the changes and selectively add the ones you
   agree with. I use magit (in emacs) for this. Git citool can also
   stage a line at a time.

3) $ git commit --amend

4) $ git reset --hard # blow away any changes you didn't agree with.
                   # remember, no uncommited changes to start with 
                   
Now repeat for each other c/c++/.h file your patch modifies.

In principle you could do something similar to produce "Style cleanups
for file a.c" commits by doing a regular commit in step 3.

d

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] uncrustify.cfg: initial support for notmuch coding style
  2011-12-17 22:50 ` David Bremner
@ 2011-12-17 23:57   ` Austin Clements
  2011-12-18  1:37     ` David Bremner
  0 siblings, 1 reply; 23+ messages in thread
From: Austin Clements @ 2011-12-17 23:57 UTC (permalink / raw)
  To: David Bremner; +Cc: notmuch

Quoth David Bremner on Dec 17 at  6:50 pm:
> On Sat, 17 Dec 2011 11:28:15 -0400, David Bremner <david@tethera.net> wrote:
> > +#
> > +# sample usage:
> > +#        uncrustify --replace -c uncrustify.cfg foo.c
> 
> There are several different possible workflows, here is what I have
> found convenient.  To simplify the example, suppose we have a single
> patch, and no uncommitted changes.

Not knowing anything about uncrustify, would it be possible to set up
a pre-commit hook that at least rejects newly added code that doesn't
fit the style?

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] uncrustify.cfg: initial support for notmuch coding style
  2011-12-17 23:57   ` Austin Clements
@ 2011-12-18  1:37     ` David Bremner
  0 siblings, 0 replies; 23+ messages in thread
From: David Bremner @ 2011-12-18  1:37 UTC (permalink / raw)
  To: Austin Clements; +Cc: notmuch

On Sat, 17 Dec 2011 18:57:33 -0500, Austin Clements <amdragon@MIT.EDU> wrote:

> Not knowing anything about uncrustify, would it be possible to set up
> a pre-commit hook that at least rejects newly added code that doesn't
> fit the style?

I think that would be possible. It might take a little slightly fancy
scripting since uncrustify just outputs the indented file.  Moved
code might yield false positives, and we'd probably need to find tune
the config a bit for this to be usable.

d

^ permalink raw reply	[flat|nested] 23+ messages in thread

* (no subject)
  2011-12-17 15:28 [PATCH] uncrustify.cfg: initial support for notmuch coding style David Bremner
  2011-12-17 22:50 ` David Bremner
@ 2012-01-10 12:07 ` David Bremner
  2012-01-10 12:07   ` [PATCH 1/2] uncrustify.cfg: initial support for notmuch coding style David Bremner
                     ` (2 more replies)
  1 sibling, 3 replies; 23+ messages in thread
From: David Bremner @ 2012-01-10 12:07 UTC (permalink / raw)
  To: notmuch

After a little fine-tuning, and some consensus about brace style on
the list, I think this is getting pretty useful. I attach a demo
uncrustification, where IMHO the output is mostly good.  Note that it
catches some style mistakes in quite recent changes.

One thing I wondered about where to put put the file; the top
directory starts to become a bit cluttered. One possibility is to make
a directory called something like HACKING/ which has files intended
for developers in it (so RELEASING could go there, along with any
coding style guides, maybe a version of the patch preparation guide),
and put uncrustify.cfg there.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH 1/2] uncrustify.cfg: initial support for notmuch coding style
  2012-01-10 12:07 ` David Bremner
@ 2012-01-10 12:07   ` David Bremner
  2012-01-12  4:00     ` Austin Clements
                       ` (2 more replies)
  2012-01-10 12:07   ` [PATCH 2/2] notmuch-reply.c: uncrustify David Bremner
  2012-01-11 16:03   ` where to put uncrustify... (was: Re: ) Tomi Ollila
  2 siblings, 3 replies; 23+ messages in thread
From: David Bremner @ 2012-01-10 12:07 UTC (permalink / raw)
  To: notmuch; +Cc: David Bremner

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 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 1 files changed, 99 insertions(+), 0 deletions(-)
 create mode 100644 uncrustify.cfg

diff --git a/uncrustify.cfg b/uncrustify.cfg
new file mode 100644
index 0000000..6613425
--- /dev/null
+++ b/uncrustify.cfg
@@ -0,0 +1,99 @@
+#
+# uncrustify config file for the linux kernel
+#
+# $Id: linux-indent.cfg 488 2006-09-09 12:44:38Z bengardner $
+# Taken from the uncrustify distribution under license (GPL2+)
+#
+# sample usage:
+#        uncrustify --replace -c uncrustify.cfg foo.c
+#
+#
+
+indent_with_tabs	= 2		# 1=indent to level only, 2=indent with tabs
+align_with_tabs		= TRUE		# use tabs to align
+align_on_tabstop	= TRUE          # align on tabstops
+input_tab_size		= 8		# original tab size
+output_tab_size		= 8		# new tab size
+indent_columns		= 4
+
+indent_label		= 2		# pos: absolute col, neg: relative column
+
+
+#
+# inter-symbol newlines
+#
+
+nl_enum_brace		= remove	# "enum {" vs "enum \n {"
+nl_union_brace		= remove	# "union {" vs "union \n {"
+nl_struct_brace		= remove	# "struct {" vs "struct \n {"
+nl_do_brace             = remove	# "do {" vs "do \n {"
+nl_if_brace             = remove	# "if () {" vs "if () \n {"
+nl_for_brace            = remove	# "for () {" vs "for () \n {"
+nl_else_brace           = remove	# "else {" vs "else \n {"
+nl_while_brace          = remove	# "while () {" vs "while () \n {"
+nl_switch_brace         = remove	# "switch () {" vs "switch () \n {"
+nl_brace_while		= remove	# "} while" vs "} \n while" - cuddle while
+nl_brace_else		= remove	# "} else" vs "} \n else" - cuddle else
+nl_func_var_def_blk	= 1
+nl_fcall_brace		= remove	# "list_for_each() {" vs "list_for_each()\n{"
+nl_fdef_brace		= force		# "int foo() {" vs "int foo()\n{"
+# nl_after_return		= TRUE;
+# nl_before_case	= 1
+
+# Add or remove newline between return type and function name in definition
+nl_func_type_name	= force
+
+#
+# Source code modifications
+#
+
+# mod_paren_on_return	= remove	# "return 1;" vs "return (1);"
+# mod_full_brace_if	= remove	# "if (a) a--;" vs "if (a) { a--; }"
+# mod_full_brace_for	= remove	# "for () a--;" vs "for () { a--; }"
+# mod_full_brace_do	= remove	# "do a--; while ();" vs "do { a--; } while ();"
+# mod_full_brace_while	= remove	# "while (a) a--;" vs "while (a) { a--; }"
+
+
+#
+# inter-character spacing options
+#
+
+# sp _return_paren	= force		# "return (1);" vs "return(1);"
+sp_sizeof_paren		= remove	# "sizeof (int)" vs "sizeof(int)"
+sp_before_sparen	= force		# "if (" vs "if("
+sp_after_sparen		= force		# "if () {" vs "if (){"
+sp_sparen_brace		= force
+sp_after_cast		= force		# "(int) a" vs "(int)a"
+sp_inside_braces	= add		# "{ 1 }" vs "{1}"
+sp_inside_braces_struct	= add		# "{ 1 }" vs "{1}"
+sp_inside_braces_enum	= add		# "{ 1 }" vs "{1}"
+sp_assign		= force
+sp_arith		= force
+sp_bool			= add
+sp_compare		= add
+sp_assign		= add
+sp_after_comma		= add
+sp_func_def_paren	= force		# "int foo (){" vs "int foo(){"
+sp_func_call_paren	= force         # "foo (" vs "foo("
+sp_func_proto_paren	= force		# "int foo ();" vs "int foo();"
+sp_brace_else		= force         # "} else" vs "}else"
+sp_else_brace		= force		# "else {" vs "else{"
+#
+# Aligning stuff
+#
+
+align_enum_equ_span	= 4		# '=' in enum definition
+# align_nl_cont		= TRUE
+# align_var_def_span	= 2
+# align_var_def_inline	= TRUE
+# align_var_def_star	= FALSE
+# align_var_def_colon	= TRUE
+# align_assign_span	= 1
+align_struct_init_span	= 0		# align stuff in a structure init '= { }'
+align_right_cmt_span	= 0
+# align_pp_define_span	= 8;
+# align_pp_define_gap	= 4;
+
+# cmt_star_cont		= FALSE
+
+# indent_brace		= 0
-- 
1.7.7.3

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* [PATCH 2/2] notmuch-reply.c: uncrustify
  2012-01-10 12:07 ` David Bremner
  2012-01-10 12:07   ` [PATCH 1/2] uncrustify.cfg: initial support for notmuch coding style David Bremner
@ 2012-01-10 12:07   ` David Bremner
  2012-01-11 15:22     ` Tomi Ollila
  2012-01-12  3:57     ` Austin Clements
  2012-01-11 16:03   ` where to put uncrustify... (was: Re: ) Tomi Ollila
  2 siblings, 2 replies; 23+ messages in thread
From: David Bremner @ 2012-01-10 12:07 UTC (permalink / raw)
  To: notmuch; +Cc: David Bremner

From: David Bremner <bremner@debian.org>

This patch shows the raw result of running uncrustify on notmuch-reply.c.
The re-indenting of "format_reply" would probably not be desirable.
---
 notmuch-reply.c |  160 +++++++++++++++++++++++++-----------------------------
 1 files changed, 74 insertions(+), 86 deletions(-)

diff --git a/notmuch-reply.c b/notmuch-reply.c
index 000f6da..00c9923 100644
--- a/notmuch-reply.c
+++ b/notmuch-reply.c
@@ -32,17 +32,17 @@ reply_part_content (GMimeObject *part);
 
 static const notmuch_show_format_t format_reply = {
     "",
-	"", NULL,
-	    "", NULL, reply_headers_message_part, ">\n",
-	    "",
-	        NULL,
-	        NULL,
-	        NULL,
-	        reply_part_content,
-	        NULL,
-	        "",
-	    "",
-	"", "",
+    "", NULL,
+    "", NULL, reply_headers_message_part, ">\n",
+    "",
+    NULL,
+    NULL,
+    NULL,
+    reply_part_content,
+    NULL,
+    "",
+    "",
+    "", "",
     ""
 };
 
@@ -54,14 +54,14 @@ show_reply_headers (GMimeMessage *message)
     stream_stdout = g_mime_stream_file_new (stdout);
     if (stream_stdout) {
 	g_mime_stream_file_set_owner (GMIME_STREAM_FILE (stream_stdout), FALSE);
-	stream_filter = g_mime_stream_filter_new(stream_stdout);
+	stream_filter = g_mime_stream_filter_new (stream_stdout);
 	if (stream_filter) {
-		g_mime_stream_filter_add(GMIME_STREAM_FILTER(stream_filter),
-					 g_mime_filter_headers_new());
-		g_mime_object_write_to_stream(GMIME_OBJECT(message), stream_filter);
-		g_object_unref(stream_filter);
+	    g_mime_stream_filter_add (GMIME_STREAM_FILTER (stream_filter),
+				      g_mime_filter_headers_new ());
+	    g_mime_object_write_to_stream (GMIME_OBJECT (message), stream_filter);
+	    g_object_unref (stream_filter);
 	}
-	g_object_unref(stream_stdout);
+	g_object_unref (stream_stdout);
     }
 }
 
@@ -94,18 +94,13 @@ reply_part_content (GMimeObject *part)
     GMimeContentDisposition *disposition = g_mime_object_get_content_disposition (part);
 
     if (g_mime_content_type_is_type (content_type, "multipart", "*") ||
-	g_mime_content_type_is_type (content_type, "message", "rfc822"))
-    {
+	g_mime_content_type_is_type (content_type, "message", "rfc822")) {
 	/* Output nothing, since multipart subparts will be handled individually. */
-    }
-    else if (g_mime_content_type_is_type (content_type, "application", "pgp-encrypted") ||
-	     g_mime_content_type_is_type (content_type, "application", "pgp-signature"))
-    {
+    } else if (g_mime_content_type_is_type (content_type, "application", "pgp-encrypted") ||
+	       g_mime_content_type_is_type (content_type, "application", "pgp-signature")) {
 	/* Ignore PGP/MIME cruft parts */
-    }
-    else if (g_mime_content_type_is_type (content_type, "text", "*") &&
-	!g_mime_content_type_is_type (content_type, "text", "html"))
-    {
+    } else if (g_mime_content_type_is_type (content_type, "text", "*") &&
+	       !g_mime_content_type_is_type (content_type, "text", "html")) {
 	GMimeStream *stream_stdout = NULL, *stream_filter = NULL;
 	GMimeDataWrapper *wrapper;
 	const char *charset;
@@ -114,33 +109,28 @@ reply_part_content (GMimeObject *part)
 	stream_stdout = g_mime_stream_file_new (stdout);
 	if (stream_stdout) {
 	    g_mime_stream_file_set_owner (GMIME_STREAM_FILE (stream_stdout), FALSE);
-	    stream_filter = g_mime_stream_filter_new(stream_stdout);
+	    stream_filter = g_mime_stream_filter_new (stream_stdout);
 	    if (charset) {
-		g_mime_stream_filter_add(GMIME_STREAM_FILTER(stream_filter),
-					 g_mime_filter_charset_new(charset, "UTF-8"));
+		g_mime_stream_filter_add (GMIME_STREAM_FILTER (stream_filter),
+					  g_mime_filter_charset_new (charset, "UTF-8"));
 	    }
 	}
-	g_mime_stream_filter_add(GMIME_STREAM_FILTER(stream_filter),
-				 g_mime_filter_reply_new(TRUE));
+	g_mime_stream_filter_add (GMIME_STREAM_FILTER (stream_filter),
+				  g_mime_filter_reply_new (TRUE));
 	wrapper = g_mime_part_get_content_object (GMIME_PART (part));
 	if (wrapper && stream_filter)
 	    g_mime_data_wrapper_write_to_stream (wrapper, stream_filter);
 	if (stream_filter)
-	    g_object_unref(stream_filter);
+	    g_object_unref (stream_filter);
 	if (stream_stdout)
-	    g_object_unref(stream_stdout);
-    }
-    else
-    {
+	    g_object_unref (stream_stdout);
+    } else {
 	if (disposition &&
-	    strcmp (disposition->disposition, GMIME_DISPOSITION_ATTACHMENT) == 0)
-	{
+	    strcmp (disposition->disposition, GMIME_DISPOSITION_ATTACHMENT) == 0) {
 	    const char *filename = g_mime_part_get_filename (GMIME_PART (part));
 	    printf ("Attachment: %s (%s)\n", filename,
 		    g_mime_content_type_to_string (content_type));
-	}
-	else
-	{
+	} else {
 	    printf ("Non-text part: %s\n",
 		    g_mime_content_type_to_string (content_type));
 	}
@@ -276,8 +266,7 @@ reply_to_header_is_redundant (notmuch_message_t *message)
     cc = notmuch_message_get_header (message, "cc");
 
     if ((to && strstr (to, addr) != 0) ||
-	(cc && strstr (cc, addr) != 0))
-    {
+	(cc && strstr (cc, addr) != 0)) {
 	return 1;
     }
 
@@ -345,16 +334,16 @@ add_recipients_from_message (GMimeMessage *reply,
 static const char *
 guess_from_received_header (notmuch_config_t *config, notmuch_message_t *message)
 {
-    const char *received,*primary,*by;
+    const char *received, *primary, *by;
     const char **other;
     char *tohdr;
-    char *mta,*ptr,*token;
-    char *domain=NULL;
-    char *tld=NULL;
-    const char *delim=". \t";
-    size_t i,j,other_len;
+    char *mta, *ptr, *token;
+    char *domain = NULL;
+    char *tld = NULL;
+    const char *delim = ". \t";
+    size_t i, j, other_len;
 
-    const char *to_headers[] = {"Envelope-to", "X-Original-To"};
+    const char *to_headers[] = { "Envelope-to", "X-Original-To" };
 
     primary = notmuch_config_get_user_primary_email (config);
     other = notmuch_config_get_user_other_email (config, &other_len);
@@ -373,22 +362,22 @@ guess_from_received_header (notmuch_config_t *config, notmuch_message_t *message
      *    'by' part of Received headers
      * If none of these work, we give up and return NULL
      */
-    for (i = 0; i < sizeof(to_headers)/sizeof(*to_headers); i++) {
-	tohdr = xstrdup(notmuch_message_get_header (message, to_headers[i]));
+    for (i = 0; i < sizeof(to_headers) / sizeof(*to_headers); i++) {
+	tohdr = xstrdup (notmuch_message_get_header (message, to_headers[i]));
 	if (tohdr && *tohdr) {
 	    /* tohdr is potentialy a list of email addresses, so here we
 	     * check if one of the email addresses is a substring of tohdr
 	     */
-	    if (strcasestr(tohdr, primary)) {
-		free(tohdr);
+	    if (strcasestr (tohdr, primary)) {
+		free (tohdr);
 		return primary;
 	    }
 	    for (j = 0; j < other_len; j++)
 		if (strcasestr (tohdr, other[j])) {
-		    free(tohdr);
+		    free (tohdr);
 		    return other[j];
 		}
-	    free(tohdr);
+	    free (tohdr);
 	}
     }
 
@@ -412,7 +401,7 @@ guess_from_received_header (notmuch_config_t *config, notmuch_message_t *message
 	 * so again we check if one of the email addresses is a
 	 * substring of ptr
 	 */
-	if (strcasestr(ptr, primary)) {
+	if (strcasestr (ptr, primary)) {
 	    return primary;
 	}
 	for (i = 0; i < other_len; i++)
@@ -430,12 +419,12 @@ guess_from_received_header (notmuch_config_t *config, notmuch_message_t *message
      * system in this step of the receive chain
      */
     by = received;
-    while((by = strstr (by, " by ")) != NULL) {
+    while ((by = strstr (by, " by ")) != NULL) {
 	by += 4;
 	if (*by == '\0')
 	    break;
 	mta = xstrdup (by);
-	token = strtok(mta," \t");
+	token = strtok (mta, " \t");
 	if (token == NULL) {
 	    free (mta);
 	    break;
@@ -458,15 +447,15 @@ guess_from_received_header (notmuch_config_t *config, notmuch_message_t *message
 	     * the test is the other way around: we check if this is a
 	     * substring of one of the email addresses.
 	     */
-	    *(tld-1) = '.';
+	    *(tld - 1) = '.';
 
-	    if (strcasestr(primary, domain)) {
-		free(mta);
+	    if (strcasestr (primary, domain)) {
+		free (mta);
 		return primary;
 	    }
 	    for (i = 0; i < other_len; i++)
-		if (strcasestr (other[i],domain)) {
-		    free(mta);
+		if (strcasestr (other[i], domain)) {
+		    free (mta);
 		    return other[i];
 		}
 	}
@@ -477,10 +466,10 @@ guess_from_received_header (notmuch_config_t *config, notmuch_message_t *message
 }
 
 static int
-notmuch_reply_format_default(void *ctx,
-			     notmuch_config_t *config,
-			     notmuch_query_t *query,
-			     notmuch_show_params_t *params)
+notmuch_reply_format_default (void *ctx,
+			      notmuch_config_t *config,
+			      notmuch_query_t *query,
+			      notmuch_show_params_t *params)
 {
     GMimeMessage *reply;
     notmuch_messages_t *messages;
@@ -491,8 +480,7 @@ notmuch_reply_format_default(void *ctx,
 
     for (messages = notmuch_query_search_messages (query);
 	 notmuch_messages_valid (messages);
-	 notmuch_messages_move_to_next (messages))
-    {
+	 notmuch_messages_move_to_next (messages)) {
 	message = notmuch_messages_get (messages);
 
 	/* The 1 means we want headers in a "pretty" order. */
@@ -524,7 +512,7 @@ notmuch_reply_format_default(void *ctx,
 				  "From", from_addr);
 
 	in_reply_to = talloc_asprintf (ctx, "<%s>",
-			     notmuch_message_get_message_id (message));
+				       notmuch_message_get_message_id (message));
 
 	g_mime_object_set_header (GMIME_OBJECT (reply),
 				  "In-Reply-To", in_reply_to);
@@ -555,10 +543,10 @@ notmuch_reply_format_default(void *ctx,
 
 /* This format is currently tuned for a git send-email --notmuch hook */
 static int
-notmuch_reply_format_headers_only(void *ctx,
-				  notmuch_config_t *config,
-				  notmuch_query_t *query,
-				  unused (notmuch_show_params_t *params))
+notmuch_reply_format_headers_only (void *ctx,
+				   notmuch_config_t *config,
+				   notmuch_query_t *query,
+				   unused (notmuch_show_params_t * params))
 {
     GMimeMessage *reply;
     notmuch_messages_t *messages;
@@ -568,8 +556,7 @@ notmuch_reply_format_headers_only(void *ctx,
 
     for (messages = notmuch_query_search_messages (query);
 	 notmuch_messages_valid (messages);
-	 notmuch_messages_move_to_next (messages))
-    {
+	 notmuch_messages_move_to_next (messages)) {
 	message = notmuch_messages_get (messages);
 
 	/* The 0 means we do not want headers in a "pretty" order. */
@@ -580,16 +567,16 @@ notmuch_reply_format_headers_only(void *ctx,
 	}
 
 	in_reply_to = talloc_asprintf (ctx, "<%s>",
-			     notmuch_message_get_message_id (message));
+				       notmuch_message_get_message_id (message));
 
-        g_mime_object_set_header (GMIME_OBJECT (reply),
+	g_mime_object_set_header (GMIME_OBJECT (reply),
 				  "In-Reply-To", in_reply_to);
 
 
 	orig_references = notmuch_message_get_header (message, "references");
 
 	/* We print In-Reply-To followed by References because git format-patch treats them
-         * specially.  Git does not interpret the other headers specially
+	 * specially.  Git does not interpret the other headers specially
 	 */
 	references = talloc_asprintf (ctx, "%s%s%s",
 				      orig_references ? orig_references : "",
@@ -598,7 +585,7 @@ notmuch_reply_format_headers_only(void *ctx,
 	g_mime_object_set_header (GMIME_OBJECT (reply),
 				  "References", references);
 
-	(void)add_recipients_from_message (reply, config, message);
+	(void) add_recipients_from_message (reply, config, message);
 
 	reply_headers = g_mime_object_to_string (GMIME_OBJECT (reply));
 	printf ("%s", reply_headers);
@@ -625,6 +612,7 @@ notmuch_reply_command (void *ctx, int argc, char *argv[])
     notmuch_query_t *query;
     char *query_string;
     int opt_index, ret = 0;
+
     int (*reply_format_func)(void *ctx, notmuch_config_t *config, notmuch_query_t *query, notmuch_show_params_t *params);
     notmuch_show_params_t params = { .part = -1 };
     int format = FORMAT_DEFAULT;
@@ -651,10 +639,10 @@ notmuch_reply_command (void *ctx, int argc, char *argv[])
 	reply_format_func = notmuch_reply_format_default;
 
     if (decrypt) {
-	GMimeSession* session = g_object_new (g_mime_session_get_type(), NULL);
+	GMimeSession* session = g_object_new (g_mime_session_get_type (), NULL);
 	params.cryptoctx = g_mime_gpg_context_new (session, "gpg");
 	if (params.cryptoctx) {
-	    g_mime_gpg_context_set_always_trust ((GMimeGpgContext*) params.cryptoctx, FALSE);
+	    g_mime_gpg_context_set_always_trust ((GMimeGpgContext *) params.cryptoctx, FALSE);
 	    params.decrypt = TRUE;
 	} else {
 	    fprintf (stderr, "Failed to construct gpg context.\n");
@@ -666,7 +654,7 @@ notmuch_reply_command (void *ctx, int argc, char *argv[])
     if (config == NULL)
 	return 1;
 
-    query_string = query_string_from_args (ctx, argc-opt_index, argv+opt_index);
+    query_string = query_string_from_args (ctx, argc - opt_index, argv + opt_index);
     if (query_string == NULL) {
 	fprintf (stderr, "Out of memory\n");
 	return 1;
@@ -695,7 +683,7 @@ notmuch_reply_command (void *ctx, int argc, char *argv[])
     notmuch_database_close (notmuch);
 
     if (params.cryptoctx)
-	g_object_unref(params.cryptoctx);
+	g_object_unref (params.cryptoctx);
 
     return ret;
 }
-- 
1.7.7.3

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [PATCH 2/2] notmuch-reply.c: uncrustify
  2012-01-10 12:07   ` [PATCH 2/2] notmuch-reply.c: uncrustify David Bremner
@ 2012-01-11 15:22     ` Tomi Ollila
  2012-01-12  3:57     ` Austin Clements
  1 sibling, 0 replies; 23+ messages in thread
From: Tomi Ollila @ 2012-01-11 15:22 UTC (permalink / raw)
  To: David Bremner, notmuch; +Cc: David Bremner

On Tue, 10 Jan 2012 08:07:08 -0400, David Bremner <david@tethera.net> wrote:
> From: David Bremner <bremner@debian.org>
> 
> This patch shows the raw result of running uncrustify on notmuch-reply.c.
> The re-indenting of "format_reply" would probably not be desirable.

Nice! It's easy to 'cherry-revert' that part. I wonder is using comments
could be used to preserver formatting in format_reply (and is that is
something wished to be done) i.e. something like:

static const notmuch_show_format_t format_reply = {
    "",
  /* */ "", NULL,
  /* ... */ "", NULL, reply_headers_message_part, ">\n",
  /* ... */ "",
  /* ....... */ NULL,
  /* ....... */ NULL,
  /* ....... */ NULL,
  /* ....... */ reply_part_content,
  /* ....... */ NULL,
  /* ....... */ "",
  /* ... */ "",
  /* */ "", "",


> ---
>  notmuch-reply.c |  160 +++++++++++++++++++++++++-----------------------------
>  1 files changed, 74 insertions(+), 86 deletions(-)
> 
> diff --git a/notmuch-reply.c b/notmuch-reply.c
> index 000f6da..00c9923 100644
> --- a/notmuch-reply.c
> +++ b/notmuch-reply.c
> @@ -32,17 +32,17 @@ reply_part_content (GMimeObject *part);
>  
>  static const notmuch_show_format_t format_reply = {
>      "",
> -	"", NULL,
> -	    "", NULL, reply_headers_message_part, ">\n",
> -	    "",
> -	        NULL,
> -	        NULL,
> -	        NULL,
> -	        reply_part_content,
> -	        NULL,
> -	        "",
> -	    "",
> -	"", "",
> +    "", NULL,
> +    "", NULL, reply_headers_message_part, ">\n",
> +    "",
> +    NULL,
> +    NULL,
> +    NULL,
> +    reply_part_content,
> +    NULL,
> +    "",
> +    "",
> +    "", "",

^ permalink raw reply	[flat|nested] 23+ messages in thread

* where to put uncrustify... (was: Re: )
  2012-01-10 12:07 ` David Bremner
  2012-01-10 12:07   ` [PATCH 1/2] uncrustify.cfg: initial support for notmuch coding style David Bremner
  2012-01-10 12:07   ` [PATCH 2/2] notmuch-reply.c: uncrustify David Bremner
@ 2012-01-11 16:03   ` Tomi Ollila
  2012-01-12  0:15     ` David Bremner
  2012-01-17 12:47     ` [PATCH] Start devel directory for developer tools and documentation David Bremner
  2 siblings, 2 replies; 23+ messages in thread
From: Tomi Ollila @ 2012-01-11 16:03 UTC (permalink / raw)
  To: David Bremner, notmuch

On Tue, 10 Jan 2012 08:07:06 -0400, David Bremner <david@tethera.net> wrote:
> After a little fine-tuning, and some consensus about brace style on
> the list, I think this is getting pretty useful. I attach a demo
> uncrustification, where IMHO the output is mostly good.  Note that it
> catches some style mistakes in quite recent changes.
> 
> One thing I wondered about where to put put the file; the top
> directory starts to become a bit cluttered. One possibility is to make
> a directory called something like HACKING/ which has files intended
> for developers in it (so RELEASING could go there, along with any
> coding style guides, maybe a version of the patch preparation guide),
> and put uncrustify.cfg there.

The usual problem: suitable names. HACKING sounds to me like
"Are you doing some serious work or just HACKING..." but I could 
not find anything better either. 

Any other thoughts ?

Tomi

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: where to put uncrustify... (was: Re: )
  2012-01-11 16:03   ` where to put uncrustify... (was: Re: ) Tomi Ollila
@ 2012-01-12  0:15     ` David Bremner
  2012-01-17 12:47     ` [PATCH] Start devel directory for developer tools and documentation David Bremner
  1 sibling, 0 replies; 23+ messages in thread
From: David Bremner @ 2012-01-12  0:15 UTC (permalink / raw)
  To: Tomi Ollila, notmuch

On Wed, 11 Jan 2012 18:03:14 +0200, Tomi Ollila <tomi.ollila@iki.fi> wrote:
> On Tue, 10 Jan 2012 08:07:06 -0400, David Bremner <david@tethera.net> wrote:
> 
> The usual problem: suitable names. HACKING sounds to me like
> "Are you doing some serious work or just HACKING..." but I could 
> not find anything better either. 
> 
> Any other thoughts ?

doc-devel, devel-doc, maint-doc, coding-docs?

d

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 2/2] notmuch-reply.c: uncrustify
  2012-01-10 12:07   ` [PATCH 2/2] notmuch-reply.c: uncrustify David Bremner
  2012-01-11 15:22     ` Tomi Ollila
@ 2012-01-12  3:57     ` Austin Clements
  2012-01-12 13:08       ` David Bremner
  1 sibling, 1 reply; 23+ messages in thread
From: Austin Clements @ 2012-01-12  3:57 UTC (permalink / raw)
  To: David Bremner; +Cc: notmuch, David Bremner

Quoth David Bremner on Jan 10 at  8:07 am:
> From: David Bremner <bremner@debian.org>
> 
> This patch shows the raw result of running uncrustify on notmuch-reply.c.
> The re-indenting of "format_reply" would probably not be desirable.

The good news is that that structure is on its way out.

> @@ -625,6 +612,7 @@ notmuch_reply_command (void *ctx, int argc, char *argv[])
>      notmuch_query_t *query;
>      char *query_string;
>      int opt_index, ret = 0;
> +

This one's a little weird.  Did it get confused by the function
prototype?

>      int (*reply_format_func)(void *ctx, notmuch_config_t *config, notmuch_query_t *query, notmuch_show_params_t *params);
>      notmuch_show_params_t params = { .part = -1 };
>      int format = FORMAT_DEFAULT;

All of the other format fixes look very reasonable to me.  I think you
managed to find one of the least canonical source files in the tree.
Was that intentional?

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 1/2] uncrustify.cfg: initial support for notmuch coding style
  2012-01-10 12:07   ` [PATCH 1/2] uncrustify.cfg: initial support for notmuch coding style David Bremner
@ 2012-01-12  4:00     ` Austin Clements
  2012-01-17 15:36     ` Tomi Ollila
  2012-01-21 21:15     ` David Bremner
  2 siblings, 0 replies; 23+ messages in thread
From: Austin Clements @ 2012-01-12  4:00 UTC (permalink / raw)
  To: David Bremner; +Cc: notmuch, David Bremner

Quoth David Bremner on Jan 10 at  8:07 am:
> 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.

Do you have examples of something this gets wrong besides the funny
indentation of notmuch_show_format_t?

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 2/2] notmuch-reply.c: uncrustify
  2012-01-12  3:57     ` Austin Clements
@ 2012-01-12 13:08       ` David Bremner
  2012-01-12 13:22         ` Jani Nikula
                           ` (2 more replies)
  0 siblings, 3 replies; 23+ messages in thread
From: David Bremner @ 2012-01-12 13:08 UTC (permalink / raw)
  To: Austin Clements; +Cc: notmuch

On Wed, 11 Jan 2012 22:57:39 -0500, Austin Clements <amdragon@MIT.EDU> wrote:
> > From: David Bremner <bremner@debian.org>
> > 
> > This patch shows the raw result of running uncrustify on notmuch-reply.c.
> > The re-indenting of "format_reply" would probably not be desirable.
> 
> The good news is that that structure is on its way out.
> 
> > @@ -625,6 +612,7 @@ notmuch_reply_command (void *ctx, int argc, char *argv[])
> >      notmuch_query_t *query;
> >      char *query_string;
> >      int opt_index, ret = 0;
> > +
> 
> This one's a little weird.  Did it get confused by the function
> prototype?

Yes, I think it is thinking the block of variable declarations ends
there.

> All of the other format fixes look very reasonable to me.  I think you
> managed to find one of the least canonical source files in the tree.
> Was that intentional?

Yes, I expect there would be less changes elsewhere. Although the
changes might be more controversial, I guess. 

The corresponding diff for notmuch-search.c follows

diff --git a/notmuch-search.c b/notmuch-search.c
index 4baab56..8928e62 100644
--- a/notmuch-search.c
+++ b/notmuch-search.c
@@ -31,16 +31,16 @@ typedef enum {
 typedef struct search_format {
     const char *results_start;
     const char *item_start;
-    void (*item_id) (const void *ctx,
-		     const char *item_type,
-		     const char *item_id);
-    void (*thread_summary) (const void *ctx,
-			    const char *thread_id,
-			    const time_t date,
-			    const int matched,
-			    const int total,
-			    const char *authors,
-			    const char *subject);
+    void (*item_id)(const void *ctx,
+		    const char *item_type,
+		    const char *item_id);
+    void (*thread_summary)(const void *ctx,
+			   const char *thread_id,
+			   const time_t date,
+			   const int matched,
+			   const int total,
+			   const char *authors,
+			   const char *subject);
     const char *tag_start;
     const char *tag;
     const char *tag_sep;
@@ -66,13 +66,13 @@ format_thread_text (const void *ctx,
 		    const char *subject);
 static const search_format_t format_text = {
     "",
-	"",
-	    format_item_id_text,
-	    format_thread_text,
-	    " (",
-		"%s", " ",
-	    ")", "\n",
-	"",
+    "",
+    format_item_id_text,
+    format_thread_text,
+    " (",
+    "%s", " ",
+    ")", "\n",
+    "",
     "\n",
     "",
 };
@@ -92,13 +92,13 @@ format_thread_json (const void *ctx,
 		    const char *subject);
 static const search_format_t format_json = {
     "[",
-	"{",
-	    format_item_id_json,
-	    format_thread_json,
-	    "\"tags\": [",
-		"\"%s\"", ", ",
-	    "]", ",\n",
-	"}",
+    "{",
+    format_item_id_json,
+    format_thread_json,
+    "\"tags\": [",
+    "\"%s\"", ", ",
+    "]", ",\n",
+    "}",
     "]\n",
     "]\n",
 };
@@ -122,7 +122,7 @@ sanitize_string (const void *ctx, const char *str)
     loop = out = talloc_strdup (ctx, str);
 
     for (; *loop; loop++) {
-	if ((unsigned char)(*loop) < 32)
+	if ((unsigned char) (*loop) < 32)
 	    *loop = '?';
     }
     return out;
@@ -160,7 +160,7 @@ format_item_id_json (const void *ctx,
     printf ("%s", json_quote_str (ctx_quote, item_id));
 
     talloc_free (ctx_quote);
-    
+
 }
 
 static void
@@ -221,8 +221,7 @@ do_search_threads (const search_format_t *format,
 
     for (i = 0;
 	 notmuch_threads_valid (threads) && (limit < 0 || i < offset + limit);
-	 notmuch_threads_move_to_next (threads), i++)
-    {
+	 notmuch_threads_move_to_next (threads), i++) {
 	int first_tag = 1;
 
 	thread = notmuch_threads_get (threads);
@@ -232,7 +231,7 @@ do_search_threads (const search_format_t *format,
 	    continue;
 	}
 
-	if (! first_thread)
+	if (!first_thread)
 	    fputs (format->item_sep, stdout);
 
 	if (output == OUTPUT_THREADS) {
@@ -258,9 +257,8 @@ do_search_threads (const search_format_t *format,
 
 	    for (tags = notmuch_thread_get_tags (thread);
 		 notmuch_tags_valid (tags);
-		 notmuch_tags_move_to_next (tags))
-	    {
-		if (! first_tag)
+		 notmuch_tags_move_to_next (tags)) {
+		if (!first_tag)
 		    fputs (format->tag_sep, stdout);
 		printf (format->tag, notmuch_tags_get (tags));
 		first_tag = 0;
@@ -311,8 +309,7 @@ do_search_messages (const search_format_t *format,
 
     for (i = 0;
 	 notmuch_messages_valid (messages) && (limit < 0 || i < offset + limit);
-	 notmuch_messages_move_to_next (messages), i++)
-    {
+	 notmuch_messages_move_to_next (messages), i++) {
 	if (i < offset)
 	    continue;
 
@@ -323,9 +320,8 @@ do_search_messages (const search_format_t *format,
 
 	    for (;
 		 notmuch_filenames_valid (filenames);
-		 notmuch_filenames_move_to_next (filenames))
-	    {
-		if (! first_message)
+		 notmuch_filenames_move_to_next (filenames)) {
+		if (!first_message)
 		    fputs (format->item_sep, stdout);
 
 		format->item_id (message, "",
@@ -333,11 +329,11 @@ do_search_messages (const search_format_t *format,
 
 		first_message = 0;
 	    }
-	    
-	    notmuch_filenames_destroy( filenames );
+
+	    notmuch_filenames_destroy ( filenames );
 
 	} else { /* output == OUTPUT_MESSAGES */
-	    if (! first_message)
+	    if (!first_message)
 		fputs (format->item_sep, stdout);
 
 	    format->item_id (message, "id:",
@@ -385,11 +381,10 @@ do_search_tags (notmuch_database_t *notmuch,
 
     for (;
 	 notmuch_tags_valid (tags);
-	 notmuch_tags_move_to_next (tags))
-    {
+	 notmuch_tags_move_to_next (tags)) {
 	tag = notmuch_tags_get (tags);
 
-	if (! first_tag)
+	if (!first_tag)
 	    fputs (format->item_sep, stdout);
 
 	format->item_id (tags, "", tag);
@@ -425,7 +420,7 @@ notmuch_search_command (void *ctx, int argc, char *argv[])
     int limit = -1; /* unlimited */
 
     enum { NOTMUCH_FORMAT_JSON, NOTMUCH_FORMAT_TEXT }
-	format_sel = NOTMUCH_FORMAT_TEXT;
+    format_sel = NOTMUCH_FORMAT_TEXT;
 
     notmuch_opt_desc_t options[] = {
 	{ NOTMUCH_OPT_KEYWORD, &sort, "sort", 's',
@@ -472,7 +467,7 @@ notmuch_search_command (void *ctx, int argc, char *argv[])
     if (notmuch == NULL)
 	return 1;
 
-    query_str = query_string_from_args (notmuch, argc-opt_index, argv+opt_index);
+    query_str = query_string_from_args (notmuch, argc - opt_index, argv + opt_index);
     if (query_str == NULL) {
 	fprintf (stderr, "Out of memory.\n");
 	return 1;

^ permalink raw reply related	[flat|nested] 23+ messages in thread

* Re: [PATCH 2/2] notmuch-reply.c: uncrustify
  2012-01-12 13:08       ` David Bremner
@ 2012-01-12 13:22         ` Jani Nikula
  2012-01-12 13:46         ` Tomi Ollila
  2012-01-12 15:26         ` Austin Clements
  2 siblings, 0 replies; 23+ messages in thread
From: Jani Nikula @ 2012-01-12 13:22 UTC (permalink / raw)
  To: David Bremner, Austin Clements; +Cc: notmuch

On Thu, 12 Jan 2012 09:08:18 -0400, David Bremner <david@tethera.net> wrote:
> The corresponding diff for notmuch-search.c follows

> -	    
> -	    notmuch_filenames_destroy( filenames );
> +
> +	    notmuch_filenames_destroy ( filenames );

Can you make it remove the extra spaces after ( and before ) too?


BR,
Jani.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 2/2] notmuch-reply.c: uncrustify
  2012-01-12 13:08       ` David Bremner
  2012-01-12 13:22         ` Jani Nikula
@ 2012-01-12 13:46         ` Tomi Ollila
  2012-01-12 15:26         ` Austin Clements
  2 siblings, 0 replies; 23+ messages in thread
From: Tomi Ollila @ 2012-01-12 13:46 UTC (permalink / raw)
  To: David Bremner, Austin Clements; +Cc: notmuch

On Thu, 12 Jan 2012 09:08:18 -0400, David Bremner <david@tethera.net> wrote:
> 
> > All of the other format fixes look very reasonable to me.  I think you
> > managed to find one of the least canonical source files in the tree.
> > Was that intentional?
> 
> Yes, I expect there would be less changes elsewhere. Although the
> changes might be more controversial, I guess. 
> 
> The corresponding diff for notmuch-search.c follows
> 
> diff --git a/notmuch-search.c b/notmuch-search.c

[ ... ]
> @@ -122,7 +122,7 @@ sanitize_string (const void *ctx, const char *str)
>      loop = out = talloc_strdup (ctx, str);
>  
>      for (; *loop; loop++) {
> -	if ((unsigned char)(*loop) < 32)
> +	if ((unsigned char) (*loop) < 32)
>  	    *loop = '?';

Should (cast) be concatenated without space to the castee

>      }
>      return out;
> @@ -160,7 +160,7 @@ format_item_id_json (const void *ctx,
>      printf ("%s", json_quote_str (ctx_quote, item_id));
>  
>      talloc_free (ctx_quote);
> -    
> +

Whee, whitespace-cleanup !

>  }
>  
[ ... ]


Tomi

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 2/2] notmuch-reply.c: uncrustify
  2012-01-12 13:08       ` David Bremner
  2012-01-12 13:22         ` Jani Nikula
  2012-01-12 13:46         ` Tomi Ollila
@ 2012-01-12 15:26         ` Austin Clements
  2012-01-21 19:00           ` David Bremner
  2 siblings, 1 reply; 23+ messages in thread
From: Austin Clements @ 2012-01-12 15:26 UTC (permalink / raw)
  To: David Bremner; +Cc: notmuch

Quoth David Bremner on Jan 12 at  9:08 am:
> @@ -425,7 +420,7 @@ notmuch_search_command (void *ctx, int argc, char *argv[])
>      int limit = -1; /* unlimited */
>  
>      enum { NOTMUCH_FORMAT_JSON, NOTMUCH_FORMAT_TEXT }
> -	format_sel = NOTMUCH_FORMAT_TEXT;
> +    format_sel = NOTMUCH_FORMAT_TEXT;

This is unfortunate, but other than the format structure flattening,
the remaining changes look reasonable to me.

>  
>      notmuch_opt_desc_t options[] = {
>  	{ NOTMUCH_OPT_KEYWORD, &sort, "sort", 's',

^ permalink raw reply	[flat|nested] 23+ messages in thread

* [PATCH] Start devel directory for developer tools and documentation.
  2012-01-11 16:03   ` where to put uncrustify... (was: Re: ) Tomi Ollila
  2012-01-12  0:15     ` David Bremner
@ 2012-01-17 12:47     ` David Bremner
  2012-01-17 23:23       ` Austin Clements
  2012-01-18  2:54       ` David Bremner
  1 sibling, 2 replies; 23+ messages in thread
From: David Bremner @ 2012-01-17 12:47 UTC (permalink / raw)
  To: notmuch; +Cc: David Bremner

From: David Bremner <bremner@debian.org>

We had a lot of back and forth about the name of this directory, but
nothing very conclusive. In the end, I just chose "devel" just to move
on.
---
 RELEASING => devel/RELEASING |    0
 TODO => devel/TODO           |    0
 2 files changed, 0 insertions(+), 0 deletions(-)
 rename RELEASING => devel/RELEASING (100%)
 rename TODO => devel/TODO (100%)

diff --git a/RELEASING b/devel/RELEASING
similarity index 100%
rename from RELEASING
rename to devel/RELEASING
diff --git a/TODO b/devel/TODO
similarity index 100%
rename from TODO
rename to devel/TODO
-- 
1.7.8.3

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 1/2] uncrustify.cfg: initial support for notmuch coding style
  2012-01-10 12:07   ` [PATCH 1/2] uncrustify.cfg: initial support for notmuch coding style David Bremner
  2012-01-12  4:00     ` Austin Clements
@ 2012-01-17 15:36     ` Tomi Ollila
  2012-01-21 21:15     ` David Bremner
  2 siblings, 0 replies; 23+ messages in thread
From: Tomi Ollila @ 2012-01-17 15:36 UTC (permalink / raw)
  To: David Bremner, notmuch

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

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] Start devel directory for developer tools and documentation.
  2012-01-17 12:47     ` [PATCH] Start devel directory for developer tools and documentation David Bremner
@ 2012-01-17 23:23       ` Austin Clements
  2012-01-18  2:54       ` David Bremner
  1 sibling, 0 replies; 23+ messages in thread
From: Austin Clements @ 2012-01-17 23:23 UTC (permalink / raw)
  To: David Bremner; +Cc: notmuch, David Bremner

Quoth David Bremner on Jan 17 at  8:47 am:
> From: David Bremner <bremner@debian.org>
> 
> We had a lot of back and forth about the name of this directory, but
> nothing very conclusive. In the end, I just chose "devel" just to move
> on.

LGTM.

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH] Start devel directory for developer tools and documentation.
  2012-01-17 12:47     ` [PATCH] Start devel directory for developer tools and documentation David Bremner
  2012-01-17 23:23       ` Austin Clements
@ 2012-01-18  2:54       ` David Bremner
  1 sibling, 0 replies; 23+ messages in thread
From: David Bremner @ 2012-01-18  2:54 UTC (permalink / raw)
  To: notmuch

On Tue, 17 Jan 2012 08:47:51 -0400, David Bremner <david@tethera.net> wrote:
> From: David Bremner <bremner@debian.org>
> 

pushed.

I need to find time to look at the uncrustify config again before I push
that.


d

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 2/2] notmuch-reply.c: uncrustify
  2012-01-12 15:26         ` Austin Clements
@ 2012-01-21 19:00           ` David Bremner
  0 siblings, 0 replies; 23+ messages in thread
From: David Bremner @ 2012-01-21 19:00 UTC (permalink / raw)
  To: Austin Clements; +Cc: notmuch

On Thu, 12 Jan 2012 10:26:52 -0500, Austin Clements <amdragon@MIT.EDU> wrote:
> Quoth David Bremner on Jan 12 at  9:08 am:
> > @@ -425,7 +420,7 @@ notmuch_search_command (void *ctx, int argc, char *argv[])
> >      int limit = -1; /* unlimited */
> >  
> >      enum { NOTMUCH_FORMAT_JSON, NOTMUCH_FORMAT_TEXT }
> > -	format_sel = NOTMUCH_FORMAT_TEXT;
> > +    format_sel = NOTMUCH_FORMAT_TEXT;
> 
> This is unfortunate, but other than the format structure flattening,
> the remaining changes look reasonable to me.

I guess we have to accept that this is a big pile of heuristics, as long
as there are not too many cases to be overriden, it should still be a
net win.

FWIW An alternative formatting that does not confuse the indenter is

enum format_enum {
     NOTMUCH_FORMAT_JSON, NOTMUCH_FORMAT_TEXT
} format_sel = NOTMUCH_FORMAT_TEXT;

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 1/2] uncrustify.cfg: initial support for notmuch coding style
  2012-01-10 12:07   ` [PATCH 1/2] uncrustify.cfg: initial support for notmuch coding style David Bremner
  2012-01-12  4:00     ` Austin Clements
  2012-01-17 15:36     ` Tomi Ollila
@ 2012-01-21 21:15     ` David Bremner
  2012-01-21 21:35       ` Tomi Ollila
  2 siblings, 1 reply; 23+ messages in thread
From: David Bremner @ 2012-01-21 21:15 UTC (permalink / raw)
  To: notmuch

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.

I pushed a revised version of this, with some input from Tomi.  I guess
this should be documented somewhere either the wiki, under devel, or
both. Perhaps we should work on a "coding style" page on the wiki, and
then copy snapshots of that into ./devel? Or just edit a text file in 
./devel?

d

^ permalink raw reply	[flat|nested] 23+ messages in thread

* Re: [PATCH 1/2] uncrustify.cfg: initial support for notmuch coding style
  2012-01-21 21:15     ` David Bremner
@ 2012-01-21 21:35       ` Tomi Ollila
  0 siblings, 0 replies; 23+ messages in thread
From: Tomi Ollila @ 2012-01-21 21:35 UTC (permalink / raw)
  To: notmuch

On Sat, 21 Jan 2012 17:15:08 -0400, David Bremner <david@tethera.net> wrote:
> 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.
> 
> I pushed a revised version of this, with some input from Tomi.  I guess
> this should be documented somewhere either the wiki, under devel, or
> both. Perhaps we should work on a "coding style" page on the wiki, and
> then copy snapshots of that into ./devel? Or just edit a text file in 
> ./devel?

It would be best to have the documents in ./devel and just references to
those on the wiki. To avoid duplication and content drifting.

That particular case:

      enum format_enum {  NOTMUCH_FORMAT_JSON, NOTMUCH_FORMAT_TEXT }
-          format_sel = NOTMUCH_FORMAT_TEXT;
+     format_sel = NOTMUCH_FORMAT_TEXT;

Is interesting, running emacs -q notmuch-search.c and tab-indenting that
same line yieds the same results. However, on top-level, i.e.:

enum format_enum { NOTMUCH_FORMAT_JSON, NOTMUCH_FORMAT_TEXT }
    format_sel = NOTMUCH_FORMAT_TEXT;

emacs indents just like above.

I like this suggestion:

>*>     enum format_enum {
>*>	    NOTMUCH_FORMAT_JSON, NOTMUCH_FORMAT_TEXT
>*>     } format_sel = NOTMUCH_FORMAT_TEXT;

> 
> d
> 

Tomi

^ permalink raw reply	[flat|nested] 23+ messages in thread

end of thread, other threads:[~2012-01-21 21:35 UTC | newest]

Thread overview: 23+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-12-17 15:28 [PATCH] uncrustify.cfg: initial support for notmuch coding style David Bremner
2011-12-17 22:50 ` David Bremner
2011-12-17 23:57   ` Austin Clements
2011-12-18  1:37     ` David Bremner
2012-01-10 12:07 ` David Bremner
2012-01-10 12:07   ` [PATCH 1/2] uncrustify.cfg: initial support for notmuch coding style David Bremner
2012-01-12  4:00     ` Austin Clements
2012-01-17 15:36     ` Tomi Ollila
2012-01-21 21:15     ` David Bremner
2012-01-21 21:35       ` Tomi Ollila
2012-01-10 12:07   ` [PATCH 2/2] notmuch-reply.c: uncrustify David Bremner
2012-01-11 15:22     ` Tomi Ollila
2012-01-12  3:57     ` Austin Clements
2012-01-12 13:08       ` David Bremner
2012-01-12 13:22         ` Jani Nikula
2012-01-12 13:46         ` Tomi Ollila
2012-01-12 15:26         ` Austin Clements
2012-01-21 19:00           ` David Bremner
2012-01-11 16:03   ` where to put uncrustify... (was: Re: ) Tomi Ollila
2012-01-12  0:15     ` David Bremner
2012-01-17 12:47     ` [PATCH] Start devel directory for developer tools and documentation David Bremner
2012-01-17 23:23       ` Austin Clements
2012-01-18  2:54       ` David Bremner

Code repositories for project(s) associated with this public inbox

	https://yhetil.org/notmuch.git/

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).