Re: tbl: incorrect width calculation in cs[s..]

[Ingo Schwarze <schwarze@usta.de>]

Hello Simon,

Simon Branch wrote on Sun, Apr 03, 2022 at 01:14:27PM -0700:

> Hello!  mandoc has some slightly odd behavior when formatting a table
> like this, where the author wants a centered title:
> 
>    .TS
>    tab(#);
>    csss
>    l1 l3 l1 l.
>    heading
>    00#NUL#01#SOH
>    .TE
> 
> Groff outputs this:
> 
>        heading
>    00 NUL   01 SOH

Which makes sense: 4 spaces before and after the heading.

> mandoc outputs this:
> 
>          heading
>    00 NUL   01 SOH

Which is clearly not right.

> There are three problems here:
> 
>    - The space after the first column isn't counted.
>    - The imaginary spaces after the last column are counted, even though
>      they aren't output.

Well, as long as we assume the spacing is always 3n (as the old code
did), we can't really say whether the magic constant "3" in the code
refers to the spacing before or after the column, so these two
perfectly cancel each other and do not represent additional bugs.

>    - Mandoc assumes that each column has three trailing spaces.

Yes.  That is indeed the issue here.

> In other words, the control flow works like this:
> 
>    - Start with the first column's width.
>    - If there are any continuation columns, add their widths and trailing
>      spaces.
>
> But I think it should work like this:
> 
>    - Start with the first column's width.
>    - If there are any continuation columns, add the widths of the rest of
>      the columns, and all the trailing spaces *except* for the ones after
>      the last column.

Yes.

Thank you for the good analysis and the perfect patch.  I committed
it to openbsd.org and bsd.lv.

> This might still be incorrect (?) if there are more columns after
> "cs...", but I think a partial fix is better than no fix at all.

Actually, my impression is it works just fine even in that case, too.
Besides, it survives the complete regression test suite, which contains
many tbl(7) tests, though admittedly few that explicitly set the
spacing between columns.


I had to apply your patch by hand because you converted tabs to
spaces.  When sending patches, please use "cvs diff -up" and make
sure neither you nor your mail program mangle the patch.

I'm including the committed patch below; please compare it to the
version you sent to see how they differ.

Yours,
  Ingo


Log Message:
-----------
When calculating the with of spanned columns, which for example matters
for centering text spanning multiple tbl(7) columns, correctly account
for the spacing between columns instead of wrongly assuming the default 
spacing of 3n.

Patch from Simon Branch <simonmbranch at gmail dot com>.

Modified Files:
--------------
    mandoc:
        tbl_term.c

Revision Data
-------------
Index: tbl_term.c
===================================================================
RCS file: /home/cvs/mandoc/mandoc/tbl_term.c,v
retrieving revision 1.75
retrieving revision 1.76
diff -Ltbl_term.c -Ltbl_term.c -u -p -r1.75 -r1.76
--- tbl_term.c
+++ tbl_term.c
@@ -820,8 +820,11 @@ tbl_literal(struct termp *tp, const stru
 	width = col->width;
 	ic = dp->layout->col;
 	hspans = dp->hspans;
-	while (hspans--)
-		width += tp->tbl.cols[++ic].width + 3;
+	while (hspans--) {
+		width += tp->tbl.cols[ic].spacing;
+		ic++;
+		width += tp->tbl.cols[ic].width;
+	}
 
 	padr = width > len ? width - len : 0;
 	padl = 0;
--
 To unsubscribe send an email to discuss+unsubscribe@mandoc.bsd.lv