coreutils
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[PATCH] maint: adjust formatting of certain continued strings


From: Jim Meyering
Subject: [PATCH] maint: adjust formatting of certain continued strings
Date: Tue, 10 Jan 2012 19:00:47 +0100

In recent git diff output, a continued string foiled the mechanism
by which git determines the name of the containing function for
each hunk.  For example, from the diffs below:

    @@ -759,8 +760,10 @@ this system doesn't provide a %lu-byte integral 
type"), quote (s_orig), size);

"this system ..." is obviously not the name of a function.
It should have printed the function name, like this:

    @@ -759,8 +760,10 @@ decode_one_format (const char *s_orig, const char *s, 
const char **next,

This change fixes most of the offending in-function continued strings
like that one, and in addition adds a new syntax-check rule to ensure
that we don't introduce any more of them.  The only slight kludge
is that I've exempted the ~100 usage functions in src/*.c, since
most of them contain continued strings that would trigger a match
of this new rule.  I have exempted three files, all for doing
approximately the same thing.  Here's the code from od.c:

    #define PRINT_FIELDS(N, T, FMT_STRING, ACTION)              \
    static void                                                 \
    N (size_t fields, size_t blank, void const *block,          \
       char const *FMT_STRING, int width, int pad)              \
    {                                                           \
    ...

The "s" in static and the "N" would trigger a match.
We want to leave those as-is, since they work just fine that way.
There would be no false positive.


>From f2ee48d9cb4406ba6b03e780032d3f308d6c6ffa Mon Sep 17 00:00:00 2001
From: Jim Meyering <address@hidden>
Date: Mon, 9 Jan 2012 22:56:54 +0100
Subject: [PATCH] maint: adjust formatting of certain continued strings

Add a rule to ding any source file that has a continued string
with a word in the first column of the following line.
Those tend to trigger malfunction in tools that try to map an
arbitrary line number to an enclosing function name.  Of course,
very many strings do precisely this, *when they are part of the
usage function*.  So we exempt the body of each usage function.
* src/dircolors.c (main): Separate a long, continued string
into two separately-quoted parts.
* src/od.c (decode_one_format): Likewise.
(decode_one_format, main): Move a space from end of
preceding linen to the beginning of the continued line.
* src/tr.c (unquote, string2_extend, validate): Likewise.
* src/seq.c (main): Split in two and use string concatenation.
* src/stat.c (default_format): Use a mix of techniques.
* cfg.mk (sc_prohibit_continued_string_alpha_in_column_1): New rule.
Exempt three files in src: system.h, od.c, printf.c.
---
 cfg.mk          |   23 +++++++++++++++++++++++
 src/dircolors.c |    4 ++--
 src/od.c        |   15 +++++++++------
 src/seq.c       |    4 ++--
 src/stat.c      |   33 ++++++++++++++-------------------
 src/tr.c        |   20 ++++++++++----------
 6 files changed, 60 insertions(+), 39 deletions(-)

diff --git a/cfg.mk b/cfg.mk
index 9e45daf..ecb2602 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -289,6 +289,26 @@ sc_prohibit_framework_failure:
        halt='use framework_failure_ instead'                           \
          $(_sc_search_regexp)

+# Exempt the contents of any usage function from the following.
+_continued_string_col_1 = \
+s/^usage .*?\n}//ms;/\\\n\w/ and print ("$$ARGV\n"),$$e=1;END{$$e||=0;exit $$e}
+# Ding any source file that has a continued string with an alphabetic in the
+# first column of the following line.  We prohibit them because they usually
+# trigger false positives in tools that try to map an arbitrary line number
+# to the enclosing function name.  Of course, very many strings do precisely
+# this, *when they are part of the usage function*.  That is why we exempt
+# the contents of any function named "usage".
+sc_prohibit_continued_string_alpha_in_column_1:
+       @perl -0777 -ne '$(_continued_string_col_1)' \
+           $$($(VC_LIST_EXCEPT) | grep '\.[ch]$$') \
+         || { echo '$(ME): continued string with word in first column' \
+               1>&2; exit 1; } || :
+# Use this to list offending lines:
+# git ls-files |grep '\.[ch]$' | xargs \
+#   perl -n -0777 -e 's/^usage.*?\n}//ms;/\\\n\w/ and print "$ARGV\n"' \
+#     | xargs grep -A1 '\\$'|grep '\.[ch][:-][_a-zA-Z]'
+
+
 ###########################################################
 _p0 = \([^"'/]\|"\([^\"]\|[\].\)*"\|'\([^\']\|[\].\)*'
 _pre = $(_p0)\|[/][^"'/*]\|[/]"\([^\"]\|[\].\)*"\|[/]'\([^\']\|[\].\)*'\)*
@@ -409,3 +429,6 @@ exclude_file_name_regexp--sc_preprocessor_indentation = \
   ^(gl/lib/rand-isaac\.[ch]|gl/tests/test-rand-isaac\.c)$$
 exclude_file_name_regexp--sc_prohibit_stat_st_blocks = \
   ^(src/system\.h|tests/du/2g)$$
+
+exclude_file_name_regexp--sc_prohibit_continued_string_alpha_in_column_1 = \
+  ^src/(system\.h|od\.c|printf\.c)$$
diff --git a/src/dircolors.c b/src/dircolors.c
index 06c9784..cc68d6f 100644
--- a/src/dircolors.c
+++ b/src/dircolors.c
@@ -437,8 +437,8 @@ main (int argc, char **argv)
   if (print_database && syntax != SHELL_SYNTAX_UNKNOWN)
     {
       error (0, 0,
-             _("the options to output dircolors' internal database and\n\
-to select a shell syntax are mutually exclusive"));
+             _("the options to output dircolors' internal database and\n"
+               "to select a shell syntax are mutually exclusive"));
       usage (EXIT_FAILURE);
     }

diff --git a/src/od.c b/src/od.c
index 68368d9..e5d2d7e 100644
--- a/src/od.c
+++ b/src/od.c
@@ -640,8 +640,9 @@ decode_one_format (const char *s_orig, const char *s, const 
char **next,
               if (MAX_INTEGRAL_TYPE_SIZE < size
                   || integral_type_size[size] == NO_SIZE)
                 {
-                  error (0, 0, _("invalid type string %s;\n\
-this system doesn't provide a %lu-byte integral type"), quote (s_orig), size);
+                  error (0, 0, _("invalid type string %s;\nthis system"
+                                 " doesn't provide a %lu-byte integral type"),
+                         quote (s_orig), size);
                   return false;
                 }
               s = p;
@@ -759,8 +760,10 @@ this system doesn't provide a %lu-byte integral type"), 
quote (s_orig), size);
               if (size > MAX_FP_TYPE_SIZE
                   || fp_type_size[size] == NO_SIZE)
                 {
-                  error (0, 0, _("invalid type string %s;\n\
-this system doesn't provide a %lu-byte floating point type"),
+                  error (0, 0,
+                         _("invalid type string %s;\n"
+                           "this system doesn't provide a %lu-byte"
+                           " floating point type"),
                          quote (s_orig), size);
                   return false;
                 }
@@ -1598,8 +1601,8 @@ main (int argc, char **argv)
               break;
             default:
               error (EXIT_FAILURE, 0,
-                     _("invalid output address radix '%c'; \
-it must be one character from [doxn]"),
+                     _("invalid output address radix '%c';\
+ it must be one character from [doxn]"),
                      optarg[0]);
               break;
             }
diff --git a/src/seq.c b/src/seq.c
index ea901b8..90e9fc1 100644
--- a/src/seq.c
+++ b/src/seq.c
@@ -428,8 +428,8 @@ main (int argc, char **argv)

   if (format_str != NULL && equal_width)
     {
-      error (0, 0, _("\
-format string may not be specified when printing equal width strings"));
+      error (0, 0, _("format string may not be specified"
+                     " when printing equal width strings"));
       usage (EXIT_FAILURE);
     }

diff --git a/src/stat.c b/src/stat.c
index 0da21d7..8254ccc 100644
--- a/src/stat.c
+++ b/src/stat.c
@@ -1236,13 +1236,11 @@ default_format (bool fs, bool terse, bool device)
         {
           /* TRANSLATORS: This string uses format specifiers from
              'stat --help' with --file-system, and NOT from printf.  */
-          format = xstrdup (_("\
-  File: \"%n\"\n\
-    ID: %-8i Namelen: %-7l Type: %T\n\
-Block size: %-10s Fundamental block size: %S\n\
-Blocks: Total: %-10b Free: %-10f Available: %a\n\
-Inodes: Total: %-10c Free: %d\n\
-"));
+          format = xstrdup (_("  File: \"%n\"\n"
+                              "    ID: %-8i Namelen: %-7l Type: %T\n"
+                              "Block size: %-10s Fundamental block size: %S\n"
+                              "Blocks: Total: %-10b Free: %-10f Available: 
%a\n"
+                              "Inodes: Total: %-10c Free: %d\n"));
         }
     }
   else /* ! fs */
@@ -1272,7 +1270,7 @@ Inodes: Total: %-10c Free: %d\n\
               /* TRANSLATORS: This string uses format specifiers from
                  'stat --help' without --file-system, and NOT from printf.  */
               format = xasprintf ("%s%s", format, _("\
-Device: %Dh/%dd\tInode: %-10i  Links: %-5h Device type: %t,%T\n\
+" "Device: %Dh/%dd\tInode: %-10i  Links: %-5h Device type: %t,%T\n\
 "));
             }
           else
@@ -1280,7 +1278,7 @@ Device: %Dh/%dd\tInode: %-10i  Links: %-5h Device type: 
%t,%T\n\
               /* TRANSLATORS: This string uses format specifiers from
                  'stat --help' without --file-system, and NOT from printf.  */
               format = xasprintf ("%s%s", format, _("\
-Device: %Dh/%dd\tInode: %-10i  Links: %h\n\
+" "Device: %Dh/%dd\tInode: %-10i  Links: %h\n\
 "));
             }
           free (temp);
@@ -1289,7 +1287,7 @@ Device: %Dh/%dd\tInode: %-10i  Links: %h\n\
           /* TRANSLATORS: This string uses format specifiers from
              'stat --help' without --file-system, and NOT from printf.  */
           format = xasprintf ("%s%s", format, _("\
-Access: (%04a/%10.10A)  Uid: (%5u/%8U)   Gid: (%5g/%8G)\n\
+" "Access: (%04a/%10.10A)  Uid: (%5u/%8U)   Gid: (%5g/%8G)\n\
 "));
           free (temp);

@@ -1298,21 +1296,18 @@ Access: (%04a/%10.10A)  Uid: (%5u/%8U)   Gid: 
(%5g/%8G)\n\
               temp = format;
               /* TRANSLATORS: This string uses format specifiers from
                  'stat --help' without --file-system, and NOT from printf.  */
-              format = xasprintf ("%s%s", format, _("\
-Context: %C\n\
-"));
+              format = xasprintf ("%s%s", format, _("Context: %C\n"));
               free (temp);
             }

           temp = format;
           /* TRANSLATORS: This string uses format specifiers from
              'stat --help' without --file-system, and NOT from printf.  */
-          format = xasprintf ("%s%s", format, _("\
-Access: %x\n\
-Modify: %y\n\
-Change: %z\n\
- Birth: %w\n\
-"));
+          format = xasprintf ("%s%s", format,
+                              _("Access: %x\n"
+                                "Modify: %y\n"
+                                "Change: %z\n"
+                                "Birth: %w\n"));
           free (temp);
         }
     }
diff --git a/src/tr.c b/src/tr.c
index 4332d9f..329d814 100644
--- a/src/tr.c
+++ b/src/tr.c
@@ -506,8 +506,8 @@ unquote (char const *s, struct E_string *es)
                              lack of clarity as meaning the resulting behavior
                              is undefined, which means we're allowed to issue
                              a warning.  */
-                          error (0, 0, _("warning: the ambiguous octal escape \
-\\%c%c%c is being\n\tinterpreted as the 2-byte sequence \\0%c%c, %c"),
+                          error (0, 0, _("warning: the ambiguous octal escape\
+ \\%c%c%c is being\n\tinterpreted as the 2-byte sequence \\0%c%c, %c"),
                                  s[i], s[i + 1], s[i + 2],
                                  s[i], s[i + 1], s[i + 2]);
                         }
@@ -1417,8 +1417,8 @@ string2_extend (const struct Spec_list *s1, struct 
Spec_list *s2)
          That's not portable however, contradicts POSIX and is dependent
          on your collating sequence.  */
       error (EXIT_FAILURE, 0,
-             _("when translating with string1 longer than string2,\n\
-the latter string must not end with a character class"));
+             _("when translating with string1 longer than string2,\nthe \
+ latter string must not end with a character class"));
       abort (); /* inform gcc that the above use of error never returns. */
       break;

@@ -1495,15 +1495,15 @@ validate (struct Spec_list *s1, struct Spec_list *s2)
           if (s2->has_equiv_class)
             {
               error (EXIT_FAILURE, 0,
-                     _("[=c=] expressions may not appear in string2 \
-when translating"));
+                     _("[=c=] expressions may not appear in string2\
+ when translating"));
             }

           if (s2->has_restricted_char_class)
             {
               error (EXIT_FAILURE, 0,
-                     _("when translating, the only character classes that may \
-appear in\nstring2 are 'upper' and 'lower'"));
+                     _("when translating, the only character classes that may\
+ appear in\nstring2 are 'upper' and 'lower'"));
             }

           validate_case_classes (s1, s2);
@@ -1535,8 +1535,8 @@ appear in\nstring2 are 'upper' and 'lower'"));
         {
           if (s2->n_indefinite_repeats > 0)
             error (EXIT_FAILURE, 0,
-                   _("the [c*] construct may appear in string2 only \
-when translating"));
+                   _("the [c*] construct may appear in string2 only\
+ when translating"));
         }
     }
 }
--
1.7.9.rc0.13.gbee72



reply via email to

[Prev in Thread] Current Thread [Next in Thread]