Jim Meyering <jim@HIDDEN>
to control <at> debbugs.gnu.org.
Full text available.Jim Meyering <jim@HIDDEN>
to control <at> debbugs.gnu.org.
Full text available.Received: (at 6318) by debbugs.gnu.org; 4 Jun 2010 05:29:37 +0000 From debbugs-submit-bounces <at> debbugs.gnu.org Fri Jun 04 01:29:37 2010 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> To: Paul Eggert <eggert@HIDDEN> Subject: Re: bug#6318: reindenting with uncrustify, maybe... 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-Debbugs-Envelope-To: 6318 Cc: 6318 <at> debbugs.gnu.org, uncrustify-developer@HIDDEN 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.
owner <at> debbugs.gnu.org, bug-coreutils@HIDDEN:bug#6318; Package coreutils.
Full text available.Received: (at 6318) by debbugs.gnu.org; 3 Jun 2010 23:43:05 +0000 From debbugs-submit-bounces <at> debbugs.gnu.org Thu Jun 03 19:43:05 2010 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 To: Jim Meyering <jim@HIDDEN> Subject: Re: bug#6318: reindenting with uncrustify, maybe... 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-Debbugs-Envelope-To: 6318 Cc: 6318 <at> debbugs.gnu.org, uncrustify-developer@HIDDEN 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.
owner <at> debbugs.gnu.org, bug-coreutils@HIDDEN:bug#6318; Package coreutils.
Full text available.
Received: (at submit) by debbugs.gnu.org; 3 Jun 2010 07:19:29 +0000
From debbugs-submit-bounces <at> debbugs.gnu.org Thu Jun 03 03:19:29 2010
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>
To: bug-coreutils@HIDDEN
Subject: Re: [Uncrustify-developer] reindenting with uncrustify, maybe...
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-Debbugs-Envelope-To: submit
Cc: uncrustify-developer@HIDDEN
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
owner <at> debbugs.gnu.org, bug-coreutils@HIDDEN:bug#6318; Package coreutils.
Full text available.
Received: (at 6318) by debbugs.gnu.org; 2 Jun 2010 20:29:12 +0000
From debbugs-submit-bounces <at> debbugs.gnu.org Wed Jun 02 16:29:12 2010
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>
To: Paul Eggert <eggert@HIDDEN>
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: 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-Debbugs-Envelope-To: 6318
Cc: 6318 <at> debbugs.gnu.org, uncrustify-developer@HIDDEN
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.
owner <at> debbugs.gnu.org, bug-coreutils@HIDDEN:bug#6318; Package coreutils.
Full text available.
Received: (at 6318) by debbugs.gnu.org; 2 Jun 2010 18:52:25 +0000
From debbugs-submit-bounces <at> debbugs.gnu.org Wed Jun 02 14:52:25 2010
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 1OJt3A-0005CZ-FL
for submit <at> debbugs.gnu.org; Wed, 02 Jun 2010 14:52:24 -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 1OJt37-0005CS-Bh
for 6318 <at> debbugs.gnu.org; Wed, 02 Jun 2010 14:52:23 -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
o52IqFmc003059; Wed, 2 Jun 2010 11:52:15 -0700 (PDT)
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
To: Jim Meyering <jim@HIDDEN>
Subject: Re: bug#6318: reindenting with uncrustify, maybe...
References: <87iq64wcxp.fsf@HIDDEN>
In-Reply-To: <87iq64wcxp.fsf@HIDDEN>
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit
X-Spam-Score: -1.3 (-)
X-Debbugs-Envelope-To: 6318
Cc: 6318 <at> debbugs.gnu.org, uncrustify-developer@HIDDEN
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 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.
owner <at> debbugs.gnu.org, bug-coreutils@HIDDEN:bug#6318; Package coreutils.
Full text available.
Received: (at submit) by debbugs.gnu.org; 31 May 2010 11:24:33 +0000
From debbugs-submit-bounces <at> debbugs.gnu.org Mon May 31 07:24:33 2010
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 1OJ36f-0001fd-Bu
for submit <at> debbugs.gnu.org; Mon, 31 May 2010 07:24:33 -0400
Received: from mx10.gnu.org ([199.232.76.166])
by debbugs.gnu.org with esmtp (Exim 4.69)
(envelope-from <meyering@HIDDEN>) id 1OJ36c-0001fY-RL
for submit <at> debbugs.gnu.org; Mon, 31 May 2010 07:24:31 -0400
Received: from lists.gnu.org ([199.232.76.165]:45657)
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 1OJ36a-0004aY-1t
for submit <at> debbugs.gnu.org; Mon, 31 May 2010 07:24:28 -0400
Received: from [140.186.70.92] (port=40000 helo=eggs.gnu.org)
by lists.gnu.org with esmtp (Exim 4.43) id 1OJ36Y-0000nP-Nv
for bug-coreutils@HIDDEN; Mon, 31 May 2010 07:24:27 -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 1OJ36X-0003K1-Ap
for bug-coreutils@HIDDEN; Mon, 31 May 2010 07:24:26 -0400
Received: from smtp1-g21.free.fr ([212.27.42.1]:50719)
by eggs.gnu.org with esmtp (Exim 4.69)
(envelope-from <meyering@HIDDEN>) id 1OJ36W-0003JK-OI
for bug-coreutils@HIDDEN; Mon, 31 May 2010 07:24:25 -0400
Received: from mx.meyering.net (mx.meyering.net [82.230.74.64])
by smtp1-g21.free.fr (Postfix) with ESMTP id E0041940045
for <bug-coreutils@HIDDEN>; Mon, 31 May 2010 13:24:17 +0200 (CEST)
Received: by rho.meyering.net (Acme Bit-Twister, from userid 1000)
id C0E3CC52; Mon, 31 May 2010 13:24:16 +0200 (CEST)
From: Jim Meyering <jim@HIDDEN>
To: bug-coreutils@HIDDEN
Subject: Re: [Uncrustify-developer] reindenting with uncrustify, maybe...
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
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-Debbugs-Envelope-To: submit
Cc: uncrustify-developer@HIDDEN
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:
> 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
owner <at> debbugs.gnu.org, bug-coreutils@HIDDEN:bug#6318; Package coreutils.
Full text available.
Received: (at submit) by debbugs.gnu.org; 31 May 2010 11:03:52 +0000
From debbugs-submit-bounces <at> debbugs.gnu.org Mon May 31 07:03:52 2010
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 1OJ2ma-0001X9-PS
for submit <at> debbugs.gnu.org; Mon, 31 May 2010 07:03:52 -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 1OJ2mX-0001X4-C2
for submit <at> debbugs.gnu.org; Mon, 31 May 2010 07:03:46 -0400
Received: from lists.gnu.org ([199.232.76.165]:55746)
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 1OJ2mU-0004AF-HS
for submit <at> debbugs.gnu.org; Mon, 31 May 2010 07:03:42 -0400
Received: from [140.186.70.92] (port=43104 helo=eggs.gnu.org)
by lists.gnu.org with esmtp (Exim 4.43) id 1OJ2mS-00064D-E8
for bug-coreutils@HIDDEN; Mon, 31 May 2010 07:03:42 -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 1OJ2mQ-0007ZI-Kp
for bug-coreutils@HIDDEN; Mon, 31 May 2010 07:03:40 -0400
Received: from smtp1-g21.free.fr ([212.27.42.1]:38300)
by eggs.gnu.org with esmtp (Exim 4.69)
(envelope-from <meyering@HIDDEN>) id 1OJ2mP-0007Yg-Uc
for bug-coreutils@HIDDEN; Mon, 31 May 2010 07:03:38 -0400
Received: from mx.meyering.net (mx.meyering.net [82.230.74.64])
by smtp1-g21.free.fr (Postfix) with ESMTP id B52E594013D
for <bug-coreutils@HIDDEN>; Mon, 31 May 2010 13:03:31 +0200 (CEST)
Received: by rho.meyering.net (Acme Bit-Twister, from userid 1000)
id 75F03F54; Mon, 31 May 2010 13:03:30 +0200 (CEST)
From: Jim Meyering <jim@HIDDEN>
To: bug-coreutils@HIDDEN
Subject: reindenting with uncrustify, maybe...
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
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: -4.0 (----)
X-Debbugs-Envelope-To: submit
Cc: uncrustify-developer@HIDDEN
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 (-----)
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
Jim Meyering <jim@HIDDEN>:bug-coreutils@HIDDEN.
Full text available.owner <at> debbugs.gnu.org, bug-coreutils@HIDDEN:bug#6318; Package coreutils.
Full text available.
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997 nCipher Corporation Ltd,
1994-97 Ian Jackson.