diagnostics: use libtextstyle for colored output

Bruno Haible released libtextstyle, a library for colored output based on CSS. Let's use it to generate colored diagnostics, provided libtextstyle is available. See https://lists.gnu.org/archive/html/bug-gnulib/2019-01/msg00176.html https://lists.gnu.org/archive/html/bison-patches/2019-02/msg00073.html https://lists.gnu.org/archive/html/bison-patches/2019-02/msg00084.html https://lists.gnu.org/archive/html/bison-patches/2019-03/msg00007.html * bootstrap.conf (gnulib_modules): Use libtextstyle when possible. * data/diagnostics.css: New. * src/complain.c (begin_use_class, end_use_class, flush) (severity_style, complain_init_color): New. Use them. * src/getargs.c (getargs_colors): New. (getargs): Use it. Skip --color and --style. * src/location.h, src/location.c (location_print): Use a style. * tests/bison.in: Force --color=yes when stderr is a tty. * tests/local.at: Disable colors during the test suite. * tests/input.at: Adjust expectations to the extra options passed on the command line.
2026-03-09 12:23:04 +00:00 · 2019-02-14 06:49:29 +01:00
parent 855fbf1c11
commit f6e38d7ac9
13 changed files with 203 additions and 25 deletions
--- a/bootstrap.conf
+++ b/bootstrap.conf
@@ -28,6 +28,7 @@ gnulib_modules='
  gpl-3.0 hash inttypes isnan javacomp-script
  javaexec-script
  ldexpl
+  libtextstyle
  malloc-gnu
  mbswidth
  non-recursive-gnulib-prefix-hack
--- a/data/diagnostics.css
+++ b/data/diagnostics.css
@@ -0,0 +1,3 @@
+.warning   { color : purple; }
+.error     { color : red; }
+.note      { color : cyan; }
--- a/data/local.mk
+++ b/data/local.mk
@@ -15,7 +15,8 @@
 ## along with this program.  If not, see <http://www.gnu.org/licenses/>.

 dist_pkgdata_DATA =                             \
-  data/README.md
+  data/README.md                                \
+  data/diagnostics.css

 skeletonsdir = $(pkgdatadir)/skeletons
 dist_skeletons_DATA =                           \
--- a/m4/.gitignore
+++ b/m4/.gitignore
@@ -195,3 +195,6 @@
 /readlink.m4
 /relocatable-lib.m4
 /relocatable.m4
+/libtextstyle.m4
+/rename.m4
+/rmdir.m4
--- a/src/complain.c
+++ b/src/complain.c
@@ -25,6 +25,11 @@
 #include <argmatch.h>
 #include <stdarg.h>
 #include <progname.h>
+#include <sys/stat.h>
+
+#if HAVE_LIBTEXTSTYLE
+# include <textstyle.h>
+#endif

 #include "complain.h"
 #include "files.h"
@@ -63,6 +68,44 @@ static severity warnings_flag[warnings_size];

 static unsigned *indent_ptr = NULL;

+#if HAVE_LIBTEXTSTYLE
+styled_ostream_t errstream = NULL;
+#endif
+
+void
+begin_use_class (const char *s _GL_UNUSED, FILE *out _GL_UNUSED)
+{
+#if HAVE_LIBTEXTSTYLE
+  if (out == stderr)
+    {
+      styled_ostream_begin_use_class (errstream, s);
+      styled_ostream_flush_to_current_style (errstream);
+    }
+#endif
+}
+
+void
+end_use_class (const char *s _GL_UNUSED, FILE *out _GL_UNUSED)
+{
+#if HAVE_LIBTEXTSTYLE
+  if (out == stderr)
+    {
+      styled_ostream_end_use_class (errstream, s);
+      styled_ostream_flush_to_current_style (errstream);
+    }
+#endif
+}
+
+void
+flush (FILE *out _GL_UNUSED)
+{
+#if HAVE_LIBTEXTSTYLE
+  if (out == stderr)
+    ostream_flush (errstream, FLUSH_THIS_STREAM);
+#endif
+  fflush (out);
+}
+
 /*------------------------.
 | --warnings's handling.  |
 `------------------------*/
@@ -165,6 +208,22 @@ warnings_argmatch (char *args)
    warning_argmatch ("all", 0, 0);
 }

+static const char*
+severity_style (severity s)
+{
+  switch (s)
+    {
+    case severity_disabled:
+    case severity_unset:
+      return "note";
+    case severity_warning:
+      return "warning";
+    case severity_error:
+    case severity_fatal:
+      return "error";
+    }
+  abort ();
+}

 static const char*
 severity_prefix (severity s)
@@ -189,6 +248,31 @@ severity_prefix (severity s)
 | complain.  |
 `-----------*/

+void
+complain_init_color (void)
+{
+#if HAVE_LIBTEXTSTYLE
+  if (color_mode == color_yes
+      || (color_mode == color_tty && isatty (STDERR_FILENO)))
+    {
+      style_file_prepare ("BISON_DIAGNOSTICS_STYLE", NULL,
+                          pkgdatadir (),
+                          "diagnostics.css");
+      /* As a fallback, use the default in the current directory.  */
+      struct stat statbuf;
+      if ((style_file_name == NULL || stat (style_file_name, &statbuf) < 0)
+          && stat ("diagnostics.css", &statbuf) == 0)
+        style_file_name = "diagnostics.css";
+    }
+  else
+    /* No styling.  */
+    style_file_name = NULL;
+  errstream =
+    styled_ostream_create (STDERR_FILENO, "(stderr)", TTYCTL_AUTO,
+                           style_file_name);
+#endif
+}
+
 void
 complain_init (void)
 {
@@ -248,19 +332,24 @@ warning_is_unset (warnings flags)
  return true;
 }

-/** Display a "[-Wyacc]" like message on \a f.  */
+/** Display a "[-Wyacc]" like message on \a out.  */

 static void
-warnings_print_categories (warnings warn_flags, FILE *f)
+warnings_print_categories (warnings warn_flags, FILE *out)
 {
  /* Display only the first match, the second is "-Wall".  */
  for (size_t i = 0; warnings_args[i]; ++i)
    if (warn_flags & warnings_types[i])
      {
        severity s = warning_severity (warnings_types[i]);
-        fprintf (f, " [-W%s%s]",
+        const char* style = severity_style (s);
+        fputs (" [", out);
+        begin_use_class (style, out);
+        fprintf (out, "-W%s%s",
                 s == severity_error ? "error=" : "",
                 warnings_args[i]);
+        end_use_class (style, out);
+        fputc (']', out);
        return;
      }
 }
@@ -302,8 +391,15 @@ error_message (const location *loc, warnings flags, severity sever,
      indent_ptr = NULL;
    }

+  const char* style = severity_style (sever);
+
  if (sever != severity_disabled)
-    fprintf (stderr, "%s: ", severity_prefix (sever));
+    {
+      begin_use_class (style, stderr);
+      fprintf (stderr, "%s:", severity_prefix (sever));
+      end_use_class (style, stderr);
+      fputc (' ', stderr);
+    }

  vfprintf (stderr, message, args);
  /* Print the type of warning, only if this is not a sub message
@@ -318,7 +414,7 @@ error_message (const location *loc, warnings flags, severity sever,
        putc ('\n', stderr);
        fflush (stderr);
        if (loc && feature_flag & feature_caret && !(flags & no_caret))
-          location_caret (*loc, stderr);
+          location_caret (*loc, style, stderr);
      }
  }
  fflush (stderr);
--- a/src/complain.h
+++ b/src/complain.h
@@ -24,6 +24,20 @@
 /* Sub-messages indent. */
 # define SUB_INDENT (4)

+/*---------------.
+| Error stream.  |
+`---------------*/
+
+/** Enable a style on \a out provided it's stderr.  */
+void begin_use_class (const char *style, FILE *out);
+
+/** Disable a style on \a out provided it's stderr.  */
+void end_use_class (const char *style, FILE *out);
+
+/** Flush \a out.  */
+void flush (FILE *out);
+
+
 /*-------------.
 | --warnings.  |
 `-------------*/
@@ -76,6 +90,9 @@ void warnings_argmatch (char *args);
 /** Initialize this module.  */
 void complain_init (void);

+/** Initialize support for colored messages.  */
+void complain_init_color (void);
+
 typedef enum
  {
    Wnone             = 0,       /**< Issue no warnings.  */
--- a/src/getargs.c
+++ b/src/getargs.c
@@ -31,6 +31,10 @@
 #include <progname.h>
 #include <quote.h>

+#if HAVE_LIBTEXTSTYLE
+# include <textstyle.h>
+#endif
+
 #include "complain.h"
 #include "files.h"
 #include "muscle-tab.h"
@@ -501,10 +505,12 @@ static char const short_options[] =
 /* Values for long options that do not have single-letter equivalents.  */
 enum
 {
-  LOCATIONS_OPTION = CHAR_MAX + 1,
-  PRINT_LOCALEDIR_OPTION,
+  COLOR_OPTION = CHAR_MAX + 1,
+  LOCATIONS_OPTION,
  PRINT_DATADIR_OPTION,
-  REPORT_FILE_OPTION
+  PRINT_LOCALEDIR_OPTION,
+  REPORT_FILE_OPTION,
+  STYLE_OPTION
 };

 static struct option const long_options[] =
@@ -532,6 +538,8 @@ static struct option const long_options[] =

  /* Hidden. */
  { "trace",       optional_argument,   0,  'T' },
+  { "color",       optional_argument,   0,  COLOR_OPTION },
+  { "style",       optional_argument,   0,  STYLE_OPTION },

  /* Output.  */
  { "defines",     optional_argument,   0,   'd' },
@@ -575,11 +583,33 @@ command_line_location (void)
 }


+/* Handle the command line options for color support.  Do it early, so
+   that error messages from getargs be also colored as per the user's
+   request.  This is consistent with the way GCC and Clang behave.  */
+
+static void
+getargs_colors (int argc _GL_UNUSED, char *argv[] _GL_UNUSED)
+{
+#if HAVE_LIBTEXTSTYLE
+  for (int i = 1; i < argc; i++)
+    {
+      const char *arg = argv[i];
+      if (STRPREFIX_LIT ("--color=", arg))
+        handle_color_option (arg + strlen ("--color="));
+      else if (STRPREFIX_LIT ("--style=", arg))
+        handle_style_option (arg + strlen ("--style="));
+    }
+  complain_init_color ();
+#endif
+}
+
+
 void
 getargs (int argc, char *argv[])
 {
-  int c;
+  getargs_colors (argc, argv);

+  int c;
  while ((c = getopt_long (argc, argv, short_options, long_options, NULL))
         != -1)
    switch (c)
@@ -723,6 +753,10 @@ getargs (int argc, char *argv[])
        yacc_loc = command_line_location ();
        break;

+      case COLOR_OPTION:
+        /* Handled in getargs_colors. */
+        break;
+
      case LOCATIONS_OPTION:
        muscle_percent_define_ensure ("locations",
                                      command_line_location (), true);
@@ -741,6 +775,10 @@ getargs (int argc, char *argv[])
        spec_verbose_file = xstrdup (AS_FILE_NAME (optarg));
        break;

+      case STYLE_OPTION:
+        /* Handled in getargs_colors. */
+        break;
+
      default:
        usage (EXIT_FAILURE);
      }
--- a/src/local.mk
+++ b/src/local.mk
@@ -126,6 +126,7 @@ src_bison_LDADD =                               \
  $(LIBTHREAD)                                  \
  $(LIB_CLOCK_GETTIME)                          \
  $(LIB_GETHRXTIME)                             \
+  $(LIBTEXTSTYLE)                               \
  lib/libbison.a


--- a/src/location.c
+++ b/src/location.c
@@ -160,7 +160,7 @@ caret_free ()
 }

 void
-location_caret (location loc, FILE *out)
+location_caret (location loc, const char *style, FILE *out)
 {
  /* FIXME: find a way to support multifile locations, and only open once each
     file. That would make the procedure future-proof.  */
@@ -193,9 +193,17 @@ location_caret (location loc, FILE *out)
    if (c != EOF)
      {
        /* Quote the file, indent by a single column.  */
-        putc (' ', out);
+        fputc (' ', out);
+        int col = 0;
        do
-          putc (c, out);
+          {
+            ++col;
+            if (col == loc.start.column)
+              begin_use_class (style, out);
+            fputc (c, out);
+            if (col + 1 == loc.end.column)
+              end_use_class (style, out);
+          }
        while ((c = getc (caret_info.source)) != EOF && c != '\n');
        putc ('\n', out);

@@ -208,8 +216,10 @@ location_caret (location loc, FILE *out)

          /* Print the carets (at least one), with the same indent as above.*/
          fprintf (out, " %*s", loc.start.column - 1, "");
+          begin_use_class (style, out);
          for (i = loc.start.column; i == loc.start.column || i < len; ++i)
            putc (i == loc.start.column ? '^' : '~', out);
+          end_use_class (style, out);
          }
        putc ('\n', out);
      }
--- a/src/location.h
+++ b/src/location.h
@@ -113,7 +113,7 @@ unsigned location_print (location loc, FILE *out);
 void caret_free (void);

 /* Output to OUT the line and caret corresponding to location LOC.  */
-void location_caret (location loc, FILE *out);
+void location_caret (location loc, const char* style, FILE *out);

 /* Return -1, 0, 1, depending whether a is before, equal, or
   after b.  */
--- a/tests/bison.in
+++ b/tests/bison.in
@@ -26,6 +26,14 @@ BISON_PKGDATADIR=$abs_top_srcdir/data
 export BISON_PKGDATADIR

 stderr=tmp-bison.$$
+
+# If stderr is a tty, force --color=yes to simulate --color=auto
+# although we save and modify stderr.
+if test -t 2; then
+    set x --color=yes ${1+"$@"}
+    shift
+fi
+
 $PREBISON "$abs_top_builddir/src/bison" ${1+"$@"} 2>"$stderr"
 status=$?

--- a/tests/input.at
+++ b/tests/input.at
@@ -1807,7 +1807,7 @@ start: %empty;
 ]])
 AT_BISON_CHECK([[-Dvar=cmd-d input-dg.y]], [[1]], [],
 [[input-dg.y:1.1-18: error: %define variable 'var' redefined
-<command line>:2:      previous definition
+<command line>:3:      previous definition
 input-dg.y: warning: fix-its can be applied.  Rerun with option '--update'. [-Wother]
 ]])

@@ -1820,7 +1820,7 @@ AT_BISON_CHECK([[-fcaret -Dvar=cmd-d input-dg.y]], [[1]], [],
 [[input-dg.y:1.1-18: error: %define variable 'var' redefined
 %define var "gram"
 ^~~~~~~~~~~~~~~~~~
-<command line>:3:      previous definition
+<command line>:4:      previous definition
 input-dg.y: warning: fix-its can be applied.  Rerun with option '--update'. [-Wother]
 ]])

@@ -1829,8 +1829,8 @@ AT_DATA([[input-unused.y]],
 start: %empty;
 ]])
 AT_BISON_CHECK([[-Dunused-d -Funused-f input-unused.y]], [[1]], [],
-[[<command line>:2: error: %define variable 'unused-d' is not used
-<command line>:3: error: %define variable 'unused-f' is not used
+[[<command line>:3: error: %define variable 'unused-d' is not used
+<command line>:4: error: %define variable 'unused-f' is not used
 ]])

 AT_CLEANUP
@@ -2251,11 +2251,11 @@ start: %empty;
 # parse.lac.* options are useless if LAC isn't actually activated.
 AT_BISON_CHECK([[-Dparse.lac.es-capacity-initial=1 input.y]],
               [[1]], [],
-[[<command line>:2: error: %define variable 'parse.lac.es-capacity-initial' is not used
+[[<command line>:3: error: %define variable 'parse.lac.es-capacity-initial' is not used
 ]])
 AT_BISON_CHECK([[-Dparse.lac.memory-trace=full input.y]],
               [[1]], [],
-[[<command line>:2: error: %define variable 'parse.lac.memory-trace' is not used
+[[<command line>:3: error: %define variable 'parse.lac.memory-trace' is not used
 ]])

 AT_CLEANUP
@@ -2326,8 +2326,8 @@ AT_BISON_CHECK([[$2 -Wno-deprecated input.y]], [[1]], [[]],
 ])

 AT_TEST([%define api.prefix {foo} %name-prefix "bar"], [], [input.y:1.1-24])
-AT_TEST([], [-Dapi.prefix={foo} -p bar],                   [<command line>:2])
-AT_TEST([%name-prefix "bar"], [-Dapi.prefix={foo}],        [<command line>:2])
+AT_TEST([], [-Dapi.prefix={foo} -p bar],                   [<command line>:3])
+AT_TEST([%name-prefix "bar"], [-Dapi.prefix={foo}],        [<command line>:3])
 AT_TEST([%define api.prefix {foo}], [-p bar],              [input.y:1.1-24])

 m4_popdef([AT_TEST])
--- a/tests/local.at
+++ b/tests/local.at
@@ -807,7 +807,7 @@ AT_BISON_CHECK_NO_XML($@)])
 # --------------------------------------------------
 # Low-level macro to run bison once.
 m4_define([AT_BISON_CHECK_],
-[AT_CHECK(AT_QUELL_VALGRIND[[ bison -fno-caret ]]$@)])
+[AT_CHECK(AT_QUELL_VALGRIND[[ bison --color=no -fno-caret ]]$@)])


 # AT_BISON_CHECK_WARNINGS(BISON_ARGS, [OTHER_AT_CHECK_ARGS])
@@ -862,7 +862,7 @@ fi]dnl
 # when a tortured grammar's XML is known to be too large for xsltproc to
 # handle.
 m4_define([AT_BISON_CHECK_NO_XML],
-[AT_CHECK(m4_null_if([$2], [], [AT_QUELL_VALGRIND ])[[bison -fno-caret ]]$@)
+[AT_CHECK(m4_null_if([$2], [], [AT_QUELL_VALGRIND ])[[bison --color=no -fno-caret ]]$@)
 AT_BISON_CHECK_WARNINGS($@)])

 # AT_BISON_CHECK_XML(BISON_ARGS, [OTHER_AT_CHECK_ARGS])