RE: [PATCH RFC] expand: add support for incremental tab stops

[Keller, Jacob E]

> -----Original Message-----
> From: Pádraig Brady [mailto:address@hidden]
> Sent: Tuesday, March 28, 2017 7:44 PM
> To: Keller, Jacob E <address@hidden>; address@hidden
> Subject: Re: [PATCH RFC] expand: add support for incremental tab stops
> 
> On 28/03/17 15:49, Jacob Keller wrote:
> > When viewing output of a diff program in unified format, the output
> > comes prefixed by a single column to show which lines are removed,
> > added, or just context. This single column can impact the display of tab
> > characters compared to how they would display if the lines were not
> > prefixed. For example, diff might show
> >
> >  a context line
> >  another context line
> > +a new line
> > -an old line
> >
> > But if there were tabs, it could incorrectly display these due to the
> > change in the column widths. Add a new option to the expand tab list
> > which will allow us to specify how to expand tabs in this scenario.
> >
> > Currently, expand has the option of specifying hard coded tab stop
> > lists, as well as extending this with a '/' notation which indicates to
> > continue with tab stops at that multiple when the hard coded list is
> > used up.
> >
> > Add a new variant, '+' prefix to the last tab, which indicates to keep
> > adding tabs at that increment after the last hard coded tab stop.
> >
> > Thus, --tabs="1,+8" is equivalent to --tabs="1,9,17,25,33,41,49,57,..."
> >
> > This is useful because we can now pipe the output of a diff command and
> > properly expand the tabs as the user would expect, keeping all tabs
> > properly aligned so that the output does not appear munged.
> >
> > It may be worth adding a separate switch for this mode "--diff" or
> > similar so that users are more likely to discover the capability, but
> > this patch does not implement such an option at this time.
> > ---
> >  src/expand-common.c  | 63
> ++++++++++++++++++++++++++++++++++++++++++++++++++--
> >  src/expand.c         |  5 +++++
> >  tests/misc/expand.pl | 14 ++++++++++++
> >  3 files changed, 80 insertions(+), 2 deletions(-)
> >
> > diff --git a/src/expand-common.c b/src/expand-common.c
> > index e1b549fab101..7b7902284e33 100644
> > --- a/src/expand-common.c
> > +++ b/src/expand-common.c
> > @@ -38,6 +38,9 @@ static uintmax_t tab_size = 0;
> >  /* If nonzero, the size of all tab stops after the last specifed.  */
> >  static uintmax_t extend_size = 0;
> >
> > +/* If nonzero, an increment for additional tab stops after the last 
> > specified. */
> > +static uintmax_t increment_size = 0;
> > +
> >  /* The maximum distance between tab stops.  */
> >  size_t max_column_width;
> >
> > @@ -94,7 +97,7 @@ set_extend_size (uintmax_t tabval)
> >  {
> >    bool ok = true;
> >
> > -  if (extend_size)
> > +  if (extend_size || increment_size)
> >      {
> >        error (0, 0,
> >               _("'/' specifier only allowed"
> > @@ -106,6 +109,23 @@ set_extend_size (uintmax_t tabval)
> >    return ok;
> >  }
> >
> > +static bool
> > +set_increment_size(uintmax_t tabval)
> > +{
> > +  bool ok = true;
> > +
> > +  if (extend_size || increment_size)
> > +    {
> > +      error (0,0,
> > +             _("'+' specifier only allowed"
> > +               " with the last value"));
> > +      ok = false;
> > +    }
> > +  increment_size = tabval;
> > +
> > +  return ok;
> > +}
> > +
> >  /* Add the comma or blank separated list of tab stops STOPS
> >     to the list of tab stops.  */
> >  extern void
> > @@ -114,6 +134,7 @@ parse_tab_stops (char const *stops)
> >    bool have_tabval = false;
> >    uintmax_t tabval = 0;
> >    bool extend_tabval = false;
> > +  bool increment_tabval = false;
> >    char const *num_start = NULL;
> >    bool ok = true;
> >
> > @@ -131,6 +152,14 @@ parse_tab_stops (char const *stops)
> >                        break;
> >                      }
> >                  }
> > +              else if (increment_tabval)
> > +                {
> > +                  if (! set_increment_size (tabval))
> > +                    {
> > +                      ok = false;
> > +                      break;
> > +                    }
> > +                }
> >                else
> >                  add_tab_stop (tabval);
> >              }
> > @@ -144,8 +173,28 @@ parse_tab_stops (char const *stops)
> >                       quote (stops));
> >                ok = false;
> >              }
> > +          else if (increment_tabval)
> > +            {
> > +              error (0, 0, _("'/' specifier mutually exclusive with '+'"));
> > +              ok = false;
> > +            }
> >            extend_tabval = true;
> >          }
> > +      else if (*stops == '+')
> > +        {
> > +          if (have_tabval)
> > +            {
> > +              error (0, 0, _("'+' specifier not at start of number: %s"),
> > +                     quote (stops));
> > +              ok = false;
> > +            }
> > +          else if (extend_tabval)
> > +            {
> > +              error (0, 0, _("'+' specifier mutually exclusive with '/'"));
> > +              ok = false;
> > +            }
> > +          increment_tabval = true;
> > +        }
> >        else if (ISDIGIT (*stops))
> >          {
> >            if (!have_tabval)
> > @@ -179,6 +228,8 @@ parse_tab_stops (char const *stops)
> >      {
> >        if (extend_tabval)
> >          ok &= set_extend_size (tabval);
> > +      else if (increment_tabval)
> > +        ok &= set_increment_size (tabval);
> >        else
> >          add_tab_stop (tabval);
> >      }
> > @@ -221,7 +272,7 @@ finalize_tab_stops (void)
> >
> >    if (first_free_tab == 0)
> >      tab_size = max_column_width = extend_size ? extend_size : 8;
> > -  else if (first_free_tab == 1 && ! extend_size)
> > +  else if (first_free_tab == 1 && ! extend_size && ! increment_size)
> >      tab_size = tab_list[0];
> >    else
> >      tab_size = 0;
> > @@ -251,6 +302,14 @@ get_next_tab_column (const uintmax_t column,
> size_t* tab_index,
> >    if (extend_size)
> >      return column + (extend_size - column % extend_size);
> >
> > +  /* incremental last tab - add increment_size to the previous tab stop */
> > +  if (increment_size)
> > +    {
> > +      uintmax_t final_tab = first_free_tab ? tab_list[first_free_tab - 1] 
> > : 0;
> > +
> > +      return column + (increment_size - ((column - final_tab) %
> increment_size));
> > +    }
> > +
> >    *last_tab = true;
> >    return 0;
> >  }
> > diff --git a/src/expand.c b/src/expand.c
> > index 2b7781ca51e2..8067ba237b2c 100644
> > --- a/src/expand.c
> > +++ b/src/expand.c
> > @@ -83,6 +83,11 @@ Convert tabs in each FILE to spaces, writing to standard
> output.\n\
> >  "), stdout);
> >        fputs (_("\
> >    -t, --tabs=LIST     use comma separated list of explicit tab positions\n\
> > +                      the final value can be preceded by either a '/' in\n\
> > +                      which case it indicates additional tab stops at 
> > that\n\
> > +                      multiple, and it can be preceded by a '+' in which\n\
> > +                      case it indicates additional tab stops 
> > incrementing\n\
> > +                      by that value.\n\
> >  "), stdout);
> >        fputs (HELP_OPTION_DESCRIPTION, stdout);
> >        fputs (VERSION_OPTION_DESCRIPTION, stdout);
> > diff --git a/tests/misc/expand.pl b/tests/misc/expand.pl
> > index b04d2e72db6b..88e4b26a3c81 100755
> > --- a/tests/misc/expand.pl
> > +++ b/tests/misc/expand.pl
> > @@ -151,6 +151,20 @@ my @Tests =
> >     ['trail8', '--tabs=1 -t/5', {IN=>"\ta\tb\tc"}, {OUT=>" a   b    c"}],
> >     ['trail9', '--tab=1,2 -t/5',{IN=>"\ta\tb\tc"}, {OUT=>" a   b    c"}],
> >
> > +   # Test incremental trailing '+' feature which specifies that
> > +   # tab stops should continue every increment
> > +   ['increment0', '--tab=1,+8',    {IN=>"+\t\tb\tc"}, {OUT=>"              
> >    b       c"}],
> > +   ['increment1', '--tabs=1,+5',   {IN=>"\ta\tb\tc"}, {OUT=>" a    b    
> > c"}],
> > +   ['increment2', '--tabs=2,+5',   {IN=>"\ta\tb\tc"}, {OUT=>"  a    b    
> > c"}],
> > +   ['increment3', '--tabs=1,2,+5', {IN=>"\ta\tb\tc"}, {OUT=>" a     b    
> > c"}],
> > +   ['increment4', '--tabs=+5',     {IN=>"\ta\tb"},    {OUT=>"     a    
> > b"}],
> > +   ['increment5', '--tabs=++5',    {IN=>"\ta\tb"},    {OUT=>"     a    
> > b"}],
> > +   ['increment6', '--tabs=+,+5',   {IN=>"\ta\tb"},    {OUT=>"     a    
> > b"}],
> > +   ['increment7', '--tabs=,+5',    {IN=>"\ta\tb"},    {OUT=>"     a    
> > b"}],
> > +   ['increment8', '--tabs=1 -t+5', {IN=>"\ta\tb\tc"}, {OUT=>" a    b    
> > c"}],
> > +   ['increment9', '--tab=1,2 -t+5',{IN=>"\ta\tb\tc"}, {OUT=>" a     b    
> > c"}],
> > +
> > +
> >     # Test errors
> >     ['e1', '--tabs="a"', {IN=>''}, {OUT=>''}, {EXIT=>1},
> >      {ERR => "$prog: tab size contains invalid character(s): 'a'\n"}],
> >
> 
> 
> I like this a lot.
> I'll document the `diff one two | expand --tabs=1,+8` example.
> 
> thanks!
> Pádraig

It looks like there may need to be some work on this for handling zero-width 
characters (like ASCI color sequences) so that it can work correctly for 
colored output as well. I'm still looking into it. I'm glad you like it.

Thanks,
Jake