bugGNU roff - Bugs: bug #63449, [tbl] causes grotty to complain...

 
 

bug #63449: [tbl] causes grotty to complain "character above first line discarded"

Submitter:  G. Branden Robinson <gbranden>
Submitted:  Wed 30 Nov 2022 12:44:24 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
   

Wed 11 Jan 2023 03:11:58 PM UTC, comment #4: 


commit 9b5c7d850934db2e9c4099e4c7df4875fc126b80
Author: G. Branden Robinson <g.branden.robinson@gmail.com>
Date:   Wed Jan 11 08:48:48 2023 -0600

    [tbl]: Really fix Savannah #63449.

    * src/preproc/tbl/main.cpp (process_format): "Or" on the `HAS_TOP_VLINE`
      flag if the format specification begins with a "|"; the beginning is a
      separate state in the FSM used to parse the description.  I missed it
      in commit 0e93ab4102, 1 December.

    Fixes <https://savannah.gnu.org/bugs/?63449>.

commit 0194401004c75296907872724b35a0f434bc0bef
Author: G. Branden Robinson <g.branden.robinson@gmail.com>
Date:   Tue Jan 10 13:50:47 2023 -0600

    [tbl]: Fix erroneous test; add 2 cases.

    * src/preproc/tbl/tests/do-not-overdraw-page-top-in-nroff-mode.sh:
      Correct erroneous check of test output, and add two more test cases.

    Test fails at this commit.


G. Branden Robinson <gbranden>
Group administrator
Tue 10 Jan 2023 07:32:27 PM UTC, comment #3: 

The fix in comment #2 was not complete enough.

These 2 cases may be exhibiting the same issue, but both should be tested.  (The first lacks a vertical rule column classifier at the left edge of the table; the second has one.)

Input:


$ cat ATTIC/table-too-tall-1.roff
.ll 64n
.nf
1234567890123456789012345678901234567890123456789012345678901234
.fi
.TS
L L L L L |.
abandoning      babysitter      cablecasts      dachshunds      earmarkingX
.TE
.pl \n(nlu

$ cat ATTIC/table-too-tall-2.roff
.ll 64n
.nf
1234567890123456789012345678901234567890123456789012345678901234
.fi
.TS
| L L L L L |.
abandoning      babysitter      cablecasts      dachshunds      earmarkingX
.TE
.pl \n(nlu


Output:


$ ./build/test-groff -t -T ascii ATTIC/table-too-tall-1.roff
1234567890123456789012345678901234567890123456789012345678901234
                                                                |
abandoning   babysitter   cablecasts   dachshunds   earmarkingX |
$ ./build/test-groff -t -T ascii ATTIC/table-too-tall-2.roff
ATTIC/table-too-tall-2.roff:7: warning: table wider than line length minus indentation
1234567890123456789012345678901234567890123456789012345678901234
|                                                                |
|abandoning   babysitter   cablecasts   dachshunds   earmarkingX |


G. Branden Robinson <gbranden>
Group administrator
Thu 01 Dec 2022 08:23:27 AM UTC, comment #2: 


commit 095bb703e648938cea5a76e36eec54c5711fe491
Author: G. Branden Robinson <g.branden.robinson@gmail.com>
Date:   Wed Nov 30 23:09:23 2022 -0600

    [tbl]: Regression-test Savannah #63449.

    * src/preproc/tbl/tests/do-not-overdraw-page-top-in-nroff-mode.sh: Do
      it.
    * src/preproc/tbl/tbl.am (tbl_TESTS): Run test.

    Test fails at this commit.

commit 0e93ab410294677e3e4ea105b04f2a18b5c7f9dc
Author: G. Branden Robinson <g.branden.robinson@gmail.com>
Date:   Thu Dec 1 00:53:11 2022 -0600

    [tbl]: Fix Savannah #63449.

    * src/preproc/tbl/table.h (class table): Add new enumeration constants
      for use with `flags` member variable: `HAS_TOP_VLINE` and
      `HAS_TOP_HLINE`, which track properties of the table.  Unlike others,
      these have no correspondence to table region options.

    * src/preproc/tbl/main.cpp (process_format): Add new local Boolean to
      track whether we're interpreting a format for the first row.  Use this
      to "or" on the `HAS_TOP_VLINE` or `HAS_TOP_HLINE` flags if "|" or [_-]
      are encountered in the first row's format, as appropriate.

      (process_data): "Or" on `HAS_TOP_HLINE` if a single or double
      horizontal line is used as the first row's data.

    * src/preproc/tbl/table.cpp (do_top): On nroff mode devices, add one 1v
      of space above the table if we're going to be drawing an unintersected
      vertical rule above the table's top.

    Fixes <https://savannah.gnu.org/bugs/?63449>.


G. Branden Robinson <gbranden>
Group administrator
Thu 01 Dec 2022 08:19:43 AM UTC, comment #1: 


original submission:

> If a non-boxed tbl(1) table has at least one vertical rule in it and the table starts at the top of the page (i.e., there is no top margin, and the current drawing position when the page begins is 1v), then this warning will happen because when formatting for troff, vertical rules are drawn 1v above the top of the page.  This is so that intersections with horizontal rules can be detected by grotty and replaced with appropriate intersection characters.  (Except on the 'utf8' output device, this is always a plus sign.)


The vertical rule has to be present on the first table row.

G. Branden Robinson <gbranden>
Group administrator
Wed 30 Nov 2022 12:44:24 PM UTC, original submission:  

Problem observed in groff 1.22.4 and goes back years.

Several references to it can be found online.  None seem to correctly diagnose the cause.  Here are two.

https://stackoverflow.com/questions/38253755/man-page-grotty-error-character-above-first-line-discarded

https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=673436

The problem ultimately lies in bug #62471, but that is difficult to resolve as it arises from a design decision and the limitations of character-cell output devices.

If a non-boxed tbl(1) table has at least one vertical rule in it and the table starts at the top of the page (i.e., there is no top margin, and the current drawing position when the page begins is 1v), then this warning will happen because when formatting for troff, vertical rules are drawn 1v above the top of the page.  This is so that intersections with horizontal rules can be detected by grotty and replaced with appropriate intersection characters.  (Except on the 'utf8' output device, this is always a plus sign.)

I have several test cases not yet structured into a regression test script, and a functional fix, but I'm not happy with it design-wise.  I think I want to move the Boolean flags I'm using into the "flags" data structure and break the rigid correspondence of that struct with tbl's region options.  It feels to me right now that that would be cleaner, or at least less dirty, than what I'm doing.

Test inputs:


$ more EXPERIMENTS/overheight-tbl*
::::::::::::::
EXPERIMENTS/overheight-tbl1.roff
::::::::::::::
.TS
L |.
foo
.TE
::::::::::::::
EXPERIMENTS/overheight-tbl2.roff
::::::::::::::
.TS
_
L |.
foo
.TE
::::::::::::::
EXPERIMENTS/overheight-tbl3.roff
::::::::::::::
.TS
L |.
_
foo
.TE
::::::::::::::
EXPERIMENTS/overheight-tbl4.roff
::::::::::::::
.TS
L.
_
foo
.TE
::::::::::::::
EXPERIMENTS/overheight-tbl5.roff
::::::::::::::
.TS
L.
foo
_
.TE


Fix, of questionable design:


diff --git a/src/preproc/tbl/main.cpp b/src/preproc/tbl/main.cpp
index 03cfb8f91..b3089cb1e 100644
--- a/src/preproc/tbl/main.cpp
+++ b/src/preproc/tbl/main.cpp
@@ -344,6 +344,8 @@ void process_input_file(FILE *fp)

 struct options {
   unsigned flags;
+  bool has_full_vlines; // the table has a '|' as a column classifier
+  bool has_top_hline; // the table as an hline in row 1 (not from a box)
   int linesize;
   char delim[2];
   char tab_char;
@@ -353,7 +355,8 @@ struct options {
 };

 options::options()
-: flags(0), linesize(0), tab_char('\t'), decimal_point_char('.')
+: flags(0), has_full_vlines(false), has_top_hline(false), linesize(0),
+  tab_char('\t'), decimal_point_char('.')
 {
   delim[0] = delim[1] = '\0';
 }
@@ -825,6 +828,7 @@ format *process_format(table_input &in, options *opt,
       case '-':                        // tbl also accepts this
        got_format = true;
        t = FORMAT_HLINE;
+       opt->has_top_hline = true;
        break;
       case '=':
        got_format = true;
@@ -1099,6 +1103,7 @@ format *process_format(table_input &in, options *opt,
        list->zero_width = 1;
        break;
       case '|':
+       opt->has_full_vlines = true;
        c = in.get();
        list->vline++;
        break;
@@ -1260,7 +1265,8 @@ table *process_data(table_input &in, format *f, options *opt)
   int format_index = 0;
   bool give_up = false;
   enum { DATA_INPUT_LINE, TROFF_INPUT_LINE, SINGLE_HLINE, DOUBLE_HLINE } type;
-  table *tbl = new table(ncolumns, opt->flags, opt->linesize,
+  table *tbl = new table(ncolumns, opt->flags, opt->has_full_vlines,
+                        opt->has_top_hline, opt->linesize,
                         opt->decimal_point_char);
   if (opt->delim[0] != '\0')
     tbl->set_delim(opt->delim[0], opt->delim[1]);
@@ -1287,6 +1293,8 @@ table *process_data(table_input &in, format *f, options *opt)
          type = SINGLE_HLINE;
        else
          type = DOUBLE_HLINE;
+       if (0 == current_row)
+         tbl->has_top_hline = true;
       }
       else {
        in.unget(d);
diff --git a/src/preproc/tbl/table.cpp b/src/preproc/tbl/table.cpp
index 798817aa3..e75c7ab8d 100644
--- a/src/preproc/tbl/table.cpp
+++ b/src/preproc/tbl/table.cpp
@@ -1242,12 +1242,13 @@ void vertical_rule::print()
   }
 }

-table::table(int nc, unsigned f, int ls, char dpc)
+table::table(int nc, unsigned f, bool hfv, bool hth, int ls, char dpc)
 : nrows(0), ncolumns(nc), linesize(ls), decimal_point_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)
+  vline(0), row_is_all_lines(0), left_separation(0),
+  right_separation(0), total_separation(0), allocated_rows(0), flags(f),
+  has_full_vlines(hfv), has_top_hline(hth)
 {
   minimum_width = new string[ncolumns];
   column_separation = ncolumns > 1 ? new int[ncolumns - 1] : 0;
@@ -2957,9 +2958,15 @@ void table::do_top()
     prints(".ls\n"
           ".vs\n");
   }
-  else if (flags & (ALLBOX | BOX)) {
+  else if (flags & (ALLBOX | BOX))
     print_single_hline(0);
-  }
+  // On terminal devices, a vertical rule of the height of the table
+  // will stick out 1v above a table that is unboxed or lack a
+  // horizontal rule on the first row.  This is necessary for
+  // grotty's rule intersection detection.  We also make room for it so
+  // that the vertical rule is not drawn above the top of the page.
+  else if (has_full_vlines && !has_top_hline)
+    prints(".if n .sp\n");
   //printfs(".mk %1\n", row_top_reg(0));
 }

diff --git a/src/preproc/tbl/table.h b/src/preproc/tbl/table.h
index c703fb1cd..247e794cb 100644
--- a/src/preproc/tbl/table.h
+++ b/src/preproc/tbl/table.h
@@ -83,6 +83,7 @@ struct vertical_rule;
 class table {
   int nrows;
   int ncolumns;
+  bool has_full_vlines;
   int linesize;
   char delim[2];
   char decimal_point_char;
@@ -142,8 +143,10 @@ public:
     NOWARN       = 0x00000080,
     EXPERIMENTAL = 0x80000000  // undocumented; use as a hook for experiments
     };
+  bool has_top_hline;
   char *expand;
-  table(int nc, unsigned flags, int linesize, char decimal_point_char);
+  table(int nc, unsigned flags, bool has_full_vlines,
+       bool has_top_hline, int linesize, char decimal_point_char);
   ~table();

   void add_text_line(int r, const string &, const char *, int);


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 7 latest changes.

    Date Changed by Updated Field Previous Value => Replaced by
    2023-01-11 gbranden StatusIn Progress Fixed
        Open/ClosedOpen Closed
    2023-01-10 gbranden StatusFixed In Progress
        Open/ClosedClosed Open
    2022-12-01 gbranden StatusIn Progress Fixed
        Open/ClosedOpen Closed
    2022-11-30 gbranden Planned ReleaseNone 1.23.0

    Back to the top

    Powered by Savane 3.13-f8d8.
    Corresponding source code