[PATCHES] various commenting, whitespace changes

[PATCHES] various commenting, whitespace changes

[Thien-Thi Nguyen]

[-- Attachment #1: Type: text/plain, Size: 246 bytes --]

If you examine branch ‘ttn/janitor’, you will see these commits,
each preceding the "actual change".  I lump them together because
they aren't very interesting by themselves.

thi

_____________________________________________________

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0003-Comment-whitespace-munging-nfc.patch --]
[-- Type: text/x-diff, Size: 4621 bytes --]

From 9c6b71e9b3334439ae6348b38412ae6a8fb154ad Mon Sep 17 00:00:00 2001
From: Thien-Thi Nguyen <ttn@gnuvola.org>
Date: Fri, 25 Jun 2010 11:12:21 +0200
Subject: [PATCH 3/8] Comment, whitespace munging; nfc.

* libguile/guile-func-name-check: Add comments; refill; kill eol whitespace.
---
 libguile/guile-func-name-check |   81 +++++++++++++++++++++++----------------
 1 files changed, 48 insertions(+), 33 deletions(-)

diff --git a/libguile/guile-func-name-check b/libguile/guile-func-name-check
index 8b4924e..f00b522 100644
--- a/libguile/guile-func-name-check
+++ b/libguile/guile-func-name-check
@@ -1,65 +1,80 @@
 #!/usr/bin/awk -f
 #
-#  Copyright (C) 2000, 2001, 2006 Free Software Foundation, Inc.
-# 
+# guile-func-name-check
+#
+# Copyright (C) 2000, 2001, 2006 Free Software Foundation, Inc.
+#
 # This program is free software; you can redistribute it and/or modify
 # it under the terms of the GNU Lesser General Public License as
 # published by the Free Software Foundation; either version 3, or (at
 # your option) any later version.
-# 
+#
 # This program is distributed in the hope that it will be useful, but
 # WITHOUT ANY WARRANTY; without even the implied warranty of
 # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
 # Lesser General Public License for more details.
-# 
+#
 # You should have received a copy of the GNU Lesser General Public
 # License along with this software; see the file COPYING.LESSER.  If
 # not, write to the Free Software Foundation, Inc., 51 Franklin
 # Street, Fifth Floor, Boston, MA 02110-1301 USA
 #
-# Written by Greg J. Badros, <gjb@cs.washington.edu>
-# 11-Jan-2000
+# Author: Greg J. Badros, <gjb@cs.washington.edu>
 
 BEGIN {
-  filename = ARGV[1];
-  in_a_func = 0;
+    filename = ARGV[1];
+    in_a_func = 0;
 }
 
-/^SCM_DEFINE/ { 
-  func_name = $0;
-  sub(/^[^\(\n]*\([ \t]*/,"", func_name);
-  sub(/[ \t]*,.*/,"", func_name);
-#  print func_name;  # GJB:FIXME:: flag to do this to list primitives?
-  in_a_func = 1;
+# Extract the function name from "SCM_DEFINE (foo, ...".
+# FIXME: This loses if the open paren is on the next line.
+/^SCM_DEFINE/ {
+    func_name = $0;
+    sub (/^[^\(\n]*\([ \t]*/, "", func_name);
+    sub (/[ \t]*,.*/, "", func_name);
+    in_a_func = 1;
 }
 
+# Check that for "SCM_DEFINE (foo, ...)", we see:
+#   #define FUNC_NAME s_foo
+#   {
+# FIXME: This loses for C string-literal (#define FUNC_NAME "foo").
+# FIXME: This loses if #define is inside the curly brace.
 /^\{/ && in_a_func {
-  if (!match(last_line,/^#define[ \t]+FUNC_NAME[ \t]+/)) {
-    printf filename ":" NR ":***" > "/dev/stderr";
-    print "Missing or erroneous `#define FUNC_NAME s_" func_name "'" > "/dev/stderr";
-  } else {
-    sub(/^#define[ \t]+FUNC_NAME[ \t]+s_/, "", last_line);
-    sub(/[ \t]*$/,"",last_line);
-    if (last_line != func_name) {
-      printf filename ":" NR ":***" > "/dev/stderr";
-      print "Mismatching FUNC_NAME.  Should be: `#define FUNC_NAME s_" func_name "'" > "/dev/stderr";
+    if (!match (last_line, /^#define[ \t]+FUNC_NAME[ \t]+/)) {
+        printf filename ":" NR ":***" > "/dev/stderr";
+        print "Missing or erroneous `#define FUNC_NAME s_" \
+            func_name "'" > "/dev/stderr";
+    } else {
+        sub (/^#define[ \t]+FUNC_NAME[ \t]+s_/, "", last_line);
+        sub (/[ \t]*$/, "", last_line);
+        if (last_line != func_name) {
+            printf filename ":" NR ":***" > "/dev/stderr";
+            print "Mismatching FUNC_NAME.  Should be: " \
+                "`#define FUNC_NAME s_" func_name "'" > "/dev/stderr";
+        }
     }
-  }
 }
 
+# If previous line closed the function, check that we see "#undef FUNC_NAME".
+# FIXME: This loses if #undef is inside the curly brace.
 1 == next_line_better_be_undef {
-  if (!match($0,/^#undef FUNC_NAME[ \t]*$/)) {
-    printf filename ":" NR ":***" > "/dev/stderr";
-    print "Missing or erroneous #undef for " func_name ": "
-          "Got `" $0 "' instead." > "/dev/stderr";
-  }
-  in_a_func = "";
-  func_name = "";
-  next_line_better_be_undef = 0;
+    if (!match ($0, /^#undef FUNC_NAME[ \t]*$/)) {
+        printf filename ":" NR ":***" > "/dev/stderr";
+        print "Missing or erroneous #undef for " func_name ": " \
+            "Got `" $0 "' instead." > "/dev/stderr";
+    }
+    in_a_func = "";
+    func_name = "";
+    next_line_better_be_undef = 0;
 }
 
+# Note function closing.
 /^\}/ && in_a_func {
-  next_line_better_be_undef = 1;
+    next_line_better_be_undef = 1;
 }
 
+# Remember this line for the next cycle.
 { last_line = $0; }
+
+# guile-func-name-check ends here
-- 
1.6.3.2


[-- Attachment #3: Type: text/plain, Size: 54 bytes --]

_____________________________________________________

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #4: 0005-Comment-whitespace-munging-nfc.patch --]
[-- Type: text/x-diff, Size: 2497 bytes --]

From 408b58584050c8568c0cc82174b3b130527b4611 Mon Sep 17 00:00:00 2001
From: Thien-Thi Nguyen <ttn@gnuvola.org>
Date: Fri, 25 Jun 2010 12:09:39 +0200
Subject: [PATCH 5/8] Comment, whitespace munging; nfc.

* libguile/pairs.c: Add space after sentence end; kill eol
whitespace; selectively untabify; add "ends here" comment.
---
 libguile/pairs.c |   15 +++++++++------
 1 files changed, 9 insertions(+), 6 deletions(-)

diff --git a/libguile/pairs.c b/libguile/pairs.c
index 68fa4c9..da0d7b9 100644
--- a/libguile/pairs.c
+++ b/libguile/pairs.c
@@ -1,5 +1,6 @@
-/* Copyright (C) 1995,1996,2000,2001, 2004, 2005, 2006, 2008, 2009 Free Software Foundation, Inc.
- * 
+/* Copyright (C) 1995, 1996, 2000, 2001, 2004, 2005, 2006, 2008,
+ *   2009 Free Software Foundation, Inc.
+ *
  * This library is free software; you can redistribute it and/or
  * modify it under the terms of the GNU Lesser General Public License
  * as published by the Free Software Foundation; either version 3 of
@@ -79,14 +80,14 @@ SCM_DEFINE (scm_cons, "cons", 2, 0, 0,
 #undef FUNC_NAME
 
 
-SCM 
+SCM
 scm_cons2 (SCM w, SCM x, SCM y)
 {
   return scm_cons (w, scm_cons (x, y));
 }
 
 
-SCM_DEFINE (scm_pair_p, "pair?", 1, 0, 0, 
+SCM_DEFINE (scm_pair_p, "pair?", 1, 0, 0,
             (SCM x),
 	    "Return @code{#t} if @var{x} is a pair; otherwise return\n"
 	    "@code{#f}.")
@@ -129,13 +130,13 @@ SCM_DEFINE (scm_set_cdr_x, "set-cdr!", 2, 0, 0,
  * two bits is only needed to indicate when cxr-ing is ready.  This is the
  * case, when all remaining pairs of bits equal 00.  */
 
-/* The compiler should unroll this. */
+/* The compiler should unroll this.  */
 #define CHASE_PAIRS(tree, FUNC_NAME, pattern)                           \
   scm_t_uint32 pattern_var = pattern;                                   \
   do                                                                    \
     {                                                                   \
       if (!scm_is_pair (tree))                                          \
-	scm_wrong_type_arg_msg (FUNC_NAME, 0, tree, "pair");            \
+        scm_wrong_type_arg_msg (FUNC_NAME, 0, tree, "pair");            \
       tree = (pattern_var & 1) ? SCM_CAR (tree) : SCM_CDR (tree);       \
       pattern_var >>= 2;                                                \
     }                                                                   \
@@ -278,3 +279,5 @@ scm_init_pairs ()
   c-file-style: "gnu"
   End:
 */
+
+/* pairs.c ends here */
-- 
1.6.3.2


[-- Attachment #5: Type: text/plain, Size: 54 bytes --]

_____________________________________________________

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #6: 0007-Add-copyright-notice-to-acinclude.m4.patch --]
[-- Type: text/x-diff, Size: 1783 bytes --]

From db33d0b49a746810a36bc73b43cf8180c1c9d0c2 Mon Sep 17 00:00:00 2001
From: Thien-Thi Nguyen <ttn@gnuvola.org>
Date: Fri, 25 Jun 2010 13:28:09 +0200
Subject: [PATCH 7/8] Add copyright notice to acinclude.m4.

* acinclude.m4: Add copyright notice, with years derived from "git log" output.
---
 acinclude.m4 |   23 +++++++++++++++++++++++
 1 files changed, 23 insertions(+), 0 deletions(-)

diff --git a/acinclude.m4 b/acinclude.m4
index 8cfe1d4..ec42743 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -1,5 +1,26 @@
 dnl -*- Autoconf -*-
 
+dnl Copyright (C) 1997, 1999, 2000, 2001, 2002, 2004, 2006,
+dnl   2007, 2008, 2009, 2010 Free Software Foundation, Inc.
+dnl
+dnl This file is part of GUILE
+dnl
+dnl GUILE is free software; you can redistribute it and/or modify it under
+dnl the terms of the GNU Lesser General Public License as published by the
+dnl Free Software Foundation; either version 3, or (at your option) any
+dnl later version.
+dnl
+dnl GUILE is distributed in the hope that it will be useful, but WITHOUT
+dnl ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+dnl FITNESS FOR A PARTICULAR PURPOSE.  See the GNU Lesser General Public
+dnl License for more details.
+dnl
+dnl You should have received a copy of the GNU Lesser General Public
+dnl License along with GUILE; see the file COPYING.LESSER.  If not, write
+dnl to the Free Software Foundation, Inc., 51 Franklin Street, Fifth
+dnl Floor, Boston, MA 02110-1301, USA.
+
+
 dnl  On the NeXT, #including <utime.h> doesn't give you a definition for
 dnl  struct utime, unless you #define _POSIX_SOURCE.
 
@@ -439,3 +460,5 @@ AC_DEFUN([GUILE_READLINE], [
   AC_SUBST(LIBGUILEREADLINE_INTERFACE_AGE)
   AC_SUBST(LIBGUILEREADLINE_INTERFACE)
 ])
+
+dnl acinclude.m4 ends here
-- 
1.6.3.2

Re: [PATCHES] various commenting, whitespace changes

[Andy Wingo]

Hello,

On Fri 25 Jun 2010 14:16, Thien-Thi Nguyen <ttn@gnuvola.org> writes:

> From 9c6b71e9b3334439ae6348b38412ae6a8fb154ad Mon Sep 17 00:00:00 2001
> Subject: [PATCH 3/8] Comment, whitespace munging; nfc.

What does NFC mean?

In general I'm a little uneasy with this kind of change, as it is
unnecessary. But, OK, I guess. Please feel free to make files more
closely follow GNU conventions, when you are working on those files; but
do avoid a general cleanup. I think it would drive me crazy ;) Perhaps
Ludovic will chime in later with more general feedback.

> --- a/libguile/guile-func-name-check
> +++ b/libguile/guile-func-name-check
[...]
>  BEGIN {
> -  filename = ARGV[1];
> -  in_a_func = 0;
> +    filename = ARGV[1];
> +    in_a_func = 0;
>  }

There is no need to change the indentation amount here; 2-space should
be fine.

[...]
> +# guile-func-name-check ends here

I would rather not have these markers. It's an extra point of bitrot.

If you really want the end-of-file markers I would want some
confirmation from Ludovic; otherwise, I am OK with you pushing these
patches without the EOF markers and without changing the awk script
indentation.

Cheers,

Andy
-- 
http://wingolog.org/

Re: [PATCHES] various commenting, whitespace changes

[Ludovic Courtès]

Hi Guilers!

Andy Wingo <wingo@pobox.com> writes:

> On Fri 25 Jun 2010 14:16, Thien-Thi Nguyen <ttn@gnuvola.org> writes:
>
>> From 9c6b71e9b3334439ae6348b38412ae6a8fb154ad Mon Sep 17 00:00:00 2001
>> Subject: [PATCH 3/8] Comment, whitespace munging; nfc.
>
> What does NFC mean?
>
> In general I'm a little uneasy with this kind of change, as it is
> unnecessary. But, OK, I guess. Please feel free to make files more
> closely follow GNU conventions, when you are working on those files; but
> do avoid a general cleanup. I think it would drive me crazy ;) Perhaps
> Ludovic will chime in later with more general feedback.

I agree with Andy.

>> +# guile-func-name-check ends here
>
> I would rather not have these markers. It's an extra point of bitrot.

+1

Ludo’.

Re: [PATCHES] various commenting, whitespace changes

[Thien-Thi Nguyen]

() Andy Wingo <wingo@pobox.com>
() Mon, 28 Jun 2010 15:44:54 +0200

Sorry for the long delay in response.

   What does NFC mean?

See LibreDWG HACKING (and build-aux/create-changelog), for more info:
git://git.sv.gnu.org/libredwg.git

It's useful to distinguish between "important" and "unimportant"
changes, especially if recording changes (ChangeLog maintenance) is
mechanical.  A simple tag like this gives you the flexibility to make
unimportant changes without the worry that such will end up as noise in
the published ChangeLog.

   In general I'm a little uneasy with this kind of change, as it is
   unnecessary.

No worries, lots of aesthetic/readability munging that i do falls into
that category -- you'll get used to it.  I think if you cut me some
slack on this matter (and perhaps implement "nfc" (or similar) filtering
as per LibreDWG create-changelog), you may find yourself not only more
accepting of these kinds of changes but also doing them yourself.

   But, OK, I guess. Please feel free to make files more closely follow
   GNU conventions, when you are working on those files; but do avoid a
   general cleanup. I think it would drive me crazy ;) Perhaps Ludovic
   will chime in later with more general feedback.

I will avoid general cleanup, but i don't intend to avoid cleanup on
files i wish to hack on (for now, this set includes makefiles, build
scripts and documentation).

I hope this targeted approach will not drive you crazy.  Rather, it
would be ideal if you were to say "hey, thanks janitor ttn for handling
that grunge -- keep up the good work!", after observing that these
changes can coincide peacefully with your own hacking sphere (if you let
that sphere become more transparent :-).

   > --- a/libguile/guile-func-name-check
   > +++ b/libguile/guile-func-name-check
   [...]
   >  BEGIN {
   > -  filename = ARGV[1];
   > -  in_a_func = 0;
   > +    filename = ARGV[1];
   > +    in_a_func = 0;
   >  }

   There is no need to change the indentation amount here; 2-space should
   be fine.

   [...]
   > +# guile-func-name-check ends here

   I would rather not have these markers. It's an extra point of bitrot.

   If you really want the end-of-file markers I would want some
   confirmation from Ludovic; otherwise, I am OK with you pushing these
   patches without the EOF markers and without changing the awk script
   indentation.

I sidestepped these issues, by rewriting in Scheme (and using Emacs'
‘indent-sexp’).  Unfortunately, it looks like hydra is not liking it:

http://hydra.nixos.org/build/606792

Perhaps a workaround would be to use ‘--no-autocompile’ in the
invocation command (libguile/guile-snarf-docs.in:55), but that seems
ugly and throws away a bug-{hunt,squash}ing opportunity.  WDYT?

Another related idea: Since Guile supports many languages, why not AWK?

Re: [PATCHES] various commenting, whitespace changes

[Andy Wingo]

Hi Thien-Thi,

Let me say first that I am really happy to have your energetic,
experienced hand going over Guile sources. Thank you! I do think that
small things are important, and your willingness to look at the small
*and* the big is much appreciated. It's nice to have thoughtful, local
commits.

On Fri 27 Aug 2010 01:29, Thien-Thi Nguyen <ttn@gnuvola.org> writes:

> I will avoid general cleanup, but i don't intend to avoid cleanup on
> files i wish to hack on (for now, this set includes makefiles, build
> scripts and documentation).

Fair enough. Please continue to separate cleanup commits from other
commits.

> I sidestepped [issues in guile-func-name-check], by rewriting in
> Scheme (and using Emacs' ‘indent-sexp’).  Unfortunately, it looks like
> hydra is not liking it:
>
> http://hydra.nixos.org/build/606792
>
> Perhaps a workaround would be to use ‘--no-autocompile’ in the
> invocation command (libguile/guile-snarf-docs.in:55), but that seems
> ugly and throws away a bug-{hunt,squash}ing opportunity.  WDYT?

I think that not autocompiling is the right thing in this case. We don't
actually want to autocompile in Guile when, um, compiling :)

Also in the future, please run clean builds when you change invocations
of guile within libguile/.  Thanks :)

> Another related idea: Since Guile supports many languages, why not AWK?

That would be fun!

Cheers,

Andy
-- 
http://wingolog.org/

Re: [PATCHES] various commenting, whitespace changes

[Andy Wingo]

On Fri 27 Aug 2010 08:16, Andy Wingo <wingo@pobox.com> writes:

> On Fri 27 Aug 2010 01:29, Thien-Thi Nguyen <ttn@gnuvola.org> writes:
>
>> I sidestepped [issues in guile-func-name-check], by rewriting in
>> Scheme (and using Emacs' ‘indent-sexp’).  Unfortunately, it looks like
>> hydra is not liking it:
>>
>> http://hydra.nixos.org/build/606792
>>
>> Perhaps a workaround would be to use ‘--no-autocompile’ in the
>> invocation command (libguile/guile-snarf-docs.in:55), but that seems
>> ugly and throws away a bug-{hunt,squash}ing opportunity.  WDYT?

I went to build a fresh Guile on a machine at work and realized that
this approach is quite slow, because it runs before the rest of Guile is
compiled.

It's much better code, but I think in order to snarf docs in Scheme we
need to do so *after* module/ has been compiled. Do you want to submit a
patch to do that? In the meantime I reverted this commit; but it's not
because the commit was bad code, it just needs to happen at a different
time.

Regards,

Andy
-- 
http://wingolog.org/

Re: [PATCHES] various commenting, whitespace changes

[Thien-Thi Nguyen]

() Andy Wingo <wingo@pobox.com>
() Fri, 27 Aug 2010 13:15:32 -0700

   I went to build a fresh Guile on a machine at work and realized that
   this approach is quite slow, because it runs before the rest of Guile is
   compiled.

   It's much better code, but I think in order to snarf docs in Scheme we
   need to do so *after* module/ has been compiled. Do you want to submit a
   patch to do that? In the meantime I reverted this commit; but it's not
   because the commit was bad code, it just needs to happen at a different
   time.

Thanks for cleaning up my goof.

I agree moving that snarfing later is the best way and will work on a patch
towards that end.