groff
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


From: G. Branden Robinson
Subject: Re: [PATCH v2] [grotty]: Use terminfo.
Date: Fri, 1 Sep 2023 12:47:58 -0500

At 2023-08-30T20:54:09+0000, Lennart Jablonka wrote:
> > Subject: [PATCH v2] [grotty]: Use terminfo.
> 
> Branden: ENQ

Indeed!

At 2023-08-21T21:40:18+0000, Lennart Jablonka wrote:
> This has nothing at all to do with making it easier to customize the
> look of man pages.
> 
> Hyperlinks are still marked up using OSC 8, hoping that whatever
> terminal sits in front of the user interprets or ignores those.
> Terminfo naturally doesn’t have a capability for hyperlinks.
> 
> Autoconf is not informed about the dependence on Curses; we simply link
> with -lcurses, as that is what X/Open Curses specifies.

I think I will want to stress here that only the terminfo bits are used.

> The suggested option to less is changed to -r, which enables passing
> through single control characters, because some terminal descriptions
> use those; I noticed that tmux’s sgr0 contains ^O.
> 
> I’m too lazy to find another name for lines_.
> ---
> v1 → v2:
> 
>   - hardcoded escape sequences are eliminated, except for OSC 8

I asked Thomas Dickey if he meant that "Hls" as used by tmux(1) is an
example of user_caps(5), but it seems I'm expected to use my own fishing
pole here.[1]

We can deal with that later, in any event.

>   - variables are renamed appropriately
>   - update_attributes is split in two; the new update_attributes’s
>     parameter list is not gigantic, I hope
>   - errors to do with setupterm and the capabilities are handled
>     properly
> 
> and more as seen in the interdiff against v1:
> 
>   diff --git a/INSTALL.extra b/INSTALL.extra
>   index 3527b1929..5ef903c77 100644
>   --- a/INSTALL.extra
>   +++ b/INSTALL.extra
>   @@ -122,7 +122,7 @@ 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

Thanks.

>    automatically used by 'preconv'.  Discovery of the 'uchardet' library
>    requires the 'pkg-config' program to be installed on your system, as
>    well as the library's C header files--on a package-based host system,
>   diff --git a/NEWS b/NEWS
>   index 98be590dd..87990f487 100644
>   --- a/NEWS
>   +++ b/NEWS
>   @@ -94,7 +94,8 @@ grotty
>    ------
>    
>    o The terminfo library is now used for applying text attributes (like
>   -  italic, bold, and colors).
>   +  italic, bold, and colors).  You now need Curses to run grotty.  The
>   +  legacy overstriking output format remains.

I'll recast this, but yes.

I feel sure we're going to need that last sentence to avoid a panic.

I'm a little uneasy with the sequencing this change imposes on
bootstrapping scenarios.  It surely was nice to not have a terminfo
dependency.  (Yes, even though it's my idea to add one.)  On the other
hands:

* Every part of groff you need to render a man page except the macro
  package itself is in C++, though I suppose that doesn't mitigate much
  with modern, freely-licensed compilers whose C and C++ support are in
  parity.

* I hate to screw embedded systems developers who have enough headaches
  as it is.  Even if they never _ship_ man pages in what they flash to
  ROM, and few will try to render man pages inside the embedded dev
  environment, a PDP-11 is embedded by today's standards, and Bell Labs
  has their man pages even in that cramped situation.

* You probably _should_ be bringing up a terminal abstraction library in
  early days.  Who knows what kind of wack emulator you'll be running
  over the serial port?  You save yourself grief by getting terminfo
  going early.

* In desperate straits, even without terminfo, you can likely render
  enough of a man page to get by with "groff -a".

>   diff --git a/src/devices/grotty/grotty.1.man 
> b/src/devices/grotty/grotty.1.man
>   index 43d9d7ffd..6fbd70612 100644
>   --- a/src/devices/grotty/grotty.1.man
>   +++ b/src/devices/grotty/grotty.1.man
>   @@ -56,8 +56,6 @@ output driver for typewriter-like (terminal) devices
>    .RB [ \-i \||\| \-r ]
>    .RB [ \-F\~\c
>    .IR font-directory ]
>   -.RB [ \-T\~\c
>   -.IR term ]
>    .RI [ file\~ .\|.\|.]
>    .YS
>    .
>   @@ -66,8 +64,6 @@ output driver for typewriter-like (terminal) devices
>    .RB [ \-bBdfhouU ]
>    .RB [ \-F\~\c
>    .IR font-directory ]
>   -.RB [ \-T\~\c
>   -.IR term ]
>    .RI [ file\~ .\|.\|.]
>    .YS

Ah, simplicity.

>   @@ -140,8 +136,8 @@ Output is written to the standard output stream.
>    By default,
>    .I grotty
>    uses
>   -.MR terminfo 5
>   -to change text attributes
>   +.MR terminfo 3
>   +capabilities to change text attributes

I managed to speak with marbles in my mouth in my previous review.  This
terminfo(3) reference does, in fact, work on my system.  You wouldn't
guess it from the sources--the _real_ sources, in Thomas Dickey's
repository (equivalently, ncurses-snapshots GitHub repo).  The `TH` line
gets rewritten from the horrendous "curs_terminfo" as part of
build/installation.  And I was _not_ trying to suggest that we `MR` to
"putp(3)" except when referring to that specific function.

So, yeah, sometimes it is best to ignore me.  :-|

>   -If no
>   -.I TERM
>   -environment variable is set
>   -or terminfo encounters a different error,
>   -.I grotty
>   -emits hardcoded SGR escape sequences
>   -(from ISO\~6429,
>   -popularly called \[lq]ANSI escapes\[rq]).
>   -.
[...]
>    .
>   +If the terminal description provides no way of changing the text
>   +attributes but does declare that overstriking characters is possible,
>   +.I grotty
>   +will use the legacy output format.

Good deal.

>    .\" ====================================================================
>   -.SS "SGR and OSC support in pagers"
>   +.SS "Control character support in pagers"
>    .\" ====================================================================

I think I may tune this to something like "Interaction with pagers".

>   +.\" TODO: using less -r means putting untrusted input on the terminal;
>   +.\" ponder this

I'll think about what advice I can offer here.  Some terminal types will
require "less -r", others--the better ones--will do fine with "less -R".

>   -SGR escape sequences are not emitted;
>   +Terminfo capability strings are not emitted;
[...]
>   -SGR and OSC escape sequences are not emitted.
>   +Terminfo capability strings and OSC escape sequences are not emitted.

Feels good to climb Mount Abstraction.

>   -Tabs are assumed to be set every 8 columns.
>   +Unless the terminal description says otherwise,
>   +tabs are assumed to be set every 8 columns.

Good catch.

>   -with the SGR attribute for italic text
>   +with the terminfo capability for italic text
[...]
>   @@ -521,6 +516,8 @@ Ignored if
>    .B \-c
>    is also specified.
>    .
>   +Disables the overstriking fallback for hardcopy terminals.
>   +.
>    .
[...]
>   -with the SGR attribute for reverse video text
>   +with the terminfo capability for reverse video text
>    rather than underlined text.

These are all good.

>   -.BI \-T\~ term
>   -Read description for terminal type
>   -.I term
>   -and fail if
>   -.I terminfo
>   -encounters and error.
>   -By default,
>   -.I grotty
>   -takes the terminal type from the environment variable
>   -.I TERM
>   -and falls back to SGR escape sequences
>   -if that is not specified or there is an error.
>   -.
>   -.
>   -.TP

Resimplification, continued.

>    .TP
>    .I TERM
>   -The terminal type for
>   -.I terminfo.
>   +The terminal type interpreted by the
>   +.MR terminfo 3
>   +library.

Yup.  Not our problem.

>   diff --git a/src/devices/grotty/tty.cpp b/src/devices/grotty/tty.cpp
>   index 9df35d8cd..bbca37ee3 100644
>   --- a/src/devices/grotty/tty.cpp
>   +++ b/src/devices/grotty/tty.cpp
>   @@ -21,6 +21,7 @@ along with this program.  If not, see 
> <http://www.gnu.org/licenses/>. */
>    #include "device.h"
>    #include "ptable.h"
>    
>   +#include <curses.h>

Are you sure?

Looking at:

https://pubs.opengroup.org/onlinepubs/7908799/xcurses/putp.html
https://pubs.opengroup.org/onlinepubs/7908799/xcurses/setupterm.html
https://pubs.opengroup.org/onlinepubs/7908799/xcurses/tparm.html

I see no suggestion that we need to #include the curses header if we're
programming to the _standard_ API.  Does your environment require this?

>    #include <term.h>
>    
>    typedef signed char schar;
>   @@ -49,15 +50,12 @@ static bool want_emboldening_by_overstriking = true;
>    static bool do_bold;
>    static bool want_italics_by_underlining = true;
>    static bool do_underline;
>   -static bool want_glyph_composition_by_overstriking = true;
>   +static bool accept_glyph_composition_by_overstriking = true;
>   +static bool do_glyph_composition_by_overstriking;
>    static bool allow_drawing_commands = true;
>   -static bool want_sgr_italics = false;
>   -static bool do_sgr_italics;
>   +static bool want_real_italics = false;
>    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;

Ok.

>    #ifndef IS_EBCDIC_HOST
>   -#define CSI "\033["
>    #define OSC8 "\033]8"
>    #define ST "\033\\"
>    #else
>   -#define CSI "\047["
>    #define OSC8 "\047]8"
>    #define ST "\047\\"
>    #endif
>    
>   -// SGR handling (ISO 6429)
>   -// bold and reverse can be unset separately using ISO 6429, but not
>   -// with terminfo
>   -static const char *sgr_bold = CSI "1m";
>   -static const char *sgr_italic = CSI "3m";
>   -static const char *sgr_underline = CSI "4m";
>   -static const char *sgr_reverse = CSI "7m";
>   -// 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
>   -static const char *sgr_exit_attributes = CSI "0m";
>   +static char *enter_italics_or_the_like_mode;

That's an awkward name, but it's an awkward problem.

>   -  void update_attributes(bool, bool, schar, schar, output_character, int);
>   +  void overstrike(bool, bool, output_character, int);
>   +  void update_attributes(bool, bool, schar, schar);

Okay, my wails soften in intensity with 4 arguments versus 6.

>      schar color_to_idx(color *);
>      void add_char(output_character, int, int, int, color *, color *,
>               unsigned char);
>   @@ -204,6 +191,7 @@ public:
>      void set_char(glyph *, font *, const environment *, int, const char *);
>      void draw(int, int *, int, const environment *);
>      void special(char *, const environment *, char);
>   +  void change_color(const environment * const);

We might call this "change_stroke_color".  I tend to dislike
nomenclatures where an unmodified noun is coupled with a modified one;
it's harder to infer what the scope/meaning of the unmodified noun is.

>   -/* 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. */
>   -void tty_printer::update_attributes(bool underline, bool bold,
>   -                               schar fore_idx, schar back_idx,
>   -                               output_character c, int w)
>   +// Both ovrestrike and update_attributes need be called for text
>   +// attributes; only one of them will take effect.  Call overstrike for
>   +// every glyph.  The update_attributes function, too, can be called for
>   +// every glyph, as it's a noop for the same attributes.

I'll need to study the code more to make full sense of this comment.

>   +
>   +void tty_printer::overstrike(bool underline, bool bold,
>   +                        output_character c, int w)
>    {
>   -  if (use_overstriking_drawing_scheme) {
>   -    if (underline) {
>   -      if (!w)
>   -   warning("can't underline zero-width character");
>   -      else {
>   -   putchar('_');
>   -   putchar('\b');
>   -      }
>   -    }
>   -    if (bold) {
>   -      if (!w)
>   -   warning("can't print zero-width character in bold");
>   -      else {
>   -   put_char(c);
>   -   putchar('\b');
>   -      }
>   +  if (!use_overstriking_drawing_scheme)
>   +    return;
>   +
>   +  if (underline) {
>   +    if (!w)
>   +      warning("can't underline zero-width character");
>   +    else {
>   +      putchar('_');
>   +      putchar('\b');
>        }
>      }
>   -  else {
>   -    if (is_underlining && !underline
>   -   || is_boldfacing && !bold
>   -   || curr_fore_idx != DEFAULT_COLOR_IDX && fore_idx == DEFAULT_COLOR_IDX
>   -   || curr_back_idx != DEFAULT_COLOR_IDX && back_idx == DEFAULT_COLOR_IDX) 
> {
>   -      putp(sgr_exit_attributes);
>   -      is_underlining = is_boldfacing = false;
>   -      curr_fore_idx = curr_back_idx = DEFAULT_COLOR_IDX;
>   +  if (bold) {
>   +    if (!w)
>   +      warning("can't print zero-width character in bold");
>   +    else {
>   +      put_char(c);
>   +      putchar('\b');
>        }
>   +  }
>   +}
>    
>   -    if (!is_underlining && underline) {
>   -      if (do_sgr_italics)
>   -   putp(sgr_italic);
>   -      else if (do_reverse_video)
>   -   putp(sgr_reverse);
>   -      else
>   -   putp(sgr_underline);
>   -      is_underlining = true;
>   -    }
>   +void tty_printer::update_attributes(bool underline, bool bold,
>   +                               schar fore_idx, schar back_idx)
>   +{
>   +  if (use_overstriking_drawing_scheme)
>   +    return;
>    
>   -    if (!is_boldfacing && bold) {
>   -      putp(sgr_bold);
>   -      is_boldfacing = true;
>   -    }
>   +  if (is_underlining && !underline
>   +      || is_boldfacing && !bold
>   +      || curr_fore_idx != DEFAULT_COLOR_IDX && fore_idx == 
> DEFAULT_COLOR_IDX
>   +      || curr_back_idx != DEFAULT_COLOR_IDX && back_idx == 
> DEFAULT_COLOR_IDX) {
>   +    putp(exit_attribute_mode);
>   +    is_underlining = is_boldfacing = false;
>   +    curr_fore_idx = curr_back_idx = DEFAULT_COLOR_IDX;
>   +  }
>    
>   -    if (curr_fore_idx != fore_idx) {
>   -      if (use_terminfo)
>   -   putp(tparm(set_a_foreground, fore_idx, 0, 0, 0, 0, 0, 0, 0, 0));
>   -      else {
>   -   fputs(CSI, stdout);
>   -   putchar('3');
>   -   putchar(fore_idx + '0');
>   -   putchar('m');
>   -      }
>   -      curr_fore_idx = fore_idx;
>   -    }
>   +  if (!is_underlining && underline) {
>   +    putp(enter_italics_or_the_like_mode);
>   +    is_underlining = true;
>   +  }
>    
>   -    if (curr_back_idx != back_idx) {
>   -      if (use_terminfo)
>   -   putp(tparm(set_a_background, back_idx, 0, 0, 0, 0, 0, 0, 0, 0));
>   -      else {
>   -   fputs(CSI, stdout);
>   -   putchar('4');
>   -   putchar(back_idx + '0');
>   -   putchar('m');
>   -      }
>   -      curr_back_idx = back_idx;
>   -    }
>   +  if (!is_boldfacing && bold) {
>   +    putp(enter_bold_mode);
>   +    is_boldfacing = true;
>   +  }
>   +
>   +  if (curr_fore_idx != fore_idx) {
>   +    putp(tparm(set_a_foreground, fore_idx, 0, 0, 0, 0, 0, 0, 0, 0));
>   +    curr_fore_idx = fore_idx;
>   +  }
>   +
>   +  if (curr_back_idx != back_idx) {
>   +    putp(tparm(set_a_background, back_idx, 0, 0, 0, 0, 0, 0, 0, 0));
>   +    curr_back_idx = back_idx;

Good grief.  Terminfo sees your 6-argument monstrosity and raises you.
What were people thinking?  It's SO MUCH CHEAPER to pass 9 longs on the
stack than a pointer to a struct (which a good compiler will find a
register for anyway).

Terminfo _was_ an improvement on termcap, but stuff like this reminds me
to moderate my praise.

>   @@ -496,8 +472,6 @@ 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?
>    void tty_printer::special_link(const char *arg, const environment *env)
>    {
>      static bool is_link_active = false;
>   @@ -562,6 +536,11 @@ void tty_printer::special_link(const char *arg, const 
> environment *env)
>        simple_add_char(*s, env);
>    }
>    
>   +void tty_printer::change_color(const environment * const env)
>   +{
>   +  add_char(0, 0, env->hpos, env->vpos, env->col, env->fill, COLOR_CHANGE);
>   +}
>   +

All righty.

>    void tty_printer::change_fill_color(const environment * const env)
>    {
>      add_char(0, 0, env->hpos, env->vpos, env->col, env->fill, COLOR_CHANGE);
>   @@ -798,7 +777,7 @@ void tty_printer::end_page(int page_length)
>         nextp->code = p->code;
>         continue;
>       }
>   -   if (!want_glyph_composition_by_overstriking)
>   +   if (!do_glyph_composition_by_overstriking)
>         continue;
>          }
>          if (hpos > p->hpos) {
>   @@ -807,7 +786,9 @@ void tty_printer::end_page(int page_length)
>         hpos--;
>       } while (hpos > p->hpos);
>          }
>   -      else {
>   +      else if (p->hpos > hpos) {
>   +   update_attributes(is_continuously_underlining, is_boldfacing,
>   +                     curr_fore_idx, curr_back_idx);
>       if (want_horizontal_tabs) {
>         for (;;) {
>           int next_tab_pos = ((hpos + tab_width) / tab_width) * tab_width;
>   @@ -816,36 +797,32 @@ void tty_printer::end_page(int page_length)
>           // 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
>   +       // cells by less.  We could pass overstrike the width
>           // the actual tab will have and let it emit multiple
>           // underlines.
>   -       update_attributes(is_continuously_underlining, is_boldfacing,
>   -                         curr_fore_idx, curr_back_idx, '\t', p->w);
>   +       overstrike(is_continuously_underlining, false, '\t', p->w);
>           putchar('\t');
>           hpos = next_tab_pos;
>         }
>       }
>       for (; hpos < p->hpos; hpos++) {
>   -     update_attributes(is_continuously_underlining, is_boldfacing,
>   -                       curr_fore_idx, curr_back_idx, ' ', p->w);
>   +     overstrike(is_continuously_underlining, false, ' ', p->w);
>         putchar(' ');
>       }
>          }
>          assert(hpos == p->hpos);
>          if (p->mode & COLOR_CHANGE) {
>   -   // test because we don't have any (c,w) to pass and won't do
>   -   // colors if those would be used
>   -   if (!use_overstriking_drawing_scheme)
>   -     update_attributes(is_underlining, is_boldfacing,
>   -                       p->fore_color_idx, p->back_color_idx, 0, 0);
>   +   update_attributes(is_underlining, is_boldfacing,
>   +                     p->fore_color_idx, p->back_color_idx);
>       continue;
>          }
>   +      overstrike(p->mode & UNDERLINE_MODE, p->mode & BOLD_MODE, p->code, 
> p->w);
>          update_attributes(p->mode & UNDERLINE_MODE, p->mode & BOLD_MODE,
>   -                   p->fore_color_idx, p->back_color_idx, p->code, p->w);
>   +                   p->fore_color_idx, p->back_color_idx);
>          put_char(p->code);
>          hpos += p->w / font::hor;
>        }
>   -    update_attributes(false, false, DEFAULT_COLOR_IDX, DEFAULT_COLOR_IDX, 
> 0, 0);
>   +    update_attributes(false, false, DEFAULT_COLOR_IDX, DEFAULT_COLOR_IDX);
>        putchar('\n');
>      }
>      if (want_form_feeds) {
>   @@ -871,34 +848,82 @@ printer *make_printer()
>    static void update_options()
>    {
>      if (use_overstriking_drawing_scheme) {
>   -    do_sgr_italics = false;
>   -    do_reverse_video = false;
>        bold_underline_mode = bold_underline_mode_option;
>        do_bold = want_emboldening_by_overstriking;
>        do_underline = want_italics_by_underlining;
>   +    do_glyph_composition_by_overstriking =
>   +      accept_glyph_composition_by_overstriking;
>   +    return;
>      }
>   -  else {
>   -    // TODO(humm): Do better error handling.
>   -    int err;
>   -    setupterm(terminal_name, 1, terminal_name != NULL ? NULL : &err);
>   -    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?
>   -      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;
>   +
>   +  bold_underline_mode = BOLD_MODE|UNDERLINE_MODE;
>   +  do_bold = true;
>   +  do_underline = true;
>   +
>   +  int err;
>   +  if (setupterm(NULL, 1, &err) == ERR)
>   +    switch (err) {
>   +      // It's a little sad that we can't use Curses's own error
>   +      // strings, just to be able to handle hardcopy terminals,
>   +      // only because ncurses behaves in a non-standard manner when
>   +      // stumbling upon the hc capability.
>   +    case -1:
>   +      fatal("terminfo database not found");
>   +    case 0:
>   +      fatal("terminal description not found");
>   +    case 1: // hardcopy terminal (non standard) / success (standard)
>   +      // We check for over_strike anyway.  Ncurses is nice enough
>   +      // to load the capabilities anyway.
>   +      ;

I would explicitly comment that we're doing nothing here, on purpose.

>        }
>   -    do_sgr_italics = want_sgr_italics;
>   -    do_reverse_video = want_reverse_video_for_italics;
>   -    bold_underline_mode = BOLD_MODE|UNDERLINE_MODE;
>   -    do_bold = true;
>   -    do_underline = true;
>   +
>   +  if (want_real_italics)
>   +    enter_italics_or_the_like_mode = enter_italics_mode;
>   +  else if (want_reverse_video_for_italics)
>   +    enter_italics_or_the_like_mode = enter_reverse_mode;
>   +  else
>   +    enter_italics_or_the_like_mode = enter_underline_mode;
>   +
>   +  if (tab_width == -2)
>   +    fatal("bad it (init_tabs) capability");
>   +
>   +  tab_width = init_tabs != -1 ? init_tabs : 8;
>   +
>   +  if (over_strike == -1)
>   +    fatal("bad os (over_strike) capability");

I'll cover these diagnostics with my own scent.

>   +
>   +  do_glyph_composition_by_overstriking =
>   +    accept_glyph_composition_by_overstriking && over_strike;
>   +
>   +  static const struct {
>   +    char *cap;
>   +    char const *nullmsg;
>   +    char const *negonemsg;

Oh.  "neg(ative) one message".  This took a bit of decoding.

>   +  } string_caps[] = {
>   +    // all the string capabilities we use in the program
>   +    {enter_bold_mode, "can't make text bold", "bad bold capability"},
>   +    {enter_italics_or_the_like_mode, "can't make text italic (or the 
> like)",
>   +      "bad italics (or the like) capability"},
>   +    {exit_attribute_mode, "can't disable text attributes",
>   +      "bad sgr0 (exit attributes) capability"},
>   +    {set_a_foreground, "can't colorize text",
>   +      "bad setaf (foreground color) capability"},
>   +    {set_a_background, "can't colorize text",
>   +      "bad setab (background color) capability"},
>   +  };

Similar.

Similar regarding scent.  Also, FWIW, the "a" means "ANSI"; it's an
explicit reference to ANSI X3.4, later superseded by ECMA-48 and ISO
6429.

>   +  for (int i = 0; i < sizeof string_caps / sizeof *string_caps; ++i) {

Sound the klaxon!  :P  I'll port this to our new friend array_length().

>   +    if (string_caps[i].cap == NULL) {
>   +      if (over_strike && !want_real_italics && 
> !want_reverse_video_for_italics) {
>   +   use_overstriking_drawing_scheme = true;
>   +   update_options();
>   +   return;
>   +      }
>   +      else
>   +   fatal(string_caps[i].nullmsg);
>   +    }
>   +    else if (string_caps[i].cap == (char *)-1)
>   +      fatal(string_caps[i].negonemsg);
>      }
>    }
>    
>   @@ -916,7 +941,7 @@ int main(int argc, char **argv)
>        { "version", no_argument, 0, 'v' },
>        { NULL, 0, 0, 0 }
>      };
>   -  while ((c = getopt_long(argc, argv, "bBcdfF:hiI:orT:uUv", long_options, 
> NULL))
>   +  while ((c = getopt_long(argc, argv, "bBcdfF:hiI:oruUv", long_options, 
> NULL))
>        != EOF)
>        switch(c) {
>        case 'v':
>   @@ -925,7 +950,7 @@ int main(int argc, char **argv)
>          break;
>        case 'i':
>          // Use italic font instead of underlining.
>   -      want_sgr_italics = true;
>   +      want_real_italics = true;
>          break;
>        case 'I':
>          // ignore include search path
>   @@ -944,7 +969,7 @@ int main(int argc, char **argv)
>          break;
>        case 'o':
>          // Do not overstrike (other than emboldening and underlining).
>   -      want_glyph_composition_by_overstriking = false;
>   +      accept_glyph_composition_by_overstriking = false;
>          break;
>        case 'r':
>          // Use reverse mode instead of underlining.
>   @@ -972,9 +997,6 @@ int main(int argc, char **argv)
>          // Ignore \D commands.
>          allow_drawing_commands = false;
>          break;
>   -    case 'T':
>   -      terminal_name = optarg;
>   -      break;
>        case CHAR_MAX + 1: // --help
>          usage(stdout);
>          break;
>   @@ -998,7 +1020,7 @@ int main(int argc, char **argv)
>    static void usage(FILE *stream)
>    {
>      fprintf(stream,
>   -"usage: %s [-bBcdfhioruU] [-F font-directory] [-T term] [file ...]\n"
>   +"usage: %s [-bBcdfhioruU] [-F font-directory] [file ...]\n"
>    "usage: %s {-v | --version}\n"
>    "usage: %s --help\n",
>         program_name, program_name, program_name);
> 
>  INSTALL.extra                   |   3 +
>  NEWS                            |  10 +
>  src/devices/grotty/grotty.1.man |  40 ++--
>  src/devices/grotty/grotty.am    |   3 +-
>  src/devices/grotty/tty.cpp      | 333 ++++++++++++++++----------------
>  5 files changed, 209 insertions(+), 180 deletions(-)

The rest of that looks fine; a return to the status quo getopt()-wise.

I'm happy with this and will start merging.  Any tweaks I make will be
in a subsequent commit, unless merging it as-is breaks the build/tests,
in which case I'll let you know.

ACK.

Regards,
Branden

[1] https://lists.gnu.org/archive/html/bug-ncurses/2023-08/msg00020.html

Attachment: signature.asc
Description: PGP signature


reply via email to

[Prev in Thread] Current Thread [Next in Thread]