Re: [PATCH 0/3] Various clean up in preparation for libtextstyle

[Akim Demaille]

Hi HS!

Hi Bruno.  Below there are a few comments about libtextstyle to which
you might want to answer.

> Le 14 févr. 2019 à 23:00, H. S. Teoh <address@hidden> a écrit :
> 
> On Thu, Feb 14, 2019 at 09:52:53PM +0100, Akim Demaille wrote:
>> 
>> I used to think that, but when clang and gcc introduced color in their
>> diagnostics, I clearly saw this as an improvement (to my eyes).  I
>> also appreciate the colors in diff, and in git.
> 
> OTOH, I found all of these instances quite annoying, and took pains to
> turn them all off.  As they say, YMMV. :-D

Yep, but it's always good to know.  Thanks for reporting.


> Caret errors are good.  I don't have a problem with that.

Ok.

I think I might make a few more adjustments, based on a few ideas I
saw on Clang.  For instance:

$ clang++-mp-7.0 /tmp/foo.cc
/tmp/foo.cc:3:3: error: use of undeclared identifier 'inti'; did you mean 'int'?
  inti i = 0;
  ^~~~
  int
1 error generated.

We do this:

$ LC_ALL=C ./_build/8d/tests/bison /tmp/foo.yy
/tmp/foo.yy:1.1-14: warning: deprecated directive, use '%define parse.error 
verbose' [-Wdeprecated]
 %error-verbose
 ^~~~~~~~~~~~~~


It would probably be nicer to print (I really appreciate
that the lines are shorter):

/tmp/foo.yy:1.1-14: warning: deprecated directive [-Wdeprecated]
 %error-verbose
 ^~~~~~~~~~~~~~
 %define parse.error verbose

Of course when caret-errors are disabled, we would keep the long
version (which is already different as in that case I spell out
the name of the deprecated directive):

$ LC_ALL=C ./_build/8d/tests/bison -fno-caret /tmp/foo.yy
/tmp/foo.yy:1.1-14: warning: deprecated directive: '%error-verbose', use 
'%define parse.error verbose' [-Wdeprecated]



> What I do
> have a problem with is color, especially the garish 8-bit kind that
> we're usually stuck with due to historical reasons (try doing 16-bit or
> more and you start getting into the messy realm of parsing $TERM and
> dealing with termcap with spotty/inconsistent support across different
> terminal implementations).

That's exactly the benefits from using libtextstyle: you don't have
to face arcane color escapes to describe what change you want, and it
takes care of the different terminal types.

> Whoa. IMNSHO, when a program needs to parse CSS just to be able to print
> diagnostic messages, then something has gone truly rotten in the state
> of programming.

:) :) :)

Well, OTOH, going for standard syntaxes is better.  And if libtextstyle
gets widespread, there is no significant additional weight in the
program itself.  And static-linking lovers can just do without, I do
not plan to make it a requirement.

I understand your reaction, and I'm not entirely convinced that this
is the right track for Bison.  Maybe Bruno wants to jump into the
conversation?

However, I see it opens the door to more colors than I first
anticipated.  I have been wanting colors in the parser's traces
for a long time, but something I now consider is making the output
files (*.output, that describe the parser) also in color.  I don't
plan to dress it like a unicorn, but highlight its structure a bit
better, and draw attention to conflicted states for instance.
But moving all the output functions on top of ostream would be
quite painful.  I really wonder if I should not try to synchronize
the ostream for style, and the FILE* for output.  And pay attention
to end-of-lines myself.

My current approach for diagnostics, which moves all the uses of stderr
on top of libtextstyle's streams is not satisfactory.  It scales poorly
with existing code, for instance all the library functions that
emit errors on stderr (getopt and argmatch for instance).  getopt
is especially tricky, since I want to use getopt to set up
libtextstyle, and I want to use libtextstyle for errors.  If I
want this to be consistent, I should make a first pass on options
to possibly set up libtextstyle for my error handling, and then
a second time to be able to report errors on the bad options
with libtextstyle.  GCC and Clang must be something similar: when
I pass a nonexistent option before -fdiagnostics-color=never/
-fno-color-diagnostics, the error is in plain style.

I will try to play with flush and simultaneous
uses of stderr and styled_ostream and see how it goes.


> If forced to write one, my version of diagnostics.css would be:
> 
>       * { }
> 
> :-P

:) :) :)

> The underline style is nice, I suppose, but the other examples you gave
> are exactly the kind of ugly garish colors straight out of the 80's that
> I find so unpalatable.

But you did get that I was kidding there right?  I'm getting worried
about the taste you might think I have :)


Cheers!