GNU logs - #6318, boring messages

Message sent to owner <at> debbugs.gnu.org, bug-coreutils@HIDDEN:

Subject: bug#6318: reindenting with uncrustify, maybe...
Resent-From: Jim Meyering <jim@HIDDEN>
Original-Sender: debbugs-submit-bounces <at> debbugs.gnu.org
Resent-To: owner <at> debbugs.gnu.org
Resent-CC: bug-coreutils@HIDDEN
Resent-Date: Mon, 31 May 2010 11:04:01 +0000
Resent-Message-ID: <handler.6318.B.12753038325902 <at> debbugs.gnu.org>
Resent-Sender: help-debbugs@HIDDEN
To: 6318 <at> debbugs.gnu.org
Cc: uncrustify-developer@HIDDEN
From: Jim Meyering <jim@HIDDEN>
Date: Mon, 31 May 2010 13:03:30 +0200
Message-ID: <87iq64wcxp.fsf@HIDDEN>
Lines: 262
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Precedence: list
Sender: debbugs-submit-bounces <at> debbugs.gnu.org
Errors-To: debbugs-submit-bounces <at> debbugs.gnu.org

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

Message sent:

Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
MIME-Version: 1.0
X-Mailer: MIME-tools 5.427 (Entity 5.427)
Content-Type: text/plain; charset=utf-8
X-Loop: help-debbugs@HIDDEN
From: help-debbugs@HIDDEN (GNU bug Tracking System)
To: Jim Meyering <jim@HIDDEN>
Subject: bug#6318: Acknowledgement (reindenting with uncrustify, maybe...)
Message-ID: <handler.6318.B.12753038325902.ack <at> debbugs.gnu.org>
References: <87iq64wcxp.fsf@HIDDEN>
X-Gnu-PR-Message: ack 6318
X-Gnu-PR-Package: coreutils
Reply-To: 6318 <at> debbugs.gnu.org
Date: Mon, 31 May 2010 11:04:02 +0000

Thank you for filing a new bug report with GNU.

This is an automatically generated reply to let you know your message
has been received.

Your message is being forwarded to the package maintainers and other
interested parties for their attention; they will reply in due course.

Your message has been sent to the package maintainer(s):
 bug-coreutils@HIDDEN

If you wish to submit further information on this problem, please
send it to 6318 <at> debbugs.gnu.org.

Please do not send mail to help-debbugs@HIDDEN unless you wish
to report a problem with the Bug-tracking system.

--=20
6318: http://debbugs.gnu.org/cgi/bugreport.cgi?bug=3D6318
GNU Bug Tracking System
Contact help-debbugs@HIDDEN with problems

Message sent to owner <at> debbugs.gnu.org, bug-coreutils@HIDDEN:

Subject: bug#6318: [Uncrustify-developer] reindenting with uncrustify, maybe...
Resent-From: Jim Meyering <jim@HIDDEN>
Original-Sender: debbugs-submit-bounces <at> debbugs.gnu.org
Resent-To: owner <at> debbugs.gnu.org
Resent-CC: bug-coreutils@HIDDEN
Resent-Date: Mon, 31 May 2010 11:25:02 +0000
Resent-Message-ID: <handler.6318.B.12753050736428 <at> debbugs.gnu.org>
Resent-Sender: help-debbugs@HIDDEN
To: bug-coreutils@HIDDEN
Cc: uncrustify-developer@HIDDEN
From: Jim Meyering <jim@HIDDEN>
In-Reply-To: <87iq64wcxp.fsf@HIDDEN> (Jim Meyering's message of "Mon, 31
	May 2010 13:03:30 +0200")
References: <87iq64wcxp.fsf@HIDDEN>
Date: Mon, 31 May 2010 13:24:16 +0200
Message-ID: <87bpbwwbz3.fsf@HIDDEN>
Lines: 21
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Precedence: list
Sender: debbugs-submit-bounces <at> debbugs.gnu.org
Errors-To: debbugs-submit-bounces <at> debbugs.gnu.org

Jim Meyering wrote:
> Use this as ~/.uncrustify.cfg:
> --------------------------------------------------------------

I've just noticed this error in my .cfg file.
Don't know if adding the "#" changes anything.
The lack should at least provoke a config-file parse error.

diff --git a/.uncrustify.cfg b/.uncrustify.cfg
index a4cab0d..07fbc6e 100644
--- a/.uncrustify.cfg
+++ b/.uncrustify.cfg
@@ -82,7 +82,7 @@ sp_func_proto_paren		= force	# "int foo ();" vs "int foo();"

 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 };'
+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

Message sent to owner <at> debbugs.gnu.org, bug-coreutils@HIDDEN:

Subject: bug#6318: reindenting with uncrustify, maybe...
Resent-From: Paul Eggert <eggert@HIDDEN>
Original-Sender: debbugs-submit-bounces <at> debbugs.gnu.org
Resent-To: owner <at> debbugs.gnu.org
Resent-CC: bug-coreutils@HIDDEN
Resent-Date: Wed, 02 Jun 2010 18:53:01 +0000
Resent-Message-ID: <handler.6318.B6318.127550474520002 <at> debbugs.gnu.org>
Resent-Sender: help-debbugs@HIDDEN
To: Jim Meyering <jim@HIDDEN>
Cc: 6318 <at> debbugs.gnu.org, uncrustify-developer@HIDDEN
Message-ID: <4C06A85F.4090801@HIDDEN>
Date: Wed, 02 Jun 2010 11:52:15 -0700
From: Paul Eggert <eggert@HIDDEN>
Organization: UCLA Computer Science Department
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US;
	rv:1.9.1.9) Gecko/20100423 Thunderbird/3.0.4
MIME-Version: 1.0
References: <87iq64wcxp.fsf@HIDDEN>
In-Reply-To: <87iq64wcxp.fsf@HIDDEN>
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit
Precedence: list
Sender: debbugs-submit-bounces <at> debbugs.gnu.org
Errors-To: debbugs-submit-bounces <at> debbugs.gnu.org

On 05/31/2010 04:03 AM, Jim Meyering wrote:
> I'm considering whether to format coreutils' sources using uncrustify.

I tried your .uncrustify.cfg out on diffutils and ran into the
following issues.  Each issue is some diff output, followed by my
comments on that diff.  Diffutils is a different program, but I figure
the suggestion would come up eventually there, too.  Many of these
issues are minor annoyances, but some of them are a bit more serious.


@@ -1510,9 +1509,15 @@ output_diff3_edscript (FILE *outputfile,
       switch (type)
 	{
 	default: continue;
-	case DIFF_2ND: if (!show_2nd) continue; conflict = true; break;
-	case DIFF_3RD: if (overlap_only) continue; conflict = false; break;
-	case DIFF_ALL: if (simple_only) continue; conflict = flagging; break;
+        case DIFF_2ND: if (! show_2nd)
+            continue;
+          conflict = true; break;
+        case DIFF_3RD: if (overlap_only)
+            continue;
+          conflict = false; break;
+        case DIFF_ALL: if (simple_only)
+            continue;
+          conflict = flagging; break;
 	}
 
       low0 = D_LOWLINE (b, mapping[FILE0]);

Ouch!  This is a complete botch of reindenting.

----------------------------------------------------------------

 #if HAVE_SIGACTION
-  /* Prefer `sigaction' if available, since `signal' can lose signals.  */
-  static struct sigaction initial_action[NUM_SIGS];
+/* Prefer `sigaction' if available, since `signal' can lose signals.  */
+static struct sigaction initial_action[NUM_SIGS];
 # define initial_handler(i) (initial_action[i].sa_handler)
-  static void signal_handler (int, void (*) (int));
+static void signal_handler (int, void (*)(int));
 #else
-  static void (*initial_action[NUM_SIGS]) ();
+static void (*initial_action[NUM_SIGS]) ();
 # define initial_handler(i) (initial_action[i])
 # define signal_handler(sig, handler) signal (sig, handler)
 #endif

This is insisting on the style where preprocessor directives are
indented independently of the non-preprocessor directives.  But it's
sometimes (as here) nice to use consistent indenting, for both
directives and non-directives.

----------------------------------------------------------------

-  mbstate_t mbstate = { 0 };
+  mbstate_t mbstate = {0};

I don't see why the spaces were removed.

----------------------------------------------------------------

-	  while (changed0[i0]) ++i0;
-	  while (changed1[i1]) ++i1;
+          while (changed0[i0])
+            ++i0;
+          while (changed1[i1])
+            ++i1;

The old version is easier to read, since one can visually align the
bodies of the two loops.

----------------------------------------------------------------

@@ -496,7 +500,7 @@ diff_2_files (struct comparison *cmp)
 	changes = 0;
 
       else
-	/* Scan both files, a buffer at a time, looking for a difference.  */
+      /* Scan both files, a buffer at a time, looking for a difference.  */
 	{
 	  /* Allocate same-sized buffers for both files.  */
 	  size_t lcm_max = PTRDIFF_MAX - 1;

It's a bit weird to unindent the comment so that it's the same as
the preceding 'else'.

----------------------------------------------------------------

-  for (l0 = p0, l1 = p1;  (l = *l0) == *l1;  l0++, l1++)
+  for (l0 = p0, l1 = p1; (l = *l0) == *l1; l0++, l1++)

I find the version with the extra spaces a bit easier to read,
as the visual code-clumps match the semantics of the code.
Perhaps "uncrustify" could be taught to preserve extra spaces
in expressions when they match the expressions' semantics?

----------------------------------------------------------------

-  int offset_width IF_LINT (= 0);
+  int offset_width IF_LINT ( = 0);

I don't see why that space was inserted.

----------------------------------------------------------------

-	      sprintf (buf, "%"PRIdMAX".%.9d", sec, nsec);
+              sprintf (buf, "%" PRIdMAX ".%.9d", sec, nsec);

I'd rather not have those spaces inserted around the PRIdMAX.

----------------------------------------------------------------

@@ -601,7 +604,7 @@ make_3way_diff (struct diff_block *threa
       /* Make the diff you just got info from into the using class */
       using[high_water_thread]
 	= last_using[high_water_thread]
-	= high_water_diff;
+            = high_water_diff;
       current[high_water_thread] = high_water_diff->next;
...
@@ -1156,7 +1157,7 @@ compare_files (struct comparison const *
       char const *fnm = cmp.file[fnm_arg].name;
       char const *dir = cmp.file[dir_arg].name;
       char const *filename = cmp.file[dir_arg].name = free0
-	= dir_file_pathname (dir, last_component (fnm));
+                                                        = dir_file_pathname (dir, last_component (fnm));
 
       if (STREQ (fnm, "-"))
 	fatal ("cannot compare `-' to a directory");
@@ -1178,11 +1179,11 @@ compare_files (struct comparison const *
       /* Neither file "exists", so there's nothing to compare.  */
     }
   else if ((same_files
-	    = (cmp.file[0].desc != NONEXISTENT
-	       && cmp.file[1].desc != NONEXISTENT
-	       && 0 < same_file (&cmp.file[0].stat, &cmp.file[1].stat)
-	       && same_file_attributes (&cmp.file[0].stat,
-					&cmp.file[1].stat)))
+              = (cmp.file[0].desc != NONEXISTENT
+                 && cmp.file[1].desc != NONEXISTENT
+                 && 0 < same_file (&cmp.file[0].stat, &cmp.file[1].stat)
+                 && same_file_attributes (&cmp.file[0].stat,
+                                          &cmp.file[1].stat)))
 	   && no_diff_means_no_output)
     {
       /* The two named files are actually the same physical file.

These changes of indentation are all bizarre; the old indentation is
nicer.

----------------------------------------------------------------

   files_can_be_treated_as_binary =
     (brief & binary
-     & ~ (ignore_blank_lines | ignore_case | strip_trailing_cr
-	  | (ignore_regexp_list.regexps || ignore_white_space)));
+     & ~(ignore_blank_lines | ignore_case | strip_trailing_cr
+         | (ignore_regexp_list.regexps || ignore_white_space)));

It was odd to see lots of places where space was inserted after "!"
and unary "-", but then a space was _removed_ after "~".  Why the
inconsistency?

----------------------------------------------------------------

 int
 diff_dirs (struct comparison const *cmp,
-	   int (*handle_file) (struct comparison const *,
-			       char const *, char const *))
+           int (*handle_file)(struct comparison const *,
+                              char const *, char const *))
 {
   struct dirdata dirdata[2];
   int volatile val = EXIT_SUCCESS;
...
-	  int v1 = (*handle_file) (cmp,
-				   0 < nameorder ? 0 : *names[0]++,
-				   nameorder < 0 ? 0 : *names[1]++);
+          int v1 = (*handle_file)(cmp,
+                                  0 < nameorder ? 0 : *names[0]++,
+                                  nameorder < 0 ? 0 : *names[1]++);

----------------------------------------------------------------

The GNU style is to have a space between the function and
the opening parenthesis before the 1st argument.  Please
don't remove that space.

----------------------------------------------------------------

-      for (i = *bucket;  ;  i = eqs[i].next)
-	if (!i)
+      for (i = *bucket;; i = eqs[i].next)
+        if (! i)

Changing ";  ;" to ";;" is questionable.

----------------------------------------------------------------

 static int const sigs[] = {
 #ifdef SIGHUP
-       SIGHUP,
+  SIGHUP,
 #endif
 #ifdef SIGQUIT
-       SIGQUIT,
+  SIGQUIT,
 #endif
 #ifdef SIGTERM
-       SIGTERM,
+  SIGTERM,
 #endif

The old indentation made the identifiers line up better.

Message sent to owner <at> debbugs.gnu.org, bug-coreutils@HIDDEN:

X-Loop: help-debbugs@HIDDEN
Subject: bug#6318: reindenting with uncrustify, maybe...
Resent-From: Jim Meyering <jim@HIDDEN>
Original-Sender: debbugs-submit-bounces <at> debbugs.gnu.org
Resent-To: owner <at> debbugs.gnu.org
Resent-CC: bug-coreutils@HIDDEN
Resent-Date: Wed, 02 Jun 2010 20:30:03 +0000
Resent-Message-ID: <handler.6318.B6318.127551055222563 <at> debbugs.gnu.org>
Resent-Sender: help-debbugs@HIDDEN
X-GNU-PR-Message: followup 6318
X-GNU-PR-Package: coreutils
X-GNU-PR-Keywords: 
To: Paul Eggert <eggert@HIDDEN>
Cc: 6318 <at> debbugs.gnu.org, uncrustify-developer@HIDDEN
Received: via spool by 6318-submit <at> debbugs.gnu.org id=B6318.127551055222563
          (code B ref 6318); Wed, 02 Jun 2010 20:30:03 +0000
Received: (at 6318) by debbugs.gnu.org; 2 Jun 2010 20:29:12 +0000
Received: from localhost ([127.0.0.1] helo=debbugs.gnu.org)
	by debbugs.gnu.org with esmtp (Exim 4.69)
	(envelope-from <debbugs-submit-bounces <at> debbugs.gnu.org>)
	id 1OJuYp-0005rs-JA
	for submit <at> debbugs.gnu.org; Wed, 02 Jun 2010 16:29:12 -0400
Received: from smtp1-g21.free.fr ([212.27.42.1])
	by debbugs.gnu.org with esmtp (Exim 4.69)
	(envelope-from <meyering@HIDDEN>) id 1OJuYm-0005rh-NM
	for 6318 <at> debbugs.gnu.org; Wed, 02 Jun 2010 16:29:10 -0400
Received: from mx.meyering.net (mx.meyering.net [82.230.74.64])
	by smtp1-g21.free.fr (Postfix) with ESMTP id D94ED9401D9;
	Wed,  2 Jun 2010 22:28:58 +0200 (CEST)
Received: by rho.meyering.net (Acme Bit-Twister, from userid 1000)
	id CBA48E153; Wed,  2 Jun 2010 22:28:46 +0200 (CEST)
From: Jim Meyering <jim@HIDDEN>
In-Reply-To: <4C06A85F.4090801@HIDDEN> (Paul Eggert's message of "Wed, 02
	Jun 2010 11:52:15 -0700")
References: <87iq64wcxp.fsf@HIDDEN> <4C06A85F.4090801@HIDDEN>
Date: Wed, 02 Jun 2010 22:28:46 +0200
Message-ID: <87mxvdfabl.fsf@HIDDEN>
Lines: 282
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
X-Spam-Score: -3.3 (---)
X-BeenThere: debbugs-submit <at> debbugs.gnu.org
X-Mailman-Version: 2.1.11
Precedence: list
List-Id: <debbugs-submit.debbugs.gnu.org>
List-Unsubscribe: <http://debbugs.gnu.org/cgi-bin/mailman/options/debbugs-submit>,
	<mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=unsubscribe>
List-Archive: <http://debbugs.gnu.org/pipermail/debbugs-submit>
List-Post: <mailto:debbugs-submit <at> debbugs.gnu.org>
List-Help: <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=help>
List-Subscribe: <http://debbugs.gnu.org/cgi-bin/mailman/listinfo/debbugs-submit>,
	<mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=subscribe>
Sender: debbugs-submit-bounces <at> debbugs.gnu.org
Errors-To: debbugs-submit-bounces <at> debbugs.gnu.org
X-Spam-Score: -3.3 (---)

Paul Eggert wrote:

> On 05/31/2010 04:03 AM, Jim Meyering wrote:
>> I'm considering whether to format coreutils' sources using uncrustify.
>
> I tried your .uncrustify.cfg out on diffutils and ran into the
> following issues.  Each issue is some diff output, followed by my
> comments on that diff.  Diffutils is a different program, but I figure
> the suggestion would come up eventually there, too.  Many of these
> issues are minor annoyances, but some of them are a bit more serious.

Hi Paul,

Thanks for the detailed feedback.

> @@ -1510,9 +1509,15 @@ output_diff3_edscript (FILE *outputfile,
>        switch (type)
>  	{
>  	default: continue;
> -	case DIFF_2ND: if (!show_2nd) continue; conflict = true; break;
> -	case DIFF_3RD: if (overlap_only) continue; conflict = false; break;
> -	case DIFF_ALL: if (simple_only) continue; conflict = flagging; break;
> +        case DIFF_2ND: if (! show_2nd)
> +            continue;
> +          conflict = true; break;
> +        case DIFF_3RD: if (overlap_only)
> +            continue;
> +          conflict = false; break;
> +        case DIFF_ALL: if (simple_only)
> +            continue;
> +          conflict = flagging; break;
>  	}
>
>        low0 = D_LOWLINE (b, mapping[FILE0]);
>
> Ouch!  This is a complete botch of reindenting.

Maybe a bug.  Or maybe there's an option to force a newline after
a case statement's ":", and we just need to find it and turn it on.

> ----------------------------------------------------------------
>
>  #if HAVE_SIGACTION
> -  /* Prefer `sigaction' if available, since `signal' can lose signals.  */
> -  static struct sigaction initial_action[NUM_SIGS];
> +/* Prefer `sigaction' if available, since `signal' can lose signals.  */
> +static struct sigaction initial_action[NUM_SIGS];
>  # define initial_handler(i) (initial_action[i].sa_handler)
> -  static void signal_handler (int, void (*) (int));
> +static void signal_handler (int, void (*)(int));
>  #else
> -  static void (*initial_action[NUM_SIGS]) ();
> +static void (*initial_action[NUM_SIGS]) ();
>  # define initial_handler(i) (initial_action[i])
>  # define signal_handler(sig, handler) signal (sig, handler)
>  #endif
>
> This is insisting on the style where preprocessor directives are
> indented independently of the non-preprocessor directives.  But it's
> sometimes (as here) nice to use consistent indenting, for both
> directives and non-directives.

Would be nice, but how do we (not to mention the tool) know when it's desired?

> ----------------------------------------------------------------
>
> -  mbstate_t mbstate = { 0 };
> +  mbstate_t mbstate = {0};
>
> I don't see why the spaces were removed.

I could have gone either way, but there were far more cases
with no spaces, so I put this in the config file:

    sp_inside_braces_struct = remove # Add or remove space inside '{' and '}'
    sp_inside_braces = remove # Add or remove space inside '{' and '}'

> ----------------------------------------------------------------
>
> -	  while (changed0[i0]) ++i0;
> -	  while (changed1[i1]) ++i1;
> +          while (changed0[i0])
> +            ++i0;
> +          while (changed1[i1])
> +            ++i1;
>
> The old version is easier to read, since one can visually align the
> bodies of the two loops.

I agree that sometimes the one-line version is desired.
coreutils' sort.c also has code like that would be similarly
degraded if we were to convert today.

I hope we can arrange something.
uncrustify's code seems readable and maintainable enough that
if something needs to be changed and we're motivated enough,
we can do it ourselves.

> ----------------------------------------------------------------
>
> @@ -496,7 +500,7 @@ diff_2_files (struct comparison *cmp)
>  	changes = 0;
>
>        else
> -	/* Scan both files, a buffer at a time, looking for a difference.  */
> +      /* Scan both files, a buffer at a time, looking for a difference.  */
>  	{
>  	  /* Allocate same-sized buffers for both files.  */
>  	  size_t lcm_max = PTRDIFF_MAX - 1;
>
> It's a bit weird to unindent the comment so that it's the same as
> the preceding 'else'.
>
> ----------------------------------------------------------------
>
> -  for (l0 = p0, l1 = p1;  (l = *l0) == *l1;  l0++, l1++)
> +  for (l0 = p0, l1 = p1; (l = *l0) == *l1; l0++, l1++)
>
> I find the version with the extra spaces a bit easier to read,
> as the visual code-clumps match the semantics of the code.
> Perhaps "uncrustify" could be taught to preserve extra spaces
> in expressions when they match the expressions' semantics?

Sounds possible.
I wouldn't want to use two spaces all the time,
perhaps only when there are "," expressions in first and/or third term.

> ----------------------------------------------------------------
>
> -  int offset_width IF_LINT (= 0);
> +  int offset_width IF_LINT ( = 0);
>
> I don't see why that space was inserted.

I suppose it's because we're enforcing spaces around all such operators.

> ----------------------------------------------------------------
>
> -	      sprintf (buf, "%"PRIdMAX".%.9d", sec, nsec);
> +              sprintf (buf, "%" PRIdMAX ".%.9d", sec, nsec);
>
> I'd rather not have those spaces inserted around the PRIdMAX.

Special case PRI* macros when adjacent to literal strings?
Another opportunity to teach uncrustify something new?
I'd welcome the feature, but its lack wouldn't motivate
me to add that capability.

> ----------------------------------------------------------------
>
> @@ -601,7 +604,7 @@ make_3way_diff (struct diff_block *threa
>        /* Make the diff you just got info from into the using class */
>        using[high_water_thread]
>  	= last_using[high_water_thread]
> -	= high_water_diff;
> +            = high_water_diff;
>        current[high_water_thread] = high_water_diff->next;
> ...
> @@ -1156,7 +1157,7 @@ compare_files (struct comparison const *
>        char const *fnm = cmp.file[fnm_arg].name;
>        char const *dir = cmp.file[dir_arg].name;
>        char const *filename = cmp.file[dir_arg].name = free0
> -	= dir_file_pathname (dir, last_component (fnm));
> +                                                        = dir_file_pathname (dir, last_component (fnm));
>
>        if (STREQ (fnm, "-"))
>  	fatal ("cannot compare `-' to a directory");
> @@ -1178,11 +1179,11 @@ compare_files (struct comparison const *
>        /* Neither file "exists", so there's nothing to compare.  */
>      }
>    else if ((same_files
> -	    = (cmp.file[0].desc != NONEXISTENT
> -	       && cmp.file[1].desc != NONEXISTENT
> -	       && 0 < same_file (&cmp.file[0].stat, &cmp.file[1].stat)
> -	       && same_file_attributes (&cmp.file[0].stat,
> -					&cmp.file[1].stat)))
> +              = (cmp.file[0].desc != NONEXISTENT
> +                 && cmp.file[1].desc != NONEXISTENT
> +                 && 0 < same_file (&cmp.file[0].stat, &cmp.file[1].stat)
> +                 && same_file_attributes (&cmp.file[0].stat,
> +                                          &cmp.file[1].stat)))
>  	   && no_diff_means_no_output)
>      {
>        /* The two named files are actually the same physical file.
>
> These changes of indentation are all bizarre; the old indentation is
> nicer.
>
> ----------------------------------------------------------------
>
>    files_can_be_treated_as_binary =
>      (brief & binary
> -     & ~ (ignore_blank_lines | ignore_case | strip_trailing_cr
> -	  | (ignore_regexp_list.regexps || ignore_white_space)));
> +     & ~(ignore_blank_lines | ignore_case | strip_trailing_cr
> +         | (ignore_regexp_list.regexps || ignore_white_space)));
>
> It was odd to see lots of places where space was inserted after "!"
> and unary "-", but then a space was _removed_ after "~".  Why the
> inconsistency?

Here's the config setting that handles space after "!":

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

I suspect that it'd be easy to add sp_tilde to
have the same effect for "~".

> ----------------------------------------------------------------
>
>  int
>  diff_dirs (struct comparison const *cmp,
> -	   int (*handle_file) (struct comparison const *,
> -			       char const *, char const *))
> +           int (*handle_file)(struct comparison const *,
> +                              char const *, char const *))
>  {
>    struct dirdata dirdata[2];
>    int volatile val = EXIT_SUCCESS;
> ...
> -	  int v1 = (*handle_file) (cmp,
> -				   0 < nameorder ? 0 : *names[0]++,
> -				   nameorder < 0 ? 0 : *names[1]++);
> +          int v1 = (*handle_file)(cmp,
> +                                  0 < nameorder ? 0 : *names[0]++,
> +                                  nameorder < 0 ? 0 : *names[1]++);
>
> ----------------------------------------------------------------
>
> The GNU style is to have a space between the function and
> the opening parenthesis before the 1st argument.  Please
> don't remove that space.

That may be a bug.
I haven't investigated it yet.
I have made notes about similar problems in coreutils:

  # FIXME: teach uncrustify not to do this:
  # -           int (*cmp) (char const *, char const *))
  # +           int (*cmp)(char const *, char const *))
  # There are similar in pr.c:
  # -            (p->char_func) (' ');
  # +            (p->char_func)(' ');


> ----------------------------------------------------------------
>
> -      for (i = *bucket;  ;  i = eqs[i].next)
> -	if (!i)
> +      for (i = *bucket;; i = eqs[i].next)
> +        if (! i)
>
> Changing ";  ;" to ";;" is questionable.

I've just added this to my ~/.uncrustify.cfg, and it appears to do
part of what you want by leaving one space between the adjacent semicolons.

sp_before_semi_for_empty = add

> ----------------------------------------------------------------
>
>  static int const sigs[] = {
>  #ifdef SIGHUP
> -       SIGHUP,
> +  SIGHUP,
>  #endif
>  #ifdef SIGQUIT
> -       SIGQUIT,
> +  SIGQUIT,
>  #endif
>  #ifdef SIGTERM
> -       SIGTERM,
> +  SIGTERM,
>  #endif
>
> The old indentation made the identifiers line up better.

Agreed, but I've resigned myself to making a few compromises,
and so far the price looks right, once we get a few of these
nits ironed out.

Message sent to owner <at> debbugs.gnu.org, bug-coreutils@HIDDEN:

X-Loop: help-debbugs@HIDDEN
Subject: bug#6318: [Uncrustify-developer] reindenting with uncrustify, maybe...
Resent-From: Jim Meyering <jim@HIDDEN>
Original-Sender: debbugs-submit-bounces <at> debbugs.gnu.org
Resent-To: owner <at> debbugs.gnu.org
Resent-CC: bug-coreutils@HIDDEN
Resent-Date: Thu, 03 Jun 2010 07:20:02 +0000
Resent-Message-ID: <handler.6318.B.12755495696981 <at> debbugs.gnu.org>
Resent-Sender: help-debbugs@HIDDEN
X-GNU-PR-Message: followup 6318
X-GNU-PR-Package: coreutils
X-GNU-PR-Keywords: 
To: bug-coreutils@HIDDEN
Cc: uncrustify-developer@HIDDEN
Received: via spool by submit <at> debbugs.gnu.org id=B.12755495696981
          (code B ref -1); Thu, 03 Jun 2010 07:20:02 +0000
Received: (at submit) by debbugs.gnu.org; 3 Jun 2010 07:19:29 +0000
Received: from localhost ([127.0.0.1] helo=debbugs.gnu.org)
	by debbugs.gnu.org with esmtp (Exim 4.69)
	(envelope-from <debbugs-submit-bounces <at> debbugs.gnu.org>)
	id 1OK4i9-0001oY-0c
	for submit <at> debbugs.gnu.org; Thu, 03 Jun 2010 03:19:29 -0400
Received: from mail.gnu.org ([199.232.76.166] helo=mx10.gnu.org)
	by debbugs.gnu.org with esmtp (Exim 4.69)
	(envelope-from <meyering@HIDDEN>) id 1OK4i6-0001oS-MT
	for submit <at> debbugs.gnu.org; Thu, 03 Jun 2010 03:19:27 -0400
Received: from lists.gnu.org ([199.232.76.165]:56036)
	by monty-python.gnu.org with esmtps
	(TLS-1.0:DHE_RSA_AES_256_CBC_SHA1:32) (Exim 4.60)
	(envelope-from <meyering@HIDDEN>) id 1OK4i3-00047o-Md
	for submit <at> debbugs.gnu.org; Thu, 03 Jun 2010 03:19:23 -0400
Received: from [140.186.70.92] (port=39649 helo=eggs.gnu.org)
	by lists.gnu.org with esmtp (Exim 4.43) id 1OK4i1-0007qw-My
	for bug-coreutils@HIDDEN; Thu, 03 Jun 2010 03:19:23 -0400
X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on eggs.gnu.org
X-Spam-Level: 
X-Spam-Status: No, score=-1.9 required=5.0 tests=BAYES_00,RCVD_IN_DNSWL_NONE
	autolearn=unavailable version=3.3.1
Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.69)
	(envelope-from <meyering@HIDDEN>) id 1OK4i0-0004cm-Ik
	for bug-coreutils@HIDDEN; Thu, 03 Jun 2010 03:19:21 -0400
Received: from smtp1-g21.free.fr ([212.27.42.1]:54326)
	by eggs.gnu.org with esmtp (Exim 4.69)
	(envelope-from <meyering@HIDDEN>) id 1OK4hz-0004cA-Vu
	for bug-coreutils@HIDDEN; Thu, 03 Jun 2010 03:19:20 -0400
Received: from mx.meyering.net (mx.meyering.net [82.230.74.64])
	by smtp1-g21.free.fr (Postfix) with ESMTP id B736F940113
	for <bug-coreutils@HIDDEN>; Thu,  3 Jun 2010 09:19:12 +0200 (CEST)
Received: by rho.meyering.net (Acme Bit-Twister, from userid 1000)
	id 2414BE02; Thu,  3 Jun 2010 09:19:11 +0200 (CEST)
From: Jim Meyering <jim@HIDDEN>
In-Reply-To: <87iq64wcxp.fsf@HIDDEN> (Jim Meyering's message of "Mon, 31
	May 2010 13:03:30 +0200")
References: <87iq64wcxp.fsf@HIDDEN>
Date: Thu, 03 Jun 2010 09:19:11 +0200
Message-ID: <87zkzc7fdc.fsf@HIDDEN>
Lines: 83
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.6 (newer, 3)
X-detected-operating-system: by monty-python.gnu.org: GNU/Linux 2.6,
	seldom 2.4 (older, 4)
X-Spam-Score: -5.3 (-----)
X-BeenThere: debbugs-submit <at> debbugs.gnu.org
X-Mailman-Version: 2.1.11
Precedence: list
List-Id: <debbugs-submit.debbugs.gnu.org>
List-Unsubscribe: <http://debbugs.gnu.org/cgi-bin/mailman/options/debbugs-submit>,
	<mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=unsubscribe>
List-Archive: <http://debbugs.gnu.org/pipermail/debbugs-submit>
List-Post: <mailto:debbugs-submit <at> debbugs.gnu.org>
List-Help: <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=help>
List-Subscribe: <http://debbugs.gnu.org/cgi-bin/mailman/listinfo/debbugs-submit>,
	<mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=subscribe>
Sender: debbugs-submit-bounces <at> debbugs.gnu.org
Errors-To: debbugs-submit-bounces <at> debbugs.gnu.org
X-Spam-Score: -5.3 (-----)

Jim Meyering wrote:
> I'm considering whether to format coreutils' sources using uncrustify.
...
> 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've fixed it for backslashes, but not yet for comments.
Here's a patch to make uncrustify honor its align_on_tabstop option
when aligning backslashes:

[Note: I let a personal preference sneak into this patch.
 I don't want to align-to-tabstop when there is only one continued line:

    #define foo \
      bar.................

 If others prefer to enable align-to-tabstop even for those
 situations, uncrustify could add an option.
]

From 94929cba20a7a589151eebf0dded38167bb195ba Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering@HIDDEN>
Date: Wed, 2 Jun 2010 13:14:28 +0200
Subject: [PATCH] Honor align_on_tabstop also when aligning backslashes.

* src/align.cpp (align_nl_cont): Honor align_on_tabstop also
when aligning backslashes.
---
 src/align.cpp |   18 ++++++++++++++++++
 1 files changed, 18 insertions(+), 0 deletions(-)

diff --git a/src/align.cpp b/src/align.cpp
index ff4f404..c79e5fc 100644
--- a/src/align.cpp
+++ b/src/align.cpp
@@ -1187,6 +1187,7 @@ static chunk_t *align_var_def_brace(chunk_t *start, int span, int *p_nl_count)
 chunk_t *align_nl_cont(chunk_t *start)
 {
    int        max_col = 0;
+   unsigned int n_continued = 0;
    chunk_t    *pc     = start;
    chunk_t    *tmp;
    ChunkStack cs;
@@ -1202,10 +1203,27 @@ chunk_t *align_nl_cont(chunk_t *start)
       if (pc->type == CT_NL_CONT)
       {
          align_add(cs, pc, max_col, 1, true);
+         n_continued++;
       }
       pc = chunk_get_next(pc);
    }

+   // Honor align_on_tabstop only when there are two or more continued lines.
+   if (cpd.settings[UO_align_on_tabstop].b && n_continued > 1)
+   {
+     int rem = (max_col - 1) % cpd.settings[UO_output_tab_size].n;
+     if (rem != 0)
+     {
+       // FIXME: cpd.settings[UO_output_tab_size].n is 0 here. that seems wrong.
+       int code_width = cpd.settings[UO_code_width].n;
+       code_width = code_width ? code_width : 80;
+
+       int t = max_col + cpd.settings[UO_output_tab_size].n - rem;
+       if (t < code_width)
+	 max_col = t;
+     }
+   }
+
    /* NL_CONT is always the last thing on a line */
    while ((tmp = cs.Pop()) != NULL)
    {
--
1.7.1.359.g19d56

Message sent to owner <at> debbugs.gnu.org, bug-coreutils@HIDDEN:

X-Loop: help-debbugs@HIDDEN
Subject: bug#6318: reindenting with uncrustify, maybe...
Resent-From: Paul Eggert <eggert@HIDDEN>
Original-Sender: debbugs-submit-bounces <at> debbugs.gnu.org
Resent-To: owner <at> debbugs.gnu.org
Resent-CC: bug-coreutils@HIDDEN
Resent-Date: Thu, 03 Jun 2010 23:44:02 +0000
Resent-Message-ID: <handler.6318.B6318.127560858532155 <at> debbugs.gnu.org>
Resent-Sender: help-debbugs@HIDDEN
X-GNU-PR-Message: followup 6318
X-GNU-PR-Package: coreutils
X-GNU-PR-Keywords: 
To: Jim Meyering <jim@HIDDEN>
Cc: 6318 <at> debbugs.gnu.org, uncrustify-developer@HIDDEN
Received: via spool by 6318-submit <at> debbugs.gnu.org id=B6318.127560858532155
          (code B ref 6318); Thu, 03 Jun 2010 23:44:02 +0000
Received: (at 6318) by debbugs.gnu.org; 3 Jun 2010 23:43:05 +0000
Received: from localhost ([127.0.0.1] helo=debbugs.gnu.org)
	by debbugs.gnu.org with esmtp (Exim 4.69)
	(envelope-from <debbugs-submit-bounces <at> debbugs.gnu.org>)
	id 1OKK40-0008Ma-No
	for submit <at> debbugs.gnu.org; Thu, 03 Jun 2010 19:43:04 -0400
Received: from kiwi.cs.ucla.edu ([131.179.128.19])
	by debbugs.gnu.org with esmtp (Exim 4.69)
	(envelope-from <eggert@HIDDEN>) id 1OKK3y-0008M7-5v
	for 6318 <at> debbugs.gnu.org; Thu, 03 Jun 2010 19:43:03 -0400
Received: from [131.179.64.200] (Penguin.CS.UCLA.EDU [131.179.64.200])
	by kiwi.cs.ucla.edu (8.13.8+Sun/8.13.8/UCLACS-6.0) with ESMTP id
	o53NgtWs026410; Thu, 3 Jun 2010 16:42:56 -0700 (PDT)
Message-ID: <4C083DFF.80008@HIDDEN>
Date: Thu, 03 Jun 2010 16:42:55 -0700
From: Paul Eggert <eggert@HIDDEN>
Organization: UCLA Computer Science Department
User-Agent: Mozilla/5.0 (X11; U; Linux i686; en-US;
	rv:1.9.1.9) Gecko/20100423 Thunderbird/3.0.4
MIME-Version: 1.0
References: <87iq64wcxp.fsf@HIDDEN> <4C06A85F.4090801@HIDDEN>
	<87mxvdfabl.fsf@HIDDEN>
In-Reply-To: <87mxvdfabl.fsf@HIDDEN>
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit
X-Spam-Score: -1.3 (-)
X-BeenThere: debbugs-submit <at> debbugs.gnu.org
X-Mailman-Version: 2.1.11
Precedence: list
List-Id: <debbugs-submit.debbugs.gnu.org>
List-Unsubscribe: <http://debbugs.gnu.org/cgi-bin/mailman/options/debbugs-submit>,
	<mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=unsubscribe>
List-Archive: <http://debbugs.gnu.org/pipermail/debbugs-submit>
List-Post: <mailto:debbugs-submit <at> debbugs.gnu.org>
List-Help: <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=help>
List-Subscribe: <http://debbugs.gnu.org/cgi-bin/mailman/listinfo/debbugs-submit>,
	<mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=subscribe>
Sender: debbugs-submit-bounces <at> debbugs.gnu.org
Errors-To: debbugs-submit-bounces <at> debbugs.gnu.org
X-Spam-Score: -2.4 (--)

On 06/02/2010 01:28 PM, Jim Meyering wrote:

> Maybe a bug.  Or maybe there's an option to force a newline after
> a case statement's ":", and we just need to find it and turn it on.

Hmm, I'm not sure I want uncrustify to be that aggressive about
reformatting.  In general, come to think of it, many of the things
I'm leery about in uncrustify come about when it inserts newlines.
Perhaps it should have an option to shut that off?


>> This is insisting on the style where preprocessor directives are
>> indented independently of the non-preprocessor directives.  But it's
>> sometimes (as here) nice to use consistent indenting, for both
>> directives and non-directives.
> 
> Would be nice, but how do we (not to mention the tool) know when it's desired?

How about if we assume that it's always desired?  That is a conservative
assumption, and should work reasonably well in practice.


> I hope we can arrange something.
> uncrustify's code seems readable and maintainable enough that
> if something needs to be changed and we're motivated enough,
> we can do it ourselves.

Yes, that's a big advantage.  It would be nice if this would
end up working out.


> I wouldn't want to use two spaces all the time,
> perhaps only when there are "," expressions in first and/or third term.

Could we have it use two spaces if there are already two spaces,
and use one space if there aren't?  Again, take the conservative
approach.


> I've just added this to my ~/.uncrustify.cfg, and it appears to do
> part of what you want by leaving one space between the adjacent semicolons.
> 
> sp_before_semi_for_empty = add

Thanks.  How should developers synchronize on their .uncrustify.cfg?
Surely this should be per-package, not per-developer.

Message sent to owner <at> debbugs.gnu.org, bug-coreutils@HIDDEN:

X-Loop: help-debbugs@HIDDEN
Subject: bug#6318: reindenting with uncrustify, maybe...
Resent-From: Jim Meyering <jim@HIDDEN>
Original-Sender: debbugs-submit-bounces <at> debbugs.gnu.org
Resent-To: owner <at> debbugs.gnu.org
Resent-CC: bug-coreutils@HIDDEN
Resent-Date: Fri, 04 Jun 2010 05:30:04 +0000
Resent-Message-ID: <handler.6318.B6318.127562937721236 <at> debbugs.gnu.org>
Resent-Sender: help-debbugs@HIDDEN
X-GNU-PR-Message: followup 6318
X-GNU-PR-Package: coreutils
X-GNU-PR-Keywords: 
To: Paul Eggert <eggert@HIDDEN>
Cc: 6318 <at> debbugs.gnu.org, uncrustify-developer@HIDDEN
Received: via spool by 6318-submit <at> debbugs.gnu.org id=B6318.127562937721236
          (code B ref 6318); Fri, 04 Jun 2010 05:30:04 +0000
Received: (at 6318) by debbugs.gnu.org; 4 Jun 2010 05:29:37 +0000
Received: from localhost ([127.0.0.1] helo=debbugs.gnu.org)
	by debbugs.gnu.org with esmtp (Exim 4.69)
	(envelope-from <debbugs-submit-bounces <at> debbugs.gnu.org>)
	id 1OKPTN-0005WT-1a
	for submit <at> debbugs.gnu.org; Fri, 04 Jun 2010 01:29:37 -0400
Received: from smtp1-g21.free.fr ([212.27.42.1])
	by debbugs.gnu.org with esmtp (Exim 4.69)
	(envelope-from <meyering@HIDDEN>) id 1OKPTL-0005WO-0N
	for 6318 <at> debbugs.gnu.org; Fri, 04 Jun 2010 01:29:36 -0400
Received: from mx.meyering.net (mx.meyering.net [82.230.74.64])
	by smtp1-g21.free.fr (Postfix) with ESMTP id A1EF49400F6;
	Fri,  4 Jun 2010 07:29:26 +0200 (CEST)
Received: by rho.meyering.net (Acme Bit-Twister, from userid 1000)
	id C1559D6BC; Fri,  4 Jun 2010 07:29:24 +0200 (CEST)
From: Jim Meyering <jim@HIDDEN>
In-Reply-To: <4C083DFF.80008@HIDDEN> (Paul Eggert's message of "Thu, 03
	Jun 2010 16:42:55 -0700")
References: <87iq64wcxp.fsf@HIDDEN> <4C06A85F.4090801@HIDDEN>
	<87mxvdfabl.fsf@HIDDEN> <4C083DFF.80008@HIDDEN>
Date: Fri, 04 Jun 2010 07:29:24 +0200
Message-ID: <87k4qf4b7v.fsf@HIDDEN>
Lines: 61
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
X-Spam-Score: -2.4 (--)
X-BeenThere: debbugs-submit <at> debbugs.gnu.org
X-Mailman-Version: 2.1.11
Precedence: list
List-Id: <debbugs-submit.debbugs.gnu.org>
List-Unsubscribe: <http://debbugs.gnu.org/cgi-bin/mailman/options/debbugs-submit>,
	<mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=unsubscribe>
List-Archive: <http://debbugs.gnu.org/pipermail/debbugs-submit>
List-Post: <mailto:debbugs-submit <at> debbugs.gnu.org>
List-Help: <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=help>
List-Subscribe: <http://debbugs.gnu.org/cgi-bin/mailman/listinfo/debbugs-submit>,
	<mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=subscribe>
Sender: debbugs-submit-bounces <at> debbugs.gnu.org
Errors-To: debbugs-submit-bounces <at> debbugs.gnu.org
X-Spam-Score: -3.3 (---)

Paul Eggert wrote:

> On 06/02/2010 01:28 PM, Jim Meyering wrote:
>
>> Maybe a bug.  Or maybe there's an option to force a newline after
>> a case statement's ":", and we just need to find it and turn it on.
>
> Hmm, I'm not sure I want uncrustify to be that aggressive about
> reformatting.  In general, come to think of it, many of the things
> I'm leery about in uncrustify come about when it inserts newlines.
> Perhaps it should have an option to shut that off?

Maybe it does, already.
I honestly don't know.  For each case like this
it's a bit of a treasure hunt: will we find the option
we want, already in working order?

>>> This is insisting on the style where preprocessor directives are
>>> indented independently of the non-preprocessor directives.  But it's
>>> sometimes (as here) nice to use consistent indenting, for both
>>> directives and non-directives.
>>
>> Would be nice, but how do we (not to mention the tool) know when it's desired?
>
> How about if we assume that it's always desired?  That is a conservative
> assumption, and should work reasonably well in practice.

And maybe there's already an option for that.
Care to look and/or to add it?

>> I hope we can arrange something.
>> uncrustify's code seems readable and maintainable enough that
>> if something needs to be changed and we're motivated enough,
>> we can do it ourselves.
>
> Yes, that's a big advantage.  It would be nice if this would
> end up working out.
>
>
>> I wouldn't want to use two spaces all the time,
>> perhaps only when there are "," expressions in first and/or third term.
>
> Could we have it use two spaces if there are already two spaces,
> and use one space if there aren't?  Again, take the conservative
> approach.

That'd be best, but from my brief foray into uncrustify's code,
there is no option for that.  Do you feel like investigating
and adding one, if needed?

>> I've just added this to my ~/.uncrustify.cfg, and it appears to do
>> part of what you want by leaving one space between the adjacent semicolons.
>>
>> sp_before_semi_for_empty = add
>
> Thanks.  How should developers synchronize on their .uncrustify.cfg?
> Surely this should be per-package, not per-developer.

Yes, of course.
I'm keeping the initial churn separate from any code base.
I will add it to coreutils' repository before we start using it in earnest.

Message received at control <at> debbugs.gnu.org:

Received: (at control) by debbugs.gnu.org; 13 Sep 2011 12:44:50 +0000
From debbugs-submit-bounces <at> debbugs.gnu.org Tue Sep 13 08:44:50 2011
Received: from localhost ([127.0.0.1] helo=debbugs.gnu.org)
	by debbugs.gnu.org with esmtp (Exim 4.69)
	(envelope-from <debbugs-submit-bounces <at> debbugs.gnu.org>)
	id 1R3SLq-0002mK-Ee
	for submit <at> debbugs.gnu.org; Tue, 13 Sep 2011 08:44:36 -0400
Received: from mx1.redhat.com ([209.132.183.28])
	by debbugs.gnu.org with esmtp (Exim 4.69)
	(envelope-from <jim@HIDDEN>) id 1R3SLi-0002m6-Jt
	for control <at> debbugs.gnu.org; Tue, 13 Sep 2011 08:44:29 -0400
Received: from int-mx01.intmail.prod.int.phx2.redhat.com
	(int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11])
	by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id p8DCdxRE000477
	(version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK)
	for <control <at> debbugs.gnu.org>; Tue, 13 Sep 2011 08:39:59 -0400
Received: from mx.meyering.net (ovpn01.gateway.prod.ext.phx2.redhat.com
	[10.5.9.1])
	by int-mx01.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP
	id p8DCdvS8007762
	for <control <at> debbugs.gnu.org>; Tue, 13 Sep 2011 08:39:58 -0400
Received: from rho.meyering.net (localhost.localdomain [127.0.0.1])
	by rho.meyering.net (Acme Bit-Twister) with ESMTP id C70D16006D
	for <control <at> debbugs.gnu.org>; Tue, 13 Sep 2011 14:39:56 +0200 (CEST)
From: Jim Meyering <jim@HIDDEN>
To: control <at> debbugs.gnu.org
Subject: Re: bug#6318: reindenting with uncrustify, maybe...
In-Reply-To: <4C06A85F.4090801@HIDDEN> (Paul Eggert's message of "Wed, 02
	Jun 2010 11:52:15 -0700")
References: <87iq64wcxp.fsf@HIDDEN> <4C06A85F.4090801@HIDDEN>
Date: Tue, 13 Sep 2011 14:39:56 +0200
Message-ID: <8762kw4nvn.fsf@HIDDEN>
Lines: 3
MIME-Version: 1.0
Content-Type: text/plain
X-Scanned-By: MIMEDefang 2.67 on 10.5.11.11
X-Spam-Score: -10.5 (----------)
X-Debbugs-Envelope-To: control
X-BeenThere: debbugs-submit <at> debbugs.gnu.org
X-Mailman-Version: 2.1.11
Precedence: list
List-Id: <debbugs-submit.debbugs.gnu.org>
List-Unsubscribe: <http://debbugs.gnu.org/cgi-bin/mailman/options/debbugs-submit>,
	<mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=unsubscribe>
List-Archive: <http://debbugs.gnu.org/cgi-bin/mailman/private/debbugs-submit>
List-Post: <mailto:debbugs-submit <at> debbugs.gnu.org>
List-Help: <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=help>
List-Subscribe: <http://debbugs.gnu.org/cgi-bin/mailman/listinfo/debbugs-submit>,
	<mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=subscribe>
Sender: debbugs-submit-bounces <at> debbugs.gnu.org
Errors-To: debbugs-submit-bounces <at> debbugs.gnu.org
X-Spam-Score: -10.5 (----------)

severity 6318 wishlist
tags 6318 + notabug
thanks

Message received at control <at> debbugs.gnu.org:

Received: (at control) by debbugs.gnu.org; 13 Sep 2011 12:44:50 +0000
From debbugs-submit-bounces <at> debbugs.gnu.org Tue Sep 13 08:44:50 2011
Received: from localhost ([127.0.0.1] helo=debbugs.gnu.org)
	by debbugs.gnu.org with esmtp (Exim 4.69)
	(envelope-from <debbugs-submit-bounces <at> debbugs.gnu.org>)
	id 1R3SLq-0002mK-Ee
	for submit <at> debbugs.gnu.org; Tue, 13 Sep 2011 08:44:36 -0400
Received: from mx1.redhat.com ([209.132.183.28])
	by debbugs.gnu.org with esmtp (Exim 4.69)
	(envelope-from <jim@HIDDEN>) id 1R3SLi-0002m6-Jt
	for control <at> debbugs.gnu.org; Tue, 13 Sep 2011 08:44:29 -0400
Received: from int-mx01.intmail.prod.int.phx2.redhat.com
	(int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11])
	by mx1.redhat.com (8.14.4/8.14.4) with ESMTP id p8DCdxRE000477
	(version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=OK)
	for <control <at> debbugs.gnu.org>; Tue, 13 Sep 2011 08:39:59 -0400
Received: from mx.meyering.net (ovpn01.gateway.prod.ext.phx2.redhat.com
	[10.5.9.1])
	by int-mx01.intmail.prod.int.phx2.redhat.com (8.13.8/8.13.8) with ESMTP
	id p8DCdvS8007762
	for <control <at> debbugs.gnu.org>; Tue, 13 Sep 2011 08:39:58 -0400
Received: from rho.meyering.net (localhost.localdomain [127.0.0.1])
	by rho.meyering.net (Acme Bit-Twister) with ESMTP id C70D16006D
	for <control <at> debbugs.gnu.org>; Tue, 13 Sep 2011 14:39:56 +0200 (CEST)
From: Jim Meyering <jim@HIDDEN>
To: control <at> debbugs.gnu.org
Subject: Re: bug#6318: reindenting with uncrustify, maybe...
In-Reply-To: <4C06A85F.4090801@HIDDEN> (Paul Eggert's message of "Wed, 02
	Jun 2010 11:52:15 -0700")
References: <87iq64wcxp.fsf@HIDDEN> <4C06A85F.4090801@HIDDEN>
Date: Tue, 13 Sep 2011 14:39:56 +0200
Message-ID: <8762kw4nvn.fsf@HIDDEN>
Lines: 3
MIME-Version: 1.0
Content-Type: text/plain
X-Scanned-By: MIMEDefang 2.67 on 10.5.11.11
X-Spam-Score: -10.5 (----------)
X-Debbugs-Envelope-To: control
X-BeenThere: debbugs-submit <at> debbugs.gnu.org
X-Mailman-Version: 2.1.11
Precedence: list
List-Id: <debbugs-submit.debbugs.gnu.org>
List-Unsubscribe: <http://debbugs.gnu.org/cgi-bin/mailman/options/debbugs-submit>,
	<mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=unsubscribe>
List-Archive: <http://debbugs.gnu.org/cgi-bin/mailman/private/debbugs-submit>
List-Post: <mailto:debbugs-submit <at> debbugs.gnu.org>
List-Help: <mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=help>
List-Subscribe: <http://debbugs.gnu.org/cgi-bin/mailman/listinfo/debbugs-submit>,
	<mailto:debbugs-submit-request <at> debbugs.gnu.org?subject=subscribe>
Sender: debbugs-submit-bounces <at> debbugs.gnu.org
Errors-To: debbugs-submit-bounces <at> debbugs.gnu.org
X-Spam-Score: -10.5 (----------)

severity 6318 wishlist
tags 6318 + notabug
thanks

Last modified: Mon, 25 Nov 2019 12:00:02 UTC

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997 nCipher Corporation Ltd, 1994-97 Ian Jackson.