bugGNU roff - Bugs: bug #63640, [tbl] does not warn when | as the...

 
 

bug #63640: [tbl] does not warn when | as the last column classifier causes table to overrun line length

Submitter:  G. Branden Robinson <gbranden>
Submitted:  Tue 10 Jan 2023 07:22:53 PM UTC
   
 
Category:  Preprocessor tbl Severity:  3 - Normal
Item Group:  Warning/Suspicious behaviour Status:  Fixed
Privacy:  Public Assigned to:  gbranden
Open/Closed:  Closed Planned Release:  1.23.0
* Mandatory Fields

Add a New Comment Rich Markup
   

Fri 03 Feb 2023 09:51:25 PM UTC, comment #2: 


commit ad70899cb0c4dcf28fc95447ccc33bd8660cf719
Author: G. Branden Robinson <g.branden.robinson@gmail.com>
Date:   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>.


G. Branden Robinson <gbranden>
Group administrator
Thu 02 Feb 2023 05:18:33 PM UTC, comment #1: 

I think I have a fix for this and a couple of other nroff-mode tbl annoyances besides.


diff --git a/src/preproc/tbl/table.cpp b/src/preproc/tbl/table.cpp
index 648e5fbe6..4a56101e7 100644
--- a/src/preproc/tbl/table.cpp
+++ b/src/preproc/tbl/table.cpp
@@ -1250,7 +1250,8 @@ table::table(int nc, unsigned f, int ls, char dpc)
   vrule_list(0), stuff_list(0), span_list(0),
   entry_list(0), entry_list_tailp(&entry_list), entry(0),
   vline(0), row_is_all_lines(0), left_separation(0),
-  right_separation(0), total_separation(0), allocated_rows(0), flags(f)
+  right_separation(0), total_separation(0), nroff_separation(0),
+  allocated_rows(0), flags(f)
 {
   minimum_width = new string[ncolumns];
   column_separation = ncolumns > 1 ? new int[ncolumns - 1] : 0;
@@ -1679,13 +1680,13 @@ void table::add_vlines(int r, const char *v)
   bool twarned = false;
   for (int i = 0; i < ncolumns+1; i++) {
     assert(v[i] < 3);
-    if (v[i] && (flags & (BOX | ALLBOX | DOUBLEBOX)) && (i == 0)
-       && (!lwarned)) {
+    if (!lwarned && v[i] && (flags & (BOX | ALLBOX | DOUBLEBOX))
+       && (i == 0)) {
       error("ignoring vertical line at leading edge of boxed table");
       lwarned = true;
     }
-    else if (v[i] && (flags & (BOX | ALLBOX | DOUBLEBOX))
-            && (i == ncolumns) && (!twarned)) {
+    else if (!twarned && v[i] && (flags & (BOX | ALLBOX | DOUBLEBOX))
+            && (i == ncolumns)) {
       error("ignoring vertical line at trailing edge of boxed table");
       twarned = true;
     }
@@ -2177,8 +2178,9 @@ void table::build_span_list()

 void table::compute_expand_width()
 {
-  // First, compute the unexpanded table width, measuring every column
-  // (including those eligible for expansion).
+  // Reduce the available line length in nroff mode.
+  printfs(".if n .ll -%1n\n", as_string(nroff_separation));
+  // Compute the width available for table expansion.
   prints(".nr " EXPAND_REG " \\n[.l]-\\n[.i]");
   for (int i = 0; i < ncolumns; i++)
     printfs("-\\n[%1]", span_width_reg(i, i));
@@ -2232,17 +2234,43 @@ void table::compute_total_separation()
   if (flags & (ALLBOX | BOX | DOUBLEBOX))
     left_separation = right_separation = 1;
   else {
-    for (int i = 0; i < nrows; i++) {
-      if (vline[i][0] > 0)
+    for (int r = 0; r < nrows; r++) {
+      if (vline[r][0] > 0)
        left_separation = 1;
-      if (vline[i][ncolumns] > 0)
+      if (vline[r][ncolumns] > 0)
        right_separation = 1;
     }
   }
-  total_separation = left_separation + right_separation;
-  int i;
-  for (i = 0; i < ncolumns - 1; i++)
-    total_separation += column_separation[i];
+  if (left_separation)
+    nroff_separation++;
+  if (right_separation)
+    nroff_separation++;
+  // Check for interior vertical lines.
+  if (ncolumns > 1) {
+    bool *is_vline_in_column = new bool[ncolumns];
+    (void) memset(is_vline_in_column, 0, ncolumns);
+    for (int r = 0; r < nrows; r++)
+      for (int c = 1; c < ncolumns; c++)
+       if (vline[r][c] > 0)
+         is_vline_in_column[c] = true;
+#if 0
+    // Presently, in nroff mode vertical lines are drawn in the gaps
+    // between columns and overstruck by column content if the column
+    // spacing is reduced below 3.  Getting around this and addressing
+    // Savannah #61597 might require the creation of pseudocolumns to
+    // house vertical lines.
+    for (int c = 0; c < ncolumns; c++) {
+      if (is_vline_in_column[c]) {
+       total_separation++;
+       nroff_separation++;
+      }
+    }
+#endif
+    delete is_vline_in_column;
+  }
+  total_separation += (left_separation + right_separation);
+  for (int c = 0; c < ncolumns - 1; c++)
+    total_separation += column_separation[c];
 }

 void table::compute_separation_factor()
@@ -2288,6 +2316,8 @@ void table::compute_column_positions()
   printfs(".nr %1 %2*\\n[" SEPARATION_FACTOR_REG "]\n",
          column_start_reg(0),
          as_string(left_separation));
+  if (left_separation)
+    printfs(".if n .nr %1 +1n\n", column_start_reg(0));
   int i;
   for (i = 1;; i++) {
     printfs(".nr %1 \\n[%2]+\\n[%3]\n",
diff --git a/src/preproc/tbl/table.h b/src/preproc/tbl/table.h
index 283c51863..5a35d95bc 100644
--- a/src/preproc/tbl/table.h
+++ b/src/preproc/tbl/table.h
@@ -100,6 +100,7 @@ class table {
   int left_separation;
   int right_separation;
   int total_separation;
+  int nroff_separation;
   int allocated_rows;
   void build_span_list();
   void compute_expand_width();
diff --git a/src/preproc/tbl/tests/check-horizontal-line-length.sh b/src/preproc/tbl/tests/check-horizontal-line-length.sh
index eeb49133e..0b6fc6be1 100755
--- a/src/preproc/tbl/tests/check-horizontal-line-length.sh
+++ b/src/preproc/tbl/tests/check-horizontal-line-length.sh
@@ -44,7 +44,7 @@ echo "checking length of plain horizontal rule" >&2
 output=$(printf "%s" "$input" | "$groff" -Tascii -t)
 echo "$output" | grep -Eqx -- '-{11}' || wail

-input='.ll 12n
+input='.ll 14n
 .TS
 | L |.
 _
@@ -56,10 +56,10 @@ _

 echo "checking intersection of vertical and horizontal rules" >&2
 output=$(printf "%s" "$input" | "$groff" -Tascii -t)
-echo "$output" | sed -n '1p' | grep -Eqx '\+-{11}\+' || wail
-echo "$output" | sed -n '3p' | grep -Eqx '\+-{11}\+' || wail
+echo "$output" | sed -n '1p' | grep -Eqx '\+-{12}\+' || wail
+echo "$output" | sed -n '3p' | grep -Eqx '\+-{12}\+' || wail

-input='.ll 12n
+input='.ll 14n
 .TS
 box;
 L.
@@ -70,8 +70,8 @@ L.

 echo "checking width of boxed table" >&2
 output=$(printf "%s" "$input" | "$groff" -Tascii -t)
-echo "$output" | sed -n '1p' | grep -Eqx '\+-{11}\+' || wail
-echo "$output" | sed -n '3p' | grep -Eqx '\+-{11}\+' || wail
+echo "$output" | sed -n '1p' | grep -Eqx '\+-{12}\+' || wail
+echo "$output" | sed -n '3p' | grep -Eqx '\+-{12}\+' || wail

 test -z "$fail"

diff --git a/src/preproc/tbl/tests/check-line-intersections.sh b/src/preproc/tbl/tests/check-line-intersections.sh
index 4d7376639..7ec5f5b9e 100755
--- a/src/preproc/tbl/tests/check-line-intersections.sh
+++ b/src/preproc/tbl/tests/check-line-intersections.sh
@@ -41,7 +41,7 @@ output=$(printf "%s" "$input" | "$groff" -Tascii -t)
 for l in 1 3 5 7
 do
     echo "checking intersections on line $l"
-    echo "$output" | sed -n ${l}p | grep -Fqx '+--+---+---+' || wail
+    echo "$output" | sed -n ${l}p | grep -Fqx '+---+---+---+' || wail
 done

 # TODO: Check `-Tutf8` output for correct crossing glyph identities.
diff --git a/src/preproc/tbl/tests/check-vertical-line-length.sh b/src/preproc/tbl/tests/check-vertical-line-length.sh
index 9b08dcb68..6bb5a0093 100755
--- a/src/preproc/tbl/tests/check-vertical-line-length.sh
+++ b/src/preproc/tbl/tests/check-vertical-line-length.sh
@@ -31,7 +31,7 @@ wail () {
 # devices to enable them to cross a potential horizontal line in the
 # table.

-input='.ll 12n
+input='.ll 14n
 .TS
 | L |.
 _
@@ -42,7 +42,7 @@ _

 echo "checking length of plain vertical rule" >&2
 output=$(printf "%s" "$input" | "$groff" -Tascii -t)
-echo "$output" | sed -n '2p' | grep -Fqx -- '|1234567890 |' || wail
+echo "$output" | sed -n '2p' | grep -Fqx -- '| 1234567890 |' || wail

 test -z "$fail"


G. Branden Robinson <gbranden>
Group administrator
Tue 10 Jan 2023 07:22:53 PM UTC, original submission:  

Affects groff 1.22.4.

Input:


.ll 64n
.nf
1234567890123456789012345678901234567890123456789012345678901234
.fi
.TS
L L L L L |.
abandoning      babysitter      cablecasts      dachshunds      earmarkingX
.TE


Output:


$ nroff -t ATTIC/table-tailpipe.roff|cat -s
1234567890123456789012345678901234567890123456789012345678901234│
abandoning   babysitter   cablecasts   dachshunds   earmarkingX │


I expected a diagnostic message about the table row being too long, but I didn't get one.

G. Branden Robinson <gbranden>
Group administrator

 

(Note: upload size limit is set to 16384 kB, after insertion of the required escape characters.)

Attach Files:
   
   
Comment:
   

No files currently attached

 

Depends on the following items: None found

Items that depend on this one: None found

 

Carbon-Copy List
  • -email is unavailable- added by gbranden (Submitted the item)
  •  

    There are 0 votes so far. Votes easily highlight which items people would like to see resolved in priority, independently of the priority of the item set by tracker managers.

    Only logged-in users can vote.

     

    Follow 5 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2023-02-03 gbranden StatusIn Progress Fixed
        Open/ClosedOpen Closed
    2023-02-02 gbranden StatusNone In Progress
        Assigned toNone gbranden
        Planned ReleaseNone 1.23.0

    Back to the top

    Powered by Savane 3.13-4448.
    Corresponding source code