Subject: bug#6318: reindenting with uncrustify, maybe...

From: Jim Meyering <[email-masked]>
Date: Mon, 31 May 2010 13:03:30 +0200

I'm considering whether to format coreutils' sources using uncrustify.

[Why? Because making the formatting automatic means
 1) we don't have to try to catch formatting violations manually, in review
 2) it will avoid subtle bugs like this one:
    http://thread.gmane.org/gmane.comp.emulators.libvirt/23715
]

uncrustify has far more options than indent, and it is maintained.

I've attached the .uncrustify.cfg file that I'm using.

To see what type of changes would currently be induced, first
get the latest uncrustify,

    git clone git://github.com/bengardner/uncrustify.git

build it from source, install it and then run these commands:

    uncrustify --replace -c ~/.uncrustify.cfg src/*.c

    # FIXME: make uncrustify treat two "!!" like "!" ?
    # -  tab_index -= !!tab_index;
    # +  tab_index -= ! ! tab_index;
    perl -pi -e 's/\! \! /\!\! /' src/*.c

    # FIXME: either teach uncrustify about case_* macros or undo this, post-run
    # -        case_GETOPT_HELP_CHAR;
    # +          case_GETOPT_HELP_CHAR;
    perl -pi -e 's/  (case_GETOPT_[^_]+_CHAR( \(|;$))/$1/' src/*.c

Here is a summary of some nits contributing to the induced 14K-line
diff.  If any of you can adjust the uncrustify config file I'm using
to make that difference smaller, I'd appreciate patches.

  - I do *not* want it to expand TABs used for comment and backslash
    alignment, but haven't yet found how to do that, or if it's even
    possible, in combination with the spaces-only-indentation mode.

    For example, I don't want this sort of change:

      -    case FTS_DC:		/* directory that causes cycles */
      +    case FTS_DC:                /* directory that causes cycles */

  - I prefer this spacing, so all of these are fine:
      -      while (!feof (in) && !ferror (in) && sum < BLOCKSIZE);
      +      while (! feof (in) && ! ferror (in) && sum < BLOCKSIZE);

  - This isn't a big deal either way.  There are many of these,
    so it'd be good to avoid the change.  I suspect that there's an
    option to do so, but haven't looked yet:

      diff --git a/src/cat.c b/src/cat.c
      index eebfb97..6a31f8d 100644
      --- a/src/cat.c
      +++ b/src/cat.c
      @@ -57,11 +57,11 @@ static int input_desc;
          an 18 digit counter needs about 1000y */
       #define LINE_COUNTER_BUF_LEN 20
       static char line_buf[LINE_COUNTER_BUF_LEN] =
      -  {
      -    ' ', ' ', ' ', ' ', ' ', ' ', ' ', ' ', ' ',
      -    ' ', ' ', ' ', ' ', ' ', ' ', ' ', ' ', '0',
      -    '\t', '\0'
      -  };
      +{
      +  ' ', ' ', ' ', ' ', ' ', ' ', ' ', ' ', ' ',
      +  ' ', ' ', ' ', ' ', ' ', ' ', ' ', ' ', '0',
      +  '\t', '\0'
      +};

  - There are a few more issues marked with FIXME comments in
    .uncrustify.cfg below.

Note that it did highlight a few real indentation nits.
For example, in dd.c's skip_via_lseek, one block is indented
by 3 rather than by the usual 2 spaces.

Use this as ~/.uncrustify.cfg:
--------------------------------------------------------------
# GNU format (sorta)

# If false, disable all multi-line comment changes, including cmt_width and
# leading chars. Default is true.
cmt_indent_multi = false

indent_with_tabs		= 0		# 1=indent to level only, 2=indent with tabs
input_tab_size			= 8		# original tab size
output_tab_size			= 8		# new tab size
indent_columns			= 2
# indent_label			= 2		# pos: absolute col, neg: relative column
indent_align_string		= true		# align broken strings
indent_brace			= 2

nl_enum_brace			= add		# "enum {" vs "enum \n {"
nl_union_brace			= add		# "union {" vs "union \n {"
nl_struct_brace			= add		# "struct {" vs "struct \n {"
nl_do_brace			= add		# "do {" vs "do \n {"
nl_if_brace			= add		# "if () {" vs "if () \n {"
nl_for_brace			= add		# "for () {" vs "for () \n {"
nl_else_brace			= add		# "else {" vs "else \n {"
nl_while_brace			= add		# "while () {" vs "while () \n {"
nl_switch_brace			= add		# "switch () {" vs "switch () \n {"
nl_func_var_def_blk		= 1
nl_before_case			= 0
nl_fcall_brace			= add		# "foo() {" vs "foo()\n{"
nl_fdef_brace			= add		# "int foo() {" vs "int foo()\n{"
# nl_after_return			= TRUE
nl_brace_while			= force
nl_brace_else			= add
nl_squeeze_ifdef		= TRUE

# mod_paren_on_return		= ignore	# "return 1;" vs "return (1);"
# mod_full_brace_if		= ignore	# "if (a) a--;" vs "if (a) { a--; }"
# mod_full_brace_for		= ignore	# "for () a--;" vs "for () { a--; }"
# mod_full_brace_do		= ignore	# "do a--; while ();" vs "do { a--; } while ();"
# mod_full_brace_while		= ignore	# "while (a) a--;" vs "while (a) { a--; }"

sp_before_semi			= remove
sp_return_paren                 = force  # space between 'return' and '('
sp_paren_paren			= remove	# space between (( and ))
sp_sizeof_paren			= remove	# "sizeof (int)" vs "sizeof(int)"
sp_before_sparen		= force		# "if (" vs "if("
sp_after_sparen			= force		# "if () {" vs "if (){"
sp_after_cast			= force # "(int) a" vs "(int)a"
sp_inside_braces		= force		# "{ 1 }" vs "{1}"
sp_inside_braces_struct		= force		# "{ 1 }" vs "{1}"
sp_inside_braces_enum		= force		# "{ 1 }" vs "{1}"
sp_inside_paren			= remove
sp_inside_fparen		= remove
sp_inside_sparen		= remove
#sp_type_func			= ignore
sp_assign			= force
sp_arith			= force
sp_bool				= force
sp_compare			= force
sp_after_comma			= force
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();"

# align_on_tabstop		= FALSE		# align on tabstops
# align_enum_equ_span		= 4
# align_nl_cont			= TRUE
# align_var_def_span		= 2
# align_var_def_inline		= TRUE
# align_var_def_star		= TRUE
# align_var_def_colon		= TRUE
# align_assign_span		= 1
# align_struct_init_span		= 3
# align_var_struct_span		= 3
# align_right_cmt_span		= 3
# align_pp_define_span		= 3
# align_pp_define_gap		= 4
# align_number_left		= TRUE
# align_typedef_span		= 5
# align_typedef_gap		= 3

# cmt_star_cont			= TRUE

eat_blanks_before_close_brace	= TRUE
eat_blanks_after_open_brace	= TRUE
nl_enum_leave_one_liners = TRUE Don't split one-line 'enum foo { BAR = 15 };'
sp_sizeof_paren = force # Add between 'sizeof' and '('

# no align

# Don't do this:
# -  {"archive", no_argument, NULL, 'a'},
# +  { "archive", no_argument, NULL, 'a'},
sp_inside_braces_struct = remove # Add or remove space inside '{' and '}'
sp_inside_braces = remove # Add or remove space inside '{' and '}'

# Don't do this:
# -  static struct option const long_options[] =
# -  {
# +  static struct option const long_options[] = {
# FIXME

# FIXME: manually unindent case_GETOPT_*

# Add or remove space after the final semicolon of an empty part of a for statement: for ( ; ; <here> ).
sp_after_semi_for_empty = force
# Add or remove space after ';' in non-empty 'for' statements. Default=Force
sp_after_semi_for = force

# FIXME: replace all "for (;;)" with "while (true)"

# Don't align these:
# -# define BIT(x)        ((uint_fast32_t) 1 << (x))
# -# define SBIT  BIT (31)
# +# define BIT(x) ((uint_fast32_t) 1 << (x))
# +# define SBIT   BIT (31)

# FIXME: Don't emit this: "for (;; )"
#
# FIXME: Don't insert empty lines like this:
# @@ -73,6 +74,7 @@ static void
#  src_to_dest_free (void *x)
#  {
#    struct Src_to_dest *a = x;
# +
#    free (a->name);
#    free (x);
#  }

# FIXME: Don't do this:
# -            state = ST_TERMYES; /* Another TERM can cancel */
# +            state = ST_TERMYES;  /* Another TERM can cancel */

# FIXME: Don't do this:
# @@ -215,7 +215,7 @@ append_quoted (const char *str)
#          case '=':
#            if (need_backslash)
#              APPEND_CHAR ('\\');
# -          /* Fall through */
# +        /* Fall through */
#
#          default:
#            need_backslash = true;

# Add space after the '!' (not) operator
# Currently this mangles "!!sym" into "! ! sym".
sp_not = add

# Tell uncrustify that _ and N_ are special "functions",
# and must have no space between their names and the following "(".
set func_call_user _ N_

# The number of newlines after a block of variable declarations.
# i.e., don't insert blank line func decls and first stmt
nl_func_var_def_blk = 0

# Spaces to indent '{' from 'case'.
# By default, the brace will appear under the 'c' in case.
# Usually set to 0 or indent_columns.
indent_case_brace = indent_columns

#  How to indent goto labels
#   >0 : absolute column where 1 is the leftmost column
#   <=0 : subtract from brace indent
indent_label = -indent_columns

# Doesn't work
# align_with_tabs			TRUE		# use tabs to align

indent_cmt_with_tabs		FALSE
align_keep_tabs			TRUE
align_on_tabstop		TRUE

# Not sure I want this.
# align_nl_cont			true

# Align trailing comment at or beyond column N; 'pulls in' comments as a
# bonus side effect (0=ignore)
align_right_cmt_at_col = 40

nl_end_of_file add