[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH 0/3] Various clean up in preparation for libtextstyle
|
From: |
Akim Demaille |
|
Subject: |
Re: [PATCH 0/3] Various clean up in preparation for libtextstyle |
|
Date: |
Sat, 16 Mar 2019 08:23:30 +0100 |
Hi Bruno!
> Le 13 mars 2019 à 02:03, Bruno Haible <address@hidden> a écrit :
>
> Hi Akim,
>
> This is now implemented, in the new tarball
> https://haible.de/bruno/gnu/libtextstyle-0.6.tar.gz
Excellent! Thanks a lot for the effort!
Here is the current state of my proposal (no documentation for the time being,
let's first see how people react to the feature). This is no longer a
sourcequake compared to my previous attempt, and I like that.
Cheers!
commit 59edd6093567b592217a62c92e999c7928b837e3
Author: Akim Demaille <address@hidden>
Date: Thu Feb 14 06:49:29 2019 +0100
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.
diff --git a/bootstrap.conf b/bootstrap.conf
index 1e58284a..f8fb1a11 100644
--- 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
diff --git a/data/diagnostics.css b/data/diagnostics.css
new file mode 100644
index 00000000..5a9da591
--- /dev/null
+++ b/data/diagnostics.css
@@ -0,0 +1,3 @@
+.warning { color : purple; }
+.error { color : red; }
+.note { color : cyan; }
diff --git a/data/local.mk b/data/local.mk
index 77c7bf75..8068346b 100644
--- 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 = \
diff --git a/m4/.gitignore b/m4/.gitignore
index 43cae90b..a66d8661 100644
--- a/m4/.gitignore
+++ b/m4/.gitignore
@@ -195,3 +195,6 @@
/readlink.m4
/relocatable-lib.m4
/relocatable.m4
+/libtextstyle.m4
+/rename.m4
+/rmdir.m4
diff --git a/src/complain.c b/src/complain.c
index 50abdb84..70ff4204 100644
--- 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);
diff --git a/src/complain.h b/src/complain.h
index 266992f6..c136c071 100644
--- 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. */
diff --git a/src/getargs.c b/src/getargs.c
index e71aafd3..541a05d7 100644
--- 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[] =
@@ -531,7 +537,9 @@ static struct option const long_options[] =
{ "verbose", no_argument, 0, 'v' },
/* Hidden. */
- { "trace", optional_argument, 0, 'T' },
+ { "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);
}
diff --git a/src/local.mk b/src/local.mk
index 4b0a0ff5..ee8ec03b 100644
--- 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
diff --git a/src/location.c b/src/location.c
index ab478107..3eae89be 100644
--- 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);
}
diff --git a/src/location.h b/src/location.h
index 3b1fcab6..6702cf10 100644
--- 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. */
diff --git a/tests/bison.in b/tests/bison.in
index afa9c2b1..9de13e42 100644
--- 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=$?
diff --git a/tests/input.at b/tests/input.at
index ef777f83..312aa5a9 100644
--- 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])
diff --git a/tests/local.at b/tests/local.at
index 8ba82c4f..b7695cfa 100644
--- 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])