On 02/09/2013 08:45 PM, Paul Eggert wrote:
On 02/09/2013 11:26 AM, Pádraig Brady wrote:
These are function names and not to be translated.
These are "developer debug" strings, which I'm tempted to remove entirely.
Certainly "simple_strtod_human:\n" should not be translated,
as it's referring to the identifier.
Since the source code is written in English, we can safely
assume developers read English, so there's no point to translating
messages intended only for developers.
Both factor and numfmt recently introduced devel debug messages,
enabled by --verbose and ---devdebug respectively.
I've cleaned this up in the attached so that both use the same
method to output the messages, and both now enable these messages
with the the same ---debug option.
Also translations are removed from these messages,
and a syntax check added to stop future translations sneaking in in future.
thanks,
Pádraig.
>From d64515e1c198fb58ff40116b58912e76f3a177e5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?P=C3=A1draig=20Brady?= <p...@draigbrady.com>
Date: Sun, 10 Feb 2013 12:47:23 +0000
Subject: [PATCH] maint: consolidate developer debug messages
Both factor and numfmt recently introduced debug messages
for developers enabled by --verbose and ---devdebug respectively.
There were a few issues though:
1. They used different mechanisms to enable these messages.
2. factor used --verbose which might be needed for something else
3. They used different methods to output the messages,
and numfmt used error() which added an unwanted newline
4. numfmt marked all these messages for translation and factor
marked a couple. We really don't need these translated.
So we fix the above issues here while renaming the enabling
option for both commands to ---debug (still undocumented).
* src/factor.c (verbose): Rename to dev_debug and change from int to
bool as it's just a toggle flag.
(long_options): Rename --verbose to ---debug.
* src/system.h (devmsg): A new inline function to output a message
if enabled by a global dev_debug variable in the compilation unit.
* src/numfmt.c: Use devmsg() rather than error().
Also remove the translation tags from these messages.
Also change debug flag to bool from int.
* tests/misc/numfmt.pl: Adjust for the ---devdebug to ---debug change.
* cfg.mk (sc_marked_devdiagnostics): Add a syntax check to ensure
translations are not added to devmsg calls.
---
cfg.mk | 7 +++
src/factor.c | 37 ++++++-----------
src/numfmt.c | 104 ++++++++++++++++++--------------------------------
src/system.h | 15 +++++++
tests/misc/numfmt.pl | 22 +++++-----
5 files changed, 83 insertions(+), 102 deletions(-)
diff --git a/cfg.mk b/cfg.mk
index fbc64b4..09858a1 100644
--- a/cfg.mk
+++ b/cfg.mk
@@ -521,6 +521,13 @@ sc_THANKS_in_duplicates:
&& { echo '$(ME): remove the above names from THANKS.in' \
1>&2; exit 1; } || :
+# Look for developer diagnostics that are marked for translation.
+# This won't find any for which devmsg's format string is on a separate line.
+sc_marked_devdiagnostics:
+ @prohibit='\<devmsg *\(.*_\(' \
+ halt='found marked developer diagnostic(s)' \
+ $(_sc_search_regexp)
+
# Override the default Cc: used in generating an announcement.
announcement_Cc_ = $(translation_project_), \
coreut...@gnu.org, coreutils-annou...@gnu.org
diff --git a/src/factor.c b/src/factor.c
index 95451a5..df3d7a0 100644
--- a/src/factor.c
+++ b/src/factor.c
@@ -216,12 +216,12 @@ static enum alg_type alg;
enum
{
- VERBOSE_OPTION = CHAR_MAX + 1
+ DEV_DEBUG_OPTION = CHAR_MAX + 1
};
static struct option const long_options[] =
{
- {"verbose", no_argument, NULL, VERBOSE_OPTION},
+ {"-debug", no_argument, NULL, DEV_DEBUG_OPTION},
{GETOPT_HELP_OPTION_DECL},
{GETOPT_VERSION_OPTION_DECL},
{NULL, 0, NULL, 0}
@@ -685,8 +685,9 @@ static const struct primes_dtab primes_dtab[] = {
the integers used to generate primes.h. */
verify (W <= WIDE_UINT_BITS);
-/* This flag is honored only in the GMP code. */
-static int verbose = 0;
+/* debugging for developers. Enables devmsg().
+ This flag is used only in the GMP code. */
+bool dev_debug = false;
/* Prove primality or run probabilistic tests. */
static bool flag_prove_primality = true;
@@ -703,18 +704,6 @@ static bool flag_prove_primality = true;
#endif
static void
-debug (char const *fmt, ...)
-{
- if (verbose)
- {
- va_list ap;
- va_start (ap, fmt);
- vfprintf (stderr, fmt, ap);
- va_end (ap);
- }
-}
-
-static void
factor_insert_refind (struct factors *factors, uintmax_t p, unsigned int i,
unsigned int off)
{
@@ -844,7 +833,7 @@ mp_factor_using_division (mpz_t t, struct mp_factors *factors)
mpz_t q;
unsigned long int p;
- debug ("[trial division] ");
+ devmsg ("[trial division] ");
mpz_init (q);
@@ -1665,7 +1654,7 @@ mp_factor_using_pollard_rho (mpz_t n, unsigned long int a,
mpz_t x, z, y, P;
mpz_t t, t2;
- debug ("[pollard-rho (%lu)] ", a);
+ devmsg ("[pollard-rho (%lu)] ", a);
mpz_inits (t, t2, NULL);
mpz_init_set_si (y, 2);
@@ -1728,7 +1717,7 @@ mp_factor_using_pollard_rho (mpz_t n, unsigned long int a,
if (!mp_prime_p (t))
{
- debug ("[composite factor--restarting pollard-rho] ");
+ devmsg ("[composite factor--restarting pollard-rho] ");
mp_factor_using_pollard_rho (t, a + 1, factors);
}
else
@@ -2247,7 +2236,7 @@ mp_factor (mpz_t t, struct mp_factors *factors)
if (mpz_cmp_ui (t, 1) != 0)
{
- debug ("[is number prime?] ");
+ devmsg ("[is number prime?] ");
if (mp_prime_p (t))
mp_factor_insert (factors, t);
else
@@ -2400,7 +2389,7 @@ print_factors (const char *input)
case LONGINT_OK:
if (((t1 << 1) >> 1) == t1)
{
- debug ("[%s]", _("using single-precision arithmetic"));
+ devmsg ("[using single-precision arithmetic] ");
print_factors_single (t1, t0);
return true;
}
@@ -2416,7 +2405,7 @@ print_factors (const char *input)
}
#if HAVE_GMP
- debug ("[%s]", _("using arbitrary-precision arithmetic"));
+ devmsg ("[using arbitrary-precision arithmetic] ");
mpz_t t;
struct mp_factors factors;
@@ -2502,8 +2491,8 @@ main (int argc, char **argv)
{
switch (c)
{
- case VERBOSE_OPTION:
- verbose = 1;
+ case DEV_DEBUG_OPTION:
+ dev_debug = true;
break;
case 's':
diff --git a/src/numfmt.c b/src/numfmt.c
index e42cbf8..f7c8e5e 100644
--- a/src/numfmt.c
+++ b/src/numfmt.c
@@ -136,7 +136,7 @@ static struct option const longopts[] =
{"delimiter", required_argument, NULL, 'd'},
{"field", required_argument, NULL, FIELD_OPTION},
{"debug", no_argument, NULL, DEBUG_OPTION},
- {"-devdebug", no_argument, NULL, DEV_DEBUG_OPTION},
+ {"-debug", no_argument, NULL, DEV_DEBUG_OPTION},
{"header", optional_argument, NULL, HEADER_OPTION},
{"format", required_argument, NULL, FORMAT_OPTION},
{"invalid", required_argument, NULL, INVALID_OPTION},
@@ -188,10 +188,10 @@ static uintmax_t header = 0;
/* Debug for users: print warnings to STDERR about possible
error (similar to sort's debug). */
-static int debug = 0;
+static bool debug;
-/* debugging for developers - to be removed in final version? */
-static int dev_debug = 0;
+/* debugging for developers. Enables devmsg(). */
+bool dev_debug = false;
/* will be set according to the current locale. */
static const char *decimal_point;
@@ -583,18 +583,16 @@ simple_strtod_human (const char *input_str,
/* 'scale_auto' is checked below. */
int scale_base = default_scale_base (allowed_scaling);
- if (dev_debug)
- error (0, 0, _("simple_strtod_human:\n input string: '%s'\n "
- "locale decimal-point: '%s'\n"), input_str, decimal_point);
+ devmsg ("simple_strtod_human:\n input string: '%s'\n "
+ "locale decimal-point: '%s'\n", input_str, decimal_point);
enum simple_strtod_error e =
simple_strtod_float (input_str, endptr, value, precision);
if (e != SSE_OK && e != SSE_OK_PRECISION_LOSS)
return e;
- if (dev_debug)
- error (0, 0, _(" parsed numeric value: %Lf\n"
- " input precision = %d\n"), *value, (int)*precision);
+ devmsg (" parsed numeric value: %Lf\n"
+ " input precision = %d\n", *value, (int)*precision);
if (**endptr != '\0')
{
@@ -619,9 +617,8 @@ simple_strtod_human (const char *input_str,
is followed by an 'i' (e.g. Ki, Mi, Gi). */
scale_base = 1024;
(*endptr)++; /* skip second ('i') suffix character. */
- if (dev_debug)
- error (0, 0, _(" Auto-scaling, found 'i', switching to base %d\n"),
- scale_base);
+ devmsg (" Auto-scaling, found 'i', switching to base %d\n",
+ scale_base);
}
*precision = 0; /* Reset, to select precision based on scale. */
@@ -637,15 +634,12 @@ simple_strtod_human (const char *input_str,
long double multiplier = powerld (scale_base, power);
- if (dev_debug)
- error (0, 0, _(" suffix power=%d^%d = %Lf\n"),
- scale_base, power, multiplier);
+ devmsg (" suffix power=%d^%d = %Lf\n", scale_base, power, multiplier);
/* TODO: detect loss of precision and overflows. */
(*value) = (*value) * multiplier;
- if (dev_debug)
- error (0, 0, _(" returning value: %Lf (%LG)\n"), *value, *value);
+ devmsg (" returning value: %Lf (%LG)\n", *value, *value);
return e;
}
@@ -695,8 +689,7 @@ double_to_human (long double val, int precision,
char *buf, size_t buf_size,
enum scale_type scale, int group, enum round_type round)
{
- if (dev_debug)
- error (0, 0, _("double_to_human:\n"));
+ devmsg ("double_to_human:\n");
if (scale == scale_none)
{
@@ -704,11 +697,9 @@ double_to_human (long double val, int precision,
val = simple_round (val, round);
val /= powerld (10, precision);
- if (dev_debug)
- error (0, 0,
- (group) ?
- _(" no scaling, returning (grouped) value: %'.*Lf\n") :
- _(" no scaling, returning value: %.*Lf\n"), precision, val);
+ devmsg ((group) ?
+ " no scaling, returning (grouped) value: %'.*Lf\n" :
+ " no scaling, returning value: %.*Lf\n", precision, val);
int i = snprintf (buf, buf_size, (group) ? "%'.*Lf" : "%.*Lf",
precision, val);
@@ -724,9 +715,7 @@ double_to_human (long double val, int precision,
/* Normalize val to scale. */
unsigned int power = 0;
val = expld (val, scale_base, &power);
- if (dev_debug)
- error (0, 0, _(" scaled value to %Lf * %0.f ^ %d\n"),
- val, scale_base, power);
+ devmsg (" scaled value to %Lf * %0.f ^ %d\n", val, scale_base, power);
/* Perform rounding. */
int ten_or_less = 0;
@@ -754,9 +743,7 @@ double_to_human (long double val, int precision,
int show_decimal_point = (val != 0) && (absld (val) < 10) && (power > 0);
/* && (absld (val) > simple_round_floor (val))) */
- if (dev_debug)
- error (0, 0, _(" after rounding, value=%Lf * %0.f ^ %d\n"),
- val, scale_base, power);
+ devmsg (" after rounding, value=%Lf * %0.f ^ %d\n", val, scale_base, power);
snprintf (buf, buf_size, (show_decimal_point) ? "%.1Lf%s" : "%.0Lf%s",
val, suffix_power_character (power));
@@ -764,8 +751,7 @@ double_to_human (long double val, int precision,
if (scale == scale_IEC_I && power > 0)
strncat (buf, "i", buf_size - strlen (buf) - 1);
- if (dev_debug)
- error (0, 0, _(" returning value: '%s'\n"), buf);
+ devmsg (" returning value: '%s'\n", buf);
return;
}
@@ -1012,14 +998,13 @@ parse_format_string (char const *fmt)
strlen (fmt + suffix_pos));
}
- if (dev_debug)
- error (0, 0, _("format String:\n input: %s\n grouping: %s\n"
+ devmsg ("format String:\n input: %s\n grouping: %s\n"
" padding width: %ld\n alignment: %s\n"
- " prefix: '%s'\n suffix: '%s'\n"),
- quote (fmt), (grouping) ? "yes" : "no",
- padding_width,
- (padding_alignment == MBS_ALIGN_LEFT) ? "Left" : "Right",
- format_str_prefix, format_str_suffix);
+ " prefix: '%s'\n suffix: '%s'\n",
+ quote (fmt), (grouping) ? "yes" : "no",
+ padding_width,
+ (padding_alignment == MBS_ALIGN_LEFT) ? "Left" : "Right",
+ format_str_prefix, format_str_suffix);
}
/* Parse a numeric value (with optional suffix) from a string.
@@ -1087,10 +1072,7 @@ prepare_padded_number (const long double val, size_t precision)
if (suffix)
strncat (buf, suffix, sizeof (buf) - strlen (buf) -1);
- if (dev_debug)
- error (0, 0, _("formatting output:\n value: %Lf\n humanized: '%s'\n"),
- val, buf);
-
+ devmsg ("formatting output:\n value: %Lf\n humanized: '%s'\n", val, buf);
if (padding_width && strlen (buf) < padding_width)
{
@@ -1098,9 +1080,7 @@ prepare_padded_number (const long double val, size_t precision)
mbsalign (buf, padding_buffer, padding_buffer_size, &w,
padding_alignment, MBA_UNIBYTE_ONLY);
- if (dev_debug)
- error (0, 0, _(" After padding: '%s'\n"), padding_buffer);
-
+ devmsg (" After padding: '%s'\n", padding_buffer);
}
else
{
@@ -1136,14 +1116,10 @@ process_suffixed_number (char *text, long double *result, size_t *precision)
{
/* trim suffix, ONLY if it's at the end of the text. */
*possible_suffix = '\0';
- if (dev_debug)
- error (0, 0, _("trimming suffix '%s'\n"), suffix);
+ devmsg ("trimming suffix '%s'\n", suffix);
}
else
- {
- if (dev_debug)
- error (0, 0, _("no valid suffix found\n"));
- }
+ devmsg ("no valid suffix found\n");
}
/* Skip white space - always. */
@@ -1164,9 +1140,7 @@ process_suffixed_number (char *text, long double *result, size_t *precision)
{
padding_width = 0;
}
- if (dev_debug)
- error (0, 0, _("setting Auto-Padding to %ld characters\n"),
- padding_width);
+ devmsg ("setting Auto-Padding to %ld characters\n", padding_width);
}
long double val = 0;
@@ -1233,9 +1207,7 @@ extract_fields (char *line, int _field,
*_data = NULL;
*_suffix = NULL;
- if (dev_debug)
- error (0, 0, _("extracting Fields:\n input: '%s'\n field: %d\n"),
- line, _field);
+ devmsg ("extracting Fields:\n input: '%s'\n field: %d\n", line, _field);
if (field > 1)
{
@@ -1245,8 +1217,7 @@ extract_fields (char *line, int _field,
if (*ptr == '\0')
{
/* not enough fields in the input - print warning? */
- if (dev_debug)
- error (0, 0, _(" TOO FEW FIELDS!\n prefix: '%s'\n"), *_prefix);
+ devmsg (" TOO FEW FIELDS!\n prefix: '%s'\n", *_prefix);
return;
}
@@ -1266,9 +1237,8 @@ extract_fields (char *line, int _field,
else
*_suffix = NULL;
- if (dev_debug)
- error (0, 0, _(" prefix: '%s'\n number: '%s'\n suffix: '%s'\n"),
- *_prefix, *_data, *_suffix);
+ devmsg (" prefix: '%s'\n number: '%s'\n suffix: '%s'\n",
+ *_prefix, *_data, *_suffix);
}
@@ -1409,12 +1379,12 @@ main (int argc, char **argv)
break;
case DEBUG_OPTION:
- debug = 1;
+ debug = true;
break;
case DEV_DEBUG_OPTION:
- dev_debug = 1;
- debug = 1;
+ dev_debug = true;
+ debug = true;
break;
case HEADER_OPTION:
diff --git a/src/system.h b/src/system.h
index 1677999..6c310ad 100644
--- a/src/system.h
+++ b/src/system.h
@@ -649,6 +649,21 @@ stzncpy (char *restrict dest, char const *restrict src, size_t len)
return dest;
}
+/* Like error(0, 0, ...), but without an implicit newline.
+ Also a noop unless the global DEV_DEBUG is set. */
+extern bool dev_debug;
+static inline void
+devmsg (char const *fmt, ...)
+{
+ if (dev_debug)
+ {
+ va_list ap;
+ va_start (ap, fmt);
+ vfprintf (stderr, fmt, ap);
+ va_end (ap);
+ }
+}
+
#ifndef ARRAY_CARDINALITY
# define ARRAY_CARDINALITY(Array) (sizeof (Array) / sizeof *(Array))
#endif
diff --git a/tests/misc/numfmt.pl b/tests/misc/numfmt.pl
index b46e4d5..61917fb 100644
--- a/tests/misc/numfmt.pl
+++ b/tests/misc/numfmt.pl
@@ -643,37 +643,37 @@ my @Tests =
# dev-debug messages - the actual messages don't matter
# just ensure the program works, and for code coverage testing.
- ['devdebug-1', '---devdebug --from=si 4.9K', {OUT=>"4900"},
+ ['devdebug-1', '---debug --from=si 4.9K', {OUT=>"4900"},
{ERR=>""},
{ERR_SUBST=>"s/.*//msg"}],
- ['devdebug-2', '---devdebug 4900', {OUT=>"4900"},
+ ['devdebug-2', '---debug 4900', {OUT=>"4900"},
{ERR=>""},
{ERR_SUBST=>"s/.*//msg"}],
- ['devdebug-3', '---devdebug --from=auto 4Mi', {OUT=>"4194304"},
+ ['devdebug-3', '---debug --from=auto 4Mi', {OUT=>"4194304"},
{ERR=>""},
{ERR_SUBST=>"s/.*//msg"}],
- ['devdebug-4', '---devdebug --to=si 4000000', {OUT=>"4.0M"},
+ ['devdebug-4', '---debug --to=si 4000000', {OUT=>"4.0M"},
{ERR=>""},
{ERR_SUBST=>"s/.*//msg"}],
- ['devdebug-5', '---devdebug --to=si --padding=5 4000000', {OUT=>" 4.0M"},
+ ['devdebug-5', '---debug --to=si --padding=5 4000000', {OUT=>" 4.0M"},
{ERR=>""},
{ERR_SUBST=>"s/.*//msg"}],
- ['devdebug-6', '---devdebug --suffix=Foo 1234Foo', {OUT=>"1234Foo"},
+ ['devdebug-6', '---debug --suffix=Foo 1234Foo', {OUT=>"1234Foo"},
{ERR=>""},
{ERR_SUBST=>"s/.*//msg"}],
- ['devdebug-7', '---devdebug --suffix=Foo 1234', {OUT=>"1234Foo"},
+ ['devdebug-7', '---debug --suffix=Foo 1234', {OUT=>"1234Foo"},
{ERR=>""},
{ERR_SUBST=>"s/.*//msg"}],
- ['devdebug-9', '---devdebug --grouping 10000', {OUT=>"10000"},
+ ['devdebug-9', '---debug --grouping 10000', {OUT=>"10000"},
{ERR=>""},
{ERR_SUBST=>"s/.*//msg"}],
- ['devdebug-10', '---devdebug --format %f 10000', {OUT=>"10000"},
+ ['devdebug-10', '---debug --format %f 10000', {OUT=>"10000"},
{ERR=>""},
{ERR_SUBST=>"s/.*//msg"}],
- ['devdebug-11', '---devdebug --format "%\'-10f" 10000',{OUT=>"10000 "},
+ ['devdebug-11', '---debug --format "%\'-10f" 10000',{OUT=>"10000 "},
{ERR=>""},
{ERR_SUBST=>"s/.*//msg"}],
- ['devdebug-12', '---devdebug --field 2 A',{OUT=>""},
+ ['devdebug-12', '---debug --field 2 A',{OUT=>""},
{ERR=>""}, {EXIT=>2},
{ERR_SUBST=>"s/.*//msg"}],
--
1.7.7.6
