groff-commit
[Top][All Lists]
Advanced

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

[groff] 18/20: [tbl]: Fix Savannah #63640 and #63749.


From: G. Branden Robinson
Subject: [groff] 18/20: [tbl]: Fix Savannah #63640 and #63749.
Date: Fri, 3 Feb 2023 16:47:56 -0500 (EST)

gbranden pushed a commit to branch master
in repository groff.

commit ad70899cb0c4dcf28fc95447ccc33bd8660cf719
Author: G. Branden Robinson <g.branden.robinson@gmail.com>
AuthorDate: Fri Feb 3 12:55:23 2023 -0600

    [tbl]: Fix Savannah #63640 and #63749.
    
    [tbl]: Fix bugs using boxes or vertical rules at table edges on nroff
    devices, particularly when combined with region or column expansion.
    
    * src/preproc/tbl/table.h (class table): Add `GAP_EXPAND` enumeration
      constant.  James Clark seems to have designed GNU tbl carefully to
      avoid distinguishing region expansion from column expansion in a
      categorical way, but I needed a way for formatting-time logic to know
      which was in use.  (Column expansion, the "x" modifier, expands
      columns--i.e., text.  Region expansion expands [or compresses] the
      _gaps_ between columns.)
    
    * src/preproc/tbl/main.cpp (process_options): Set `GAP_EXPAND` flag in
      table if "expand" region option seen.
    
    * src/preproc/tbl/table.cpp: Add new macro `LEFTOVER_FACTOR_REG` to name
      a new roff register for the remainder of gap-expansion space when the
      amount of space available for expansion is divided by the number of
      gaps.
    
      (table::compute_overall_width): If _not_ expanding a table in
      either respect and in nroff mode, reduce line length by 1n for each of
      any left and right border (because the vertical lines eat character
      cells).  This prevents bordered or boxed tables from being overset
      even when they use neither expansion feature.
    
      (table::compute_separation_factor): If gap-expanding a table, store
      any remainder from the division used to compute the separation factor
      into the new `LEFTOVER_FACTOR_REG`.
    
      (table::compute_column_positions): Insert that remainder into the gap
      before the last (rightmost) column of the table.  This _could_ be done
      more elegantly by spreading each en in a symmetric way across a subset
      of the gaps.  (It is necessarily a subset by the pigeonhole
      principle.)  But it didn't seem worth the effort for a feature (region
      expansion) that few users employ.  (Usually what you want is the "x"
      column modifier.)  Alternatively, "forget it, Jake--it's a terminal
      emulator".
    
    * src/preproc/tbl/tbl.am (tbl_XFAIL_TESTS): Remove now-passing tests.
    
    Fixes <https://savannah.gnu.org/bugs/?63640> and
    <https://savannah.gnu.org/bugs/?63749>.
---
 ChangeLog                 | 43 +++++++++++++++++++++++++++++++++++++++++++
 src/preproc/tbl/main.cpp  |  1 +
 src/preproc/tbl/table.cpp | 24 +++++++++++++++++++++++-
 src/preproc/tbl/table.h   |  3 ++-
 src/preproc/tbl/tbl.am    |  4 +---
 5 files changed, 70 insertions(+), 5 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 4ac8a3fe9..ecae94519 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,46 @@
+2023-02-03  G. Branden Robinson <g.branden.robinson@gmail.com>
+
+       [tbl]: Fix bugs using boxes or vertical rules at table edges on
+       nroff devices, particularly when combined with region or column
+       expansion.
+
+       * src/preproc/tbl/table.h (class table): Add `GAP_EXPAND`
+       enumeration constant.  James Clark seems to have designed GNU
+       tbl carefully to avoid distinguishing region expansion from
+       column expansion in a categorical way, but I needed a way for
+       formatting-time logic to know which was in use.  (Column
+       expansion, the "x" modifier, expands columns--i.e., text.
+       Region expansion expands [or compresses] the _gaps_ between
+       columns.)
+       * src/preproc/tbl/main.cpp (process_options): Set `GAP_EXPAND`
+       flag in table if "expand" region option seen.
+       * src/preproc/tbl/table.cpp: Add new macro `LEFTOVER_FACTOR_REG`
+       to name a new roff register for the remainder of gap-expansion
+       space when the amount of space available for expansion is
+       divided by the number of gaps.
+       (table::compute_overall_width): If _not_ expanding a table in
+       either respect and in nroff mode, reduce line length by 1n for
+       each of any left and right border (because the vertical lines
+       eat character cells).  This prevents bordered or boxed tables
+       from being overset even when they use neither expansion feature.
+       (table::compute_separation_factor): If gap-expanding a table,
+       store any remainder from the division used to compute the
+       separation factor into the new `LEFTOVER_FACTOR_REG`.
+       (table::compute_column_positions): Insert that remainder into
+       the gap before the last (rightmost) column of the table.  This
+       _could_ be done more elegantly by spreading each en in a
+       symmetric way across a subset of the gaps.  (It is necessarily a
+       subset by the pigeonhole principle.)  But it didn't seem worth
+       the effort for a feature (region expansion) that few users
+       employ.  (Usually what you want is the "x" column modifier.)
+       Alternatively, "forget it, Jake--it's a terminal emulator".
+
+       * src/preproc/tbl/tbl.am (tbl_XFAIL_TESTS): Remove now-passing
+       tests.
+
+       Fixes <https://savannah.gnu.org/bugs/?63640> and
+       <https://savannah.gnu.org/bugs/?63749>.
+
 2023-02-03  G. Branden Robinson <g.branden.robinson@gmail.com>
 
        * src/preproc/tbl/table.cpp (table::compute_column_positions):
diff --git a/src/preproc/tbl/main.cpp b/src/preproc/tbl/main.cpp
index a3308baa8..db105c217 100644
--- a/src/preproc/tbl/main.cpp
+++ b/src/preproc/tbl/main.cpp
@@ -475,6 +475,7 @@ options *process_options(table_input &in)
       if (arg)
        error("'expand' region option does not take an argument");
       opt->flags |= table::EXPAND;
+      opt->flags |= table::GAP_EXPAND;
     }
     else if (strieq(p, "box") || strieq(p, "frame")) {
       if (arg)
diff --git a/src/preproc/tbl/table.cpp b/src/preproc/tbl/table.cpp
index 759dbd073..53784b11e 100644
--- a/src/preproc/tbl/table.cpp
+++ b/src/preproc/tbl/table.cpp
@@ -1,4 +1,4 @@
-/* Copyright (C) 1989-2022 Free Software Foundation, Inc.
+/* Copyright (C) 1989-2023 Free Software Foundation, Inc.
      Written by James Clark (jjc@jclark.com)
 
 This file is part of groff.
@@ -28,6 +28,7 @@ const int DEFAULT_COLUMN_SEPARATION = 3;
 
 #define DELIMITER_CHAR "\\[tbl]"
 #define SEPARATION_FACTOR_REG PREFIX "sep"
+#define LEFTOVER_FACTOR_REG PREFIX "leftover"
 #define BOTTOM_REG PREFIX "bot"
 #define RESET_MACRO_NAME PREFIX "init"
 #define LINESIZE_REG PREFIX "lps"
@@ -2179,6 +2180,12 @@ void table::build_span_list()
 void table::compute_overall_width()
 {
   prints(".\\\" compute overall width\n");
+  if (!(flags & GAP_EXPAND)) {
+    if (left_separation)
+      printfs(".if n .ll -%1n\n", as_string(left_separation));
+    if (right_separation)
+      printfs(".if n .ll -%1n\n", as_string(right_separation));
+  }
   // Compute the amount of horizontal space available for expansion,
   // measuring every column _including_ those eligible for expansion.
   // This is the minimum required to set the table without compression.
@@ -2266,6 +2273,14 @@ void table::compute_separation_factor()
   for (int i = 0; i < ncolumns; i++)
     printfs("-\\n[%1]", span_width_reg(i, i));
   printfs("/%1\n", as_string(total_separation));
+  // Store the remainder for use in compute_column_positions().
+  if (flags & GAP_EXPAND) {
+    prints(".if n \\\n");
+    prints(".  nr " LEFTOVER_FACTOR_REG " \\n[.l]-\\n[.i]");
+    for (int i = 0; i < ncolumns; i++)
+      printfs("-\\n[%1]", span_width_reg(i, i));
+    printfs("%%%1\n", as_string(total_separation));
+  }
   prints(".ie \\n[" SEPARATION_FACTOR_REG "]<=0 \\{\\\n");
   if (!(flags & NOWARN)) {
     // Protect characters in diagnostic message (especially :, [, ])
@@ -2317,6 +2332,13 @@ void table::compute_column_positions()
            column_start_reg(i),
            column_end_reg(i-1),
            as_string(column_separation[i-1]));
+    // If we have leftover expansion room in a table using the "expand"
+    // region option, put it prior to the last column so that the table
+    // looks as if expanded to the available line length.
+    if ((ncolumns > 2) && (flags & GAP_EXPAND) && (i == (ncolumns - 1)))
+      printfs(".if n .if \\n[" LEFTOVER_FACTOR_REG "] .nr %1 +(1n>?\\n["
+             LEFTOVER_FACTOR_REG "])\n",
+             column_start_reg(i));
     printfs(".nr %1 \\n[%2]+\\n[%3]/2\n",
            column_divide_reg(i),
            column_end_reg(i-1),
diff --git a/src/preproc/tbl/table.h b/src/preproc/tbl/table.h
index be6dc4450..62346fa4e 100644
--- a/src/preproc/tbl/table.h
+++ b/src/preproc/tbl/table.h
@@ -140,9 +140,10 @@ public:
     NOKEEP        = 0x00000020,
     NOSPACES      = 0x00000040,
     NOWARN        = 0x00000080,
-    // The next two properties help manage nroff mode output.
+    // The next few properties help manage nroff mode output.
     HAS_TOP_VLINE = 0x00000100,
     HAS_TOP_HLINE = 0x00000200,
+    GAP_EXPAND    = 0x00000400,
     EXPERIMENTAL  = 0x80000000 // undocumented
     };
   char *expand;
diff --git a/src/preproc/tbl/tbl.am b/src/preproc/tbl/tbl.am
index 4333b732a..1996e302b 100644
--- a/src/preproc/tbl/tbl.am
+++ b/src/preproc/tbl/tbl.am
@@ -48,9 +48,7 @@ TESTS += $(tbl_TESTS)
 EXTRA_DIST += $(tbl_TESTS)
 
 tbl_XFAIL_TESTS = \
-  src/preproc/tbl/tests/expand-region-option-works.sh \
-  src/preproc/tbl/tests/table-lacks-spurious-top-border.sh \
-  src/preproc/tbl/tests/x-column-modifier-works.sh
+  src/preproc/tbl/tests/table-lacks-spurious-top-border.sh
 XFAIL_TESTS += $(tbl_XFAIL_TESTS)
 
 



reply via email to

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