Re: [PATCH] [grotty]: Use terminfo.

[Lennart Jablonka]

Quoth G. Branden Robinson:
> Hi Lennart,
>
> Thanks a lot for taking a stab at this!
>
> In lieu of a proper code review, I'll just riff on this to get the
> issues out before an audience.
>
> At 2023-08-19T20:08:06+0000, Lennart Jablonka wrote:
> > This has nothing at all to do with making it easier to customize the
> > look of man pages.
>
> I sort of agree, if you refer very broadly to all questions of formatted
> appearance.  groff man(7) has several tunable parameters for that.
>
> If you mean by applying a color scheme, it makes one approach easier, in
> principle.  It opens an avenue for people to do it by redefining
> terminfo capabilities to produce whatever terminal escape sequences they
> want, and then letting grotty observe the desired terminal type via the
> $TERM environment variable (actually it'd be terminfo doing that, which
> was the whole point of Savannah #63583).
>
> There remains the avenue of redefining man(7) macros in .../man.local,
> but historically that has not been a popular approach, and one has to
> understand a moderate amount of groff formatting language to do that.

Can we state at least somewhere how much of a hack (cf. a certain 
earlier message of mine) all these things are?   Like in NEWS.

> > If it stays, I’m unsure of the interface to grotty.  The -T disabling
> > the fallback seems hackish.
>
> I'm unsure that we need a `-T` option if we getenv("TERM") (or if
> terminfo(3) does).  No other part of groff cares about the terminal type
> in any way, so sticking TERM=xterm+my-pimped-out-colors-for-man-pages in
> the environment of a groff command isn't going to mess anything else up.
>
> Okay, it might mess up the pager they pipe groff/grotty's output
> through, but that's Not Our Problem.

I like that Not Our Problem.   Alright.

> (I observe that ncurses doesn't actually _provide_ a terminfo(3) page,
> which is silly.  Or rather, the page is there, but it doesn't bother to
> mention its own name in its NAME section so that makewhatis/mandb can
> find it.  Guess I'll be sending a patch.)

Wait, man-db does’t use a man page’s title to look it up at all?   
That seems bad.

> Until it does, read "terminfo(3)" as "putp(3)".  It's brings up the
> relevant page and is quicker to type anyway.

That conflicts with my sense of aesthetic.   References should use 
the man page title.   The correct way to refer to putp’s man page 
is something like “Print stuff using putp (see terminfo(3)).”   
(Alex, you don’t happen to be listening in?   KWIC index for the 
man-pages book when?)

> > Terminfo naturally doesn’t have a capability for hyperlinks.
>
> It could get one.  Thomas Dickey has had no compunction about adding
> other ones.  I understand, but am disappointed that, his new terminfo
> capabilities seem always to be only 2 characters long (for termcap
> compatibility).  I feel that the termcap API has been mollycoddled for
> far too long.

See below on Curses.

> > Autoconf is not informed about the depence on Curses; we simply link
> > with -lcurses, as that is what X/Open Curses specifies.
>
> Yeah, we'll need to Do The Right Thing here.  Not a problem.  I can take
> care of it.

See below on Curses.

> > The suggested option to less is changed to -r, which enables passing
> > through single control characters, because some terminal descriptions
> > use those;
>
> Last time I mentioned less(1)'s `-r` option, I alarmed Ingo Schwarze.
> It might be worth elaborating this point.

I’ve read Ingo’s reply;  perhaps later I’ll think about this 
stuff.   /me is annoyed at the state of the world.

> > I noticed that xterm’s sgr0 contains ^O.
>
> Hmm...I see that for "xterm-xf86-v32" (meaning the xterm distributed
> with XFree86 3.2, which is 25+ years old), "xtermm" (Solaris
> monochrome), "color_xterm" (a long-dead fork from ~1996).  Other xterm
> terminfo descriptions--and there are many--seem to use "ESC [ m"
> (xterm-r5, xterm-r6); "ESC ( B ESC [ m" (xterm-basic); or 8-bit controls
> "CSI m" (xterm-8bit).

Oops, you’re right.   It’s not xterm, it’s tmux (inheriting it 
from screen).

> > I didn’t test this a whole lot, much less wrote automated tests.
> > Have fun.
>
> Oh, boy, the very best part.  :-|  But the one that helps me sleep at
> night instead of awakening in a sweat over what the users may discover.
>
> > PS: There doesn’t happen to be a convention on whether to use
> > non-ASCII characters in commit messages, right?  Seeing as you,
> > Branden, yourself aren’t always quite consistent with that.
>
> I don't have a firm position on this.  My impression is that Git users
> are almost invariably UTF-8 partisans.  I may have applied patches from
> people who were using ISO Latin-something, though.  And also, Git front
> ends like cgit are sometimes baffled when a commit message is in UTF-8
> but the diff part is to an ISO Latin-1 file.
>
> So the inconsistency might not _always_ be just me.  But sometimes,
> sure.

Uh, I meant non-ASCII UTF-8 instead of ASCII and in
particular punctuation.   Sometimes ASCII ' and " are 
used, sometimes ‘ ’ “ ”.

> > PPS: Mandoc underlines italic \~s.
> >
> >         .RI [ file\~ .\|.\|.]
> >
> > is rendered as
> >
> >         [_f_i_l_e_ ...]
> >
> > by mandoc.  Just noticed that.
>
> Yeah.  I think that's ugly.  I'm glad we don't.

> > --- a/INSTALL.extra
> > +++ b/INSTALL.extra
> > @@ -122,12 +122,15 @@ Several programs distributed with GNU roff are written in 
> > the Perl
> >  language.  Version 5.6.1 (1 April 2001) or later is required.
> >
> >  The 'uchardet' library is an optional dependency of the 'preconv'
> > -program: if this library is found by 'configure', it will be
> > +program: If this library is found by 'configure', it will be
>
> Nack.  What comes after the colon may be an independent clause, but it
> is not a sentence.  ;-)

Widening my horizon, I see some style guides¹ that don’t seem to 
recommend capitalizing independent clauses following colons.   
Contrary to what I believed until now.

¹ One style guide.   I didn’t widen my horizon too much.

Btw, in Groff docs, is there a certain house style you’d like to 
imitate, is there a guide you give some authority over matters of 
usage for Groff?

> > +The 'grotty' program depends on a Curses library, which is specified
> > +by X/Open Curses.
>
> Yeah, I can elaborate this.  After some research.

Here goes.

I like to point to standards.   My use of base utilities follows 
POSIX sans few quite specific things (c99 -c -o being unspecified 
doesn’t concern me much);  dependence on GNU coreutils or gmake 
I don’t want to take on.   My C follows ISO C or POSIX;  of GNU 
C I stay clear.   Ncurses I avoid mentioning, as there is 
a relevant standard:  X/Open Curses.   Its latest edition being 
Issue 7, downloadable gratis with an Open Group account.

For no reading in particular, I like the idea of comparing Curses 
to groff.

Originally there was troff;  nowadays, groff is the most 
widespread implementation.

Originally there was curses (and termcap and terminfo);  nowadays, 
ncurses is the most widespread implementation.

There are other troff implementations/descendants:  
Heirloom troff, neatroff, 9front’s troff.

There are other Curses implementations/descendants:  
NetBSD Curses, PDCurses.

Those that worked on troff a long time ago now use groff.

Those that worked on curses a long time ago now use ncurses.

For better or worse, there are documents (man pages in particular) 
depending on groff in particular.

For better or worse, there are programs depending on ncurses in 
particular.

Whenever groff introduces a shiny feature that is easy to use for 
authors (of man pages), like \(lq \(rq or .MR, other 
implementations are likely to feel some pressure to copy it to 
remain viable.

Whenever ncurses introduces a shiny feature that is easy to use 
for authors of programs, like, idk (I don’t know Curses well), new 
capabilities or something, I suppose other implementations fell 
some pressure to copy it to remain viable.

For troff, there is no standard.

For Curses, there is a standard.

Now that I’ve compared their situations without making any point, 
I can go back to doing that.   There is a standard, so I don’t 
want to make ncurses special.

For capabilities, that means not requiring any not in 
X/Open Curses and submitting new ones for standardization.   
(I have no idea how the X/Open Curses standardization process 
works.)   That does not preclude the conditional use of new 
capabilities.   I like that idea for OSC 8.

For building, see below.

> > +Output drivers
> > +--------------
> > +
> > +grotty
> > +------
> > +
> > +o The terminfo library is now used for applying text attributes (like
> > +  italic, bold, and colors).
>
> We will want to say more about this.  What might break?

Right.

  - you need Curses
  - assuming I drop the hardcoded SGR escape sequences in a v2, 
    you need a working terminal description

> > diff --git a/src/devices/grotty/grotty.1.man b/src/devices/grotty/grotty.1.man
> > index 41eabbcfa..ebdfd1639 100644
> > --- a/src/devices/grotty/grotty.1.man
> > +++ b/src/devices/grotty/grotty.1.man
> > @@ -56,6 +56,8 @@ output driver for typewriter-like (terminal) devices
> >  .RB [ \-i \||\| \-r ]
> >  .RB [ \-F\~\c
> >  .IR font-directory ]
> > +.RB [ \-T\~\c
> > +.IR term ]
> [...]
> > +.RB [ \-T\~\c
> > +.IR term ]
>
> As noted above, I want to see if we can go without this and just get
> away with terminfo(3) doing a getenv("TERM") for us.

ack

> > @@ -136,9 +140,8 @@ Output is written to the standard output stream.
> >  .P
> >  By default,
> >  .I grotty
> > -emits SGR escape sequences
> > -(from ISO\~6429,
> > -popularly called \[lq]ANSI escapes\[rq])
> > +uses
> > +.MR terminfo 5
>
> I would refer to the library page in section 3 here.  That's what we
> use.  The section 5 page is a likely destination for the colorized man
> page community, though, so I reckon we could throw them a bone later in
> the page, and link to it.

Whoever reads grotty(1) is unlikely to care about the specific 
functions we use and is more interested in the idea of terminfo.   
But sure.

> > @@ -148,6 +151,15 @@ reverse video
> >  [\[lq]negative image\[rq]]
> >  and colors).
> >  .
> > +If no
> > +.I TERM
> > +environment variable is set
> > +or terminfo encounters a different error,
>
> Recast maybe.
>
> If
> .I terminfo
> encounters an error
> (perhaps because
> .I TERM
> is unset or its value unrecognized),

ack

> > +.I grotty
> > +emits hardcoded SGR escape sequences
> > +(from ISO\~6429,
> > +popularly called \[lq]ANSI escapes\[rq]).
>
> […]
>
> > +.TP
> > +.I TERM
> > +The terminal type for
> > +.I terminfo.
>
> We don't interpret it ourselves, but a library we use does.  I think I'd
> throw to the library's man page for it.
>
> .TP
> .I TERM
> declares the terminal type interpreted by the
> .MR terminfo 3
> library.

ack

> I see I could recast the description of GROFF_NO_SGR to be similarly
> "stemmy".

If you commit before I send v2, it’s likely that I’ll follow your 
style changes.

> >  grotty_LDADD = $(LIBM) \
> >    libdriver.a \
> >    libgroff.a \
> > -  lib/libgnu.a
> > +  lib/libgnu.a \
> > +  -lcurses
>
> Need to see what the Right Way is here.  The main issue may just be
> deciding on the name for the library, which would involve Autoconf.
> I'd also like to use terminfo directly (-ltinfo) rather than curses, if
> possible.  I don't think curses proper has anything we need.

The one and only name provided and guaranteed to work by X/Open 
Curses is -l curses.   Besides, it should be cheap to link with 
a static -l curses when only using terminfo and I like to ignore 
things I dislike, such as dynamic linking.

If you want to tell Autoconf of Curses, go ahead, but keep in mind 
a comparison with -l m.   When using math functions, would you 
check whether -l m works or is needed?   I wouldn’t, as POSIX 
guarantees -l m to work.

> I _don't_ want to refactor grotty(1), as a single program, to support
> being built with or without terminfo.  That way lies a rope bridge over
> a chasm belching flames hundreds of feet into the air, and its name is
> dlopen().  (Portability problems are what scare me.)

> If we need to keep a terminfo-free output driver around, then I'd rather
> keep grotty more or less as-is and do this work in a new
> terminfo-enhanced TTY driver.  "grotitty" or something.
>
> Shared routines could of course be moved into a common.cpp file.

I don’t see where you’d consider using dlopen here, but otherwise 
mostly agree.   Though then I’d still drop SGR and have 
a more or less clean split between overstriking and terminfo.

> > diff --git a/src/devices/grotty/tty.cpp b/src/devices/grotty/tty.cpp
> > index 1b92ad8c0..9df35d8cd 100644
> > --- a/src/devices/grotty/tty.cpp
> > +++ b/src/devices/grotty/tty.cpp
> > […]
> > @@ -38,7 +38,7 @@ extern "C" const char *Version_string;
> >  #define SHRT_MAX 32767
> >  #endif
> >
> > -#define TAB_WIDTH 8
> > +static int tab_width = 8;
>
> Strictly, this is a style fix that might be committed first.  Symbol
> visibility in the debugger is a big +1.

That would introduce a whole commit for the purpose of having just 
a single commit to be able to debug non-terminfo grotty with 
symbol tab_width.   Nah, disagree.   And I didn’t consider the 
additional symbol, I just wanted to assign to tab_width later.

Btw, with gcc -g3, you get macro symbols.

Oh, and even if I were to change this to define the constant 
differently, I’d do

        enum {TAB_WIDTH = 8};

> >  // A character of the output device fits in a 32-bit word.
> >  typedef unsigned int output_character;
> > @@ -56,10 +56,16 @@ static bool do_sgr_italics;
> >  static bool want_reverse_video_for_italics = false;
> >  static bool do_reverse_video;
> >  static bool use_overstriking_drawing_scheme = false;
> > +static bool use_terminfo;
> > +static char *terminal_name;
> >
> >  static void update_options();
> >  static void usage(FILE *stream);
> >
> > +// TODO(humm): We can get proper line drawing characters from terminfo
> > +// using acsc, smacs, rmacs; we could do that for -Tascii + terminfo.
> > +// There are other characters there we could theoreticlly use as well,
> > +// like degree and pi.
>
> I fear this promises to be a bit of a headache, not that I object to the
> idea.  I'd like to kick this sort of thing past
> <https://savannah.gnu.org/bugs/?62471>, which I perceive as more
> important.  What do you think?

You might have less despair now that 1.23.0 is out, I am still 
filled with despair on looking at that page.   My despair might 
fade with a little more looking at it, but I don’t wanna do that 
now.   But yes, using the alternate character set seems entirely 
unimportant to me, and at least much less important than tbl bugs.

> Also I need to understand the boundaries of the DEC Alternate Character
> Set feature.  I guess if the terminal supports it, we have an overriding
> set of glyph mappings we'd use instead of what's in the font description
> files.  That would then make the font description files not necessarily
> the Source of Truth for the output driver's behavior.  Headache
> intensifies.  :(

Yes, headache indeed.

> > +// bold and reverse can be unset separately using ISO 6429, but not
> > +// with terminfo
>
> That sounds like something we should complain about to Thomas Dickey.

Yeah, and to whoever decides what goes in X/Open Curses.

Oh, and note that the capability sgr exists, which doesn’t play 
well with specifying colors.

> >  // many terminals can't handle 'CSI 39 m' and 'CSI 49 m' to reset
> >  // the foreground and background color, respectively; we thus use
> >  // 'CSI 0 m' exclusively
> > -#define SGR_DEFAULT CSI "0m"
> > +static const char *sgr_exit_attributes = CSI "0m";
>
> Mmmm.  I don't like hiding other people's bugs.  With respect to
> terminal emulators in particular, though, doing so seems to have been a
> popular thing, and it may be one of the reasons terminal emulators have
> tended to suck in some respects.  They're often brilliant at stuff like
> alpha-blended color gradients and URL tooltip pop-ups, but completely
> defecate themselves when it comes to basic matters of competency like
> VT100 emulation.
>
> Kinda tempted to revert this 0m hack (in a separate commit) and see what
> happens.

I agree.   Terminfo has the capability op for resetting colors, if 
I understand it correctly.

> Your change per se is fine.
>
> >  class tty_printer : public printer {
> > -  tty_glyph **lines;
> > +  tty_glyph **lines_; // clashes with Curses lines
>
> I hear the bike shed crying out for paint...

Painting is too much work.   If a passerby wants to paint it, they 
can go ahead.

> > -  void make_underline(int);
> > -  void make_bold(output_character, int);
> > +  void update_attributes(bool, bool, schar, schar, output_character, int);
>
> Functions with gigantic argument lists make me a sad panda.  Can we use
> a struct instead?

To complicate its use?   I don’t know.   We could duplicate the 
members tracking the state to track the wanted state, then have 
a function for updating the current state.

> >    schar color_to_idx(color *);
> >    void add_char(output_character, int, int, int, color *, color *,
> >                 unsigned char);
>
> This one was making me sad already.
>
> > @@ -201,10 +204,8 @@ public:
>
> (No comments on this hunk.)
>
> > @@ -271,54 +272,91 @@ tty_printer::tty_printer() : cached_v(0)
> [...]
> > -void tty_printer::make_underline(int w)
> > +/* Update the current text attributes, doing little more than necessary
> > +work.  The c and w parameters are not used unless
> > +use_overstriking_drawing_scheme and at least one of underline and bold
> > +is set. */
>
> Should we bust this function into two?  One for overstriking output, and
> one for everything else?

It works.   I’ll include the split version in v2.

> […]
>
> > @@ -458,6 +496,8 @@ void tty_printer::special(char *arg, const environment 
> > *env, char type)
> >  // repeated arbitrarily and are separated by colons.  Omission of the
> >  // URI ends the hyperlink that was begun by specifying it.  See
> >  // <https://gist.github.com/egmontkob/eb114294efbcd5adb1944c9f3cb5feda>.
> > +//
> > +// TODO(humm): What should be done with OSC 8 when using terminfo?
>
> Assume an ISO 6429-conformant terminal and blast it out.  If anyone
> complains, sweatily point at Thomas Dickey and protest that the mean old
> terminfo maintainer wouldn't give us a capability for it.
>
> That's, like, page 1 of "How to Get Ahead in Software Engineering".

ack

> > -void tty_printer::change_color(const environment * const env)
> > -{
> > -  add_char(0, 0, env->hpos, env->vpos, env->col, env->fill, COLOR_CHANGE);
> > -}
> > -
>
> I have lost track of why we no longer need this member function.
> Coalesced into update_attributes() with the huge argument list?

It … I don’t know.   I’ll re-add it in v2.   I didn’t remove any 
calls to it, it just looked unused.   I now see that it’s 
overriding a virtual member function.   Wanna switch to C++11 to 
be able to use the override keyword?

> >  void tty_printer::change_fill_color(const environment * const env)
> >  {
> >    add_char(0, 0, env->hpos, env->vpos, env->col, env->fill, COLOR_CHANGE);
> > @@ -683,42 +718,12 @@ void tty_printer::put_char(output_character wc)
> >      do *++p = (unsigned char)(((wc >> (6 * --count)) & 0x3f) | 0x80);
> >        while (count > 0);
> >      *++p = '\0';
> > -    putstring(buf);
> > +    fputs(buf, stdout);
>
> A lingering use of fputs() rather than putp().  Your changed version
> looks correct, and I'm not deeply enamored of using C preprocessor
> macros the way this file was with `putstring`.

The putp function does padding:  You can embed something like $<5> 
in a capability to indicate 5 milliseconds of padding at that 
position.   We don’t want that for non-capability strings, as 
those should not be interpreted by putp or, at the very least, 
don’t need to be.

> > -void tty_printer::put_color(schar color_index, int back)
> > -{
> > -  if (color_index == DEFAULT_COLOR_IDX) {
> > -    putstring(SGR_DEFAULT);
> > -    // set bold and underline again
> > -    if (is_boldfacing)
> > -      putstring(SGR_BOLD);
> > -    if (is_underlining) {
> > -      if (do_sgr_italics)
> > -       putstring(SGR_ITALIC);
> > -      else if (do_reverse_video)
> > -       putstring(SGR_REVERSE);
> > -      else
> > -       putstring(SGR_UNDERLINE);
> > -    }
> > -    // set other color again
> > -    back = !back;
> > -    color_index = back ? curr_back_idx : curr_fore_idx;
> > -  }
> > -  if (color_index != DEFAULT_COLOR_IDX) {
> > -    putstring(CSI);
> > -    if (back)
> > -      putchar('4');
> > -    else
> > -      putchar('3');
> > -    putchar(color_index + '0');
> > -    putchar('m');
> > -  }
> > -}
> > -
>
> Obsoleted by `tty_printer::update_attributes()`?

yes

> […]
>
> > -           if (is_continuously_underlining)
> > -             make_underline(p->w);
> > -           else if (!use_overstriking_drawing_scheme
> > -                    && is_underlining) {
> > -             if (do_sgr_italics)
> > -               putstring(SGR_NO_ITALIC);
> > -             else if (do_reverse_video)
> > -               putstring(SGR_NO_REVERSE);
> > -             else
> > -               putstring(SGR_NO_UNDERLINE);
> > -             is_underlining = false;
> > -           }
>
> Why is this going away?

subsumed by the call to update_attributes

> > +           // TODO(humm): Should we not take the width of the actual
> > +           // tab?  The current status works, albeit not with
> > +           // typewriters: _\b\t is rendered as multiple underlined
> > +           // cells by less.  We could pass update_attributes the width
> > +           // the actual tab will have and let it emit multiple
> > +           // underlines.
>
> If we support tab stop settings other than at every 8 character cells,
> we might be the first Unix application that ever has.  termcap and
> terminfo took a long time to show up; habits and attitudes, if not code,
> had fossilized by then.  ncurses's tabs(1) man page is informative.

True, but supporting other tabs stops isn’t much work for us.   
This comment, though, isn’t about 8-cell or other tab stops:  It’s 
about how to underline tabs.

Here is the content of a test file “underline.less” I wrote:

        Special behavior for underlined tabs:
        _      a
        
        And with multiple underlines:
        ________        a
        
        And with more than eight underlines:
        
        __________    a

(Surely your email viewer shows you backspace characters?)

View with less and notice how all three lines with tabs look the 
same:

         ̲ ̲ ̲ ̲ ̲ ̲ ̲ ̲a

(Surely your email viewer handles U+0332 COMBINING LOW LINE well?)

The idea is to do the underlining in a way that works for 
typewriters as well, in contrast to what happens right now.   
A typewriter would print the first line with a tab of 
underline.less as

         ̲       a

and that is what would happen with groff right now.

> […]
> > +    // TODO(humm): Do better error handling.
> > +    int err;
> > +    setupterm(terminal_name, 1, terminal_name != NULL ? NULL : &err);
>
> I don't get why the ternary expression here.  setupterm _does_ have a
> return value.  It returns the integer-valued macros `OK` or `ERR`.  (I
> said curses was a late development in Unix, but it was early enough to
> pull off this astounding feat of name space pollution.)

The idea was to let Curses do the dying if we explicitly specify a terminal 
type we want to use via -T but fall back hardcoded SGR control 
sequences if we don’t use -T and Curses fails.

> Essentially a lot of terminfo/curses functions return Booleans.  But
> (originally) this was K&R C--everything's an int and range checks are
> for [censored].  If you want Booleans, put them into bitfields so your
> machine's shift and rotate instructions get some exercise.
>
> ncurses's terminfo(3) page says that `err` can be populated with the
> values 1, 0, or -1, _all of which indicate errors_.
>
> So we want something like
>
> +    int tistatus = setupterm(terminal_name, 1, &err);
> +    if (tistatus != OK) {
> +      switch(err) {
> +      case -1:
> +        // Can't find terminal database, fall back to something.
> +        break;
> +      case 0:
> +        // Terminal has useless (generic) capability set, fall back to
> +        // something.
> +        break;
> +      case 1:
> +        // Terminal is "hardcopy"--NO PROBLEM!  Feed the overstrikers
> +        // their delicious gruel!
> +        break;
> +      default:
> +        // Fatal error: we don't understand what the library is trying
> +        // to tell us.
> +        break;
> +      }
>
> This puts another chit on the "keep only one terminal output driver"
> square of our architectural bingo sheet.

Quoting X/Open Curses Issue 7:

> If errret is not null, the integer it points to is set to one of 
> the following values to report the function outcome:
>
>  −1  The terminfo database was not found (function fails).
>
>   0  The entry for the terminal was not found in terminfo 
>      (function fails).
>
>   1  Success.
>
> If setupterm( )  detects an error and errret is a null pointer, 
> the setupterm( )  function writes a diagnostic message and exits.

This doesn’t sound like setupterm can fail with err == 1.   
Ncurses seems to disagree and notes a check of the hc capability, 
which is provided by X/Open Curses, but doesn’t affect the 
operation of the terminfo functions there.

Since ncurses does return 1 on error, I’ll add a check for that.   
Will do something like what you provided in v2.

> > +    if (err == 1) {
> > +      // TODO(humm): Do error handling for the capabilities.  Can they
> > +      // be null pointers?  Can they be (char*)-1?  Should we fall back
> > +      // to underline if italic is not available and the like?  Should
> > +      // we ignore attributes we can't fulfill, or should we abort?
>
> Terminfo capabilities come 3 types: Booleans ("flags"), integers
> ("numbers"), and strings.
>
> At this point I'll just quote the man page.
>
> tput(3):
>   Terminal Capability Functions
>     The tigetflag, tigetnum and tigetstr routines return the value of
>     the capability corresponding to the terminfo capname passed to
>     them, such as xenl.  The capname for each capability is given in
>     the table column entitled capname code in the capabilities section
>     of terminfo(5).
>
>     These routines return special values to denote errors.
>
>     The tigetflag routine returns
>
>     -1   if capname is not a boolean capability, or
>
>     0    if it is canceled or absent from the terminal description.
>
>     The tigetnum routine returns
>
>     -2   if capname is not a numeric capability, or
>
>     -1   if it is canceled or absent from the terminal description.
>
>     The tigetstr routine returns
>
>     (char *)-1
>          if capname is not a string capability, or
>
>     0    if it is canceled or absent from the terminal description.
>
> (This is old-school C.  We never inquire about the type of '0'.  In
> other words, I don't get why this last item isn't spelled "NULL", or
> '(char *)0') when its predecessor was documented with a cast.  :-|

Because there’s no reason to do that.   The integer constant 
expression 0 can be implicitly converted to a null pointer of any 
pointer type.   Old school C might be to drop the first cast; 
what’s there is perfectly fine modern C.

> So, re-quoting your questions:
>
> > +      // TODO(humm): Do error handling for the capabilities.  Can they
> > +      // be null pointers?
>
> Yes, for tigetstr().

But we don’t use tigetstr, we use the globals.   Can they be null 
pointers?   (Yes, they can.)

> >        // Can they be (char*)-1?
>
> Oh yes, and it's super bad news if it is.  It means someone's terminal
> description is incomprehensible.  That's a fatal().

But can they be (chat*)-1 for the globals as well?   (Probably, 
although it’s not trivial to get it to do that.   tic seems to 
verbosely ignore fields with the wrong type.)

> >        // Should we fall back
> > +      // to underline if italic is not available and the like?
>
> I could go either way on this.  I lean toward "no, you get only what you
> ask for".  But we can throw a warning.
>
> >        // Should
> > +      // we ignore attributes we can't fulfill, or should we abort?
>
> Same.  I wouldn't abort for this.

Alright:  No fallback, abort on missing capability.

> > +      use_terminfo = true;
> > +      sgr_bold = enter_bold_mode;
> > +      sgr_italic = enter_italics_mode;
> > +      sgr_underline = enter_underline_mode;
> > +      sgr_reverse = enter_reverse_mode;
> > +      sgr_exit_attributes = exit_attribute_mode;
> > +      tab_width = init_tabs;
> > +    }
>
> I guess these variables need renaming, since, conceivably, they might
> not have anything to do with ANSI X3.4/ISO 6429/ECMA-48's "SGR" at all.
> Nor do we care, if terminfo can get us valid and effectual information.

ack

> > -  while ((c = getopt_long(argc, argv, "bBcdfF:hiI:oruUv", long_options, NULL))
> > +  while ((c = getopt_long(argc, argv, "bBcdfF:hiI:orT:uUv", long_options, 
> > NULL))
> [...]
> > +    case 'T':
> > +      terminal_name = optarg;
> > +      break;
> [...]
> > -"usage: %s [-bBcdfhioruU] [-F font-directory] [file ...]\n"
> > +"usage: %s [-bBcdfhioruU] [-F font-directory] [-T term] [file ...]\n"
>
> Not needed if we don't add `-T` at all.
>
> Whew!  Plenty of food for thought here.  Thanks for undertaking this!

will send v2 soon-ish I guess