#992002 mandoc: -Thtml: tbl font requests ignored

Package:
mandoc
Source:
mdocml
Description:
BSD manpage compiler toolset
Submitter:
наб
Date:
2021-08-10 13:45:03 UTC
Severity:
normal
Tags:
#992002#5
Date:
2021-08-08 11:11:35 UTC
From:
To:
Dear Maintainer,

Given the following document:
-- >8 --
.Dd
.Dt T 1
.Os
.
\fBtext\fItext\f(BItext\fR
.Pp
.TS
lfB lfI lfBI lb li lbi .
text	text	text	text	text	text
.TE
-- >8 --
mandoc -Thtml -Ofragment outputs the following:
-- >8 --
<table class="head">
  <tr>
    <td class="head-ltitle">T(1)</td>
    <td class="head-vol">General Commands Manual</td>
    <td class="head-rtitle">T(1)</td>
  </tr>
</table>
<div class="manual-text"><b>text</b><i>text</i><b><i>text</i></b>
<p class="Pp"></p>
<table class="tbl">
  <tr>
    <td>text</td>
    <td>text</td>
    <td>text</td>
    <td>text</td>
    <td>text</td>
    <td>text</td>
  </tr>
</table>
</div>
<table class="foot">
  <tr>
    <td class="foot-date">August 8, 2021</td>
    <td class="foot-os">Debian</td>
  </tr>
</table>
-- >8 --

When run as mandoc | less, all \f requests are properly formatted,
as are single-letter f tbl requests (I take this to be a limitation
of the tbl parser/unsupported groff extension, considering -Tlint
also dislikes it).
"bi" is formatted as bold, groff formats it as italic. This is fine,
I think? but worth noting.

However, as you can see, with -Thtml, the \f requests are correct,
but all tbl font requests are ignored entirely.
(Plus, if you remove the final \fR, they're all bold+italic.
 This is to be vaguely expected, had no other fonts been specified.)

Best,
наб

#992002#10
Date:
2021-08-08 12:49:19 UTC
From:
To:
Easy enough, oddly. Patch attached, applies cleanly on top of 1.22.4-6.
Please consider it.

Given the following document:
-- >8 --
.Dd
.Dt V 1
.Os
.
\fBtext\fItext\f(BItext\f(CRtext\f(CBtext\f(CItext\fR
.Pp
.TS
lfB lfI lfBI lb li lbi lfCR lfCB lfCI .
text	text	text	text	text	text	text	text	text
.TE
-- >8 --

When rendering to a teletype, the fonts are
  b, ul, bul;  b, ul, bul;  normal, b, ul
this is as expected!

#992002#19
Date:
2021-08-08 13:08:46 UTC
From:
To:
Can you send your patches to upstream directly please?
See https://mandoc.bsd.lv/contact.html

It’s awkward for Debian maintainers to sit in the middle.
Thanks!

#992002#24
Date:
2021-08-08 13:24:52 UTC
From:
To:
Hi!

tbl's -Thtml ignores font requests; additionally, the tbl f-request
parser only allows single-character fonts. Cf. the Debian bug
(http://bugs.debian.org/992002) for additional context.

Please consider the following patch.
наб
--- With this patch, the following document: -- >8 -- .Dd .Dt V 1 .Os . \fBtext\fItext\f(BItext\f(CRtext\f(CBtext\f(CItext\fR .Pp .TS lfB lfI lfBI lb li lbi lfCR lfCB lfCI . text text text text text text text text text .TE -- >8 -- Renders to a teletype with the expected fonts: b, ul, bul; b, ul, bul; normal, b, ul
--- mdocml-1.14.5.orig/tbl.7 +++ mdocml-1.14.5/tbl.7 @@ -178,10 +178,11 @@ of any other column also having the .Cm e modifier. .It Cm f -The next character selects the font to use for this cell. +The next two characters select the font to use for this cell. +One-character font names must be followed by a blank or period. See the .Xr roff 7 -manual for supported one-character font names. +manual for supported font names. .It Cm i Use an italic font for the contents of this cell. .It Cm m --- mdocml-1.14.5.orig/tbl.h +++ mdocml-1.14.5/tbl.h @@ -59,12 +59,13 @@ struct tbl_cell { int flags; #define TBL_CELL_BOLD (1 << 0) /* b, B, fB */ #define TBL_CELL_ITALIC (1 << 1) /* i, I, fI */ -#define TBL_CELL_TALIGN (1 << 2) /* t, T */ -#define TBL_CELL_UP (1 << 3) /* u, U */ -#define TBL_CELL_BALIGN (1 << 4) /* d, D */ -#define TBL_CELL_WIGN (1 << 5) /* z, Z */ -#define TBL_CELL_EQUAL (1 << 6) /* e, E */ -#define TBL_CELL_WMAX (1 << 7) /* x, X */ +#define TBL_CELL_FONTCW (1 << 2) /* fC[RBI] */ +#define TBL_CELL_TALIGN (1 << 3) /* t, T */ +#define TBL_CELL_UP (1 << 4) /* u, U */ +#define TBL_CELL_BALIGN (1 << 5) /* d, D */ +#define TBL_CELL_WIGN (1 << 6) /* z, Z */ +#define TBL_CELL_EQUAL (1 << 7) /* e, E */ +#define TBL_CELL_WMAX (1 << 8) /* x, X */ enum tbl_cellt pos; };
--- mdocml-1.14.5.orig/tbl_html.c +++ mdocml-1.14.5/tbl_html.c @@ -25,6 +25,7 @@ #include <string.h> #include "mandoc.h" +#include "mandoc_aux.h" #include "tbl.h" #include "out.h" #include "html.h" @@ -218,6 +219,7 @@ print_tbl(struct html *h, const struct t else valign = NULL; + int flags = cp->flags; for (i = dp->hspans; i > 0; i--) cp = cp->next; switch (cp->vert) { @@ -239,8 +241,36 @@ print_tbl(struct html *h, const struct t "vertical-align", valign, "text-align", halign, "border-right-style", rborder); - if (dp->string != NULL) - print_text(h, dp->string); + if (dp->string != NULL) { + const char *font = NULL; + switch (flags & (TBL_CELL_BOLD | TBL_CELL_ITALIC | TBL_CELL_FONTCW)) { + case TBL_CELL_BOLD: + font = "\\fB"; + break; + case TBL_CELL_ITALIC: + font = "\\fI"; + break; + case TBL_CELL_BOLD | TBL_CELL_ITALIC: + font = "\\f(BI"; + break; + case TBL_CELL_FONTCW: + font = "\\f(CR"; + break; + case TBL_CELL_FONTCW | TBL_CELL_BOLD: + font = "\\f(CB"; + break; + case TBL_CELL_FONTCW | TBL_CELL_ITALIC: + font = "\\f(CI"; + break; + } + if (font) { + char *str; + mandoc_asprintf(&str, "%s%s\\fP", font, dp->string); + print_text(h, str); + free(str); + } else + print_text(h, dp->string); + } } print_tagq(h, tt); --- mdocml-1.14.5.orig/tbl_layout.c +++ mdocml-1.14.5/tbl_layout.c @@ -170,9 +170,7 @@ mod: if (p[*pos] == '(') goto mod; - /* Support only one-character font-names for now. */ - - if (p[*pos] == '\0' || (p[*pos + 1] != ' ' && p[*pos + 1] != '.')) { + if (p[*pos] == '\0' || (!isalnum(p[*pos + 1]) && p[*pos + 1] != ' ' && p[*pos + 1] != '.')) { mandoc_msg(MANDOCERR_FT_BAD, ln, *pos, "TS %s", p + *pos - 1); if (p[*pos] != '\0') @@ -182,23 +180,27 @@ mod: goto mod; } - switch (p[(*pos)++]) { - case '3': - case 'B': + char fn[3] = {'\0'}, *fp = fn; + fn[0] = p[(*pos)++]; + fn[1] = p[*pos] == ' ' || p[*pos] == '.' ? '\0' : p[(*pos)++]; + +refont: + if (!strcmp(fp, "3") || !strcmp(fp, "B")) cp->flags |= TBL_CELL_BOLD; - goto mod; - case '2': - case 'I': + else if (!strcmp(fp, "2") || !strcmp(fp, "I")) cp->flags |= TBL_CELL_ITALIC; - goto mod; - case '1': - case 'R': - goto mod; - default: + else if (!strcmp(fp, "1") || !strcmp(fp, "R")) + ; + else if (!strcmp(fp, "BI")) + cp->flags |= TBL_CELL_BOLD | TBL_CELL_ITALIC; + else if (fp[0] == 'C' && fp[1]) { + cp->flags |= TBL_CELL_FONTCW; + ++fp; + goto refont; + } else mandoc_msg(MANDOCERR_FT_BAD, - ln, *pos - 1, "TS f%c", p[*pos - 1]); - goto mod; - } + ln, *pos - strlen(fn), "TS f%s", fn); + goto mod; } static void --- mdocml-1.14.5.orig/tbl_term.c +++ mdocml-1.14.5/tbl_term.c @@ -922,10 +922,17 @@ tbl_word(struct termp *tp, const struct int prev_font; prev_font = tp->fonti; - if (dp->layout->flags & TBL_CELL_BOLD) - term_fontpush(tp, TERMFONT_BOLD); - else if (dp->layout->flags & TBL_CELL_ITALIC) - term_fontpush(tp, TERMFONT_UNDER); + switch (dp->layout->flags & (TBL_CELL_BOLD | TBL_CELL_ITALIC)) { + case TBL_CELL_BOLD | TBL_CELL_ITALIC: + term_fontpush(tp, TERMFONT_BI); + break; + case TBL_CELL_BOLD: + term_fontpush(tp, TERMFONT_BOLD); + break; + case TBL_CELL_ITALIC: + term_fontpush(tp, TERMFONT_UNDER); + break; + } term_word(tp, dp->string);
#992002#29
Date:
2021-08-09 08:58:19 UTC
From:
To:
Hello Nab,

Nab wrote on Sun, Aug 08, 2021 at 03:24:53PM +0200:

Not in CVS HEAD; see https://cvsweb.bsd.lv/mandoc/tbl_html.c revision 1.34,
committed on May 16 earlier this year.

Yes, that is still a missing feature.

Thank you very much for sending quite a non-trivial patch.
That was very helpful in understanding what exactly is needed.

I started from your patch and changed a few aspects:

 * You couldn't possibly know that i'm trying to work towards a
   unified system for identifying fonts using the mandoc.h
   enum mandoc_esc ESCAPE_FONT* identifiers.  Having different
   font identifiers for each output module is not good.
   So i added ESCAPE_FONTCB and ESCAPE_FONTCI and used those.
   A nice side effect is that CB and CI now work in HTML
   for all of \f, .ft, and tbl(7) f and that tbl(7) fBI
   now also works for terminal output.
 * GNU tbl(1) appears to ignore space characters between the f
   modifier and the font name, so "lf   B" is the same as "lfB".

Not quite.  The expected output for lbi is ul, not bul.
The i overrides the b rather than add to it.
So lbi is the same as lfI, not as lfBI.
[...]

Again, <td><i>text</i></td> with my version of the patch.

These become:

      <td><span class="Li"><b>text</b></span></td>
      <td><span class="Li"><i>text</i></span></td>

Could you please check out from CVS (instead of the last release),
apply the following patch, and tell me whether it looks reasonable
and works for you?

When this gets committed, i will credit you for reporting the
missing feature.  Do i understand correctly that "Nabija" is your
first name and "Czleweli" your last name?

Thanks again,
  Ingo

#992002#34
Date:
2021-08-09 23:08:31 UTC
From:
To:
Oh, indeed. I tested and based my patch on 1.14.5 from Debian,
didn't realise that's almost two years old by now.
Will use the CVS next time.
This is great news! A bunch of my pages use C[BI] and the HTML renders
look much better, thanks!
Huh, so it does! That's not explicitly mentioned by the manual and so
I didn't think to test it. Now, tbl(1) says
  Key characters can be separated by spaces or tabs.
so consider the following document:
-- >8 --
.TS
lfBI	lf BI	lf	BI	.
a	b	c
.TE
-- >8 --
(In order, none, space, tab follow 'f';
 base64: LlRTCmxmQkkJbGYgQkkJbGYJQkkJLgphCWIJYwouVEUK)

groff renders it with a, b, and c as BI,
but mandoc with your patch with a+b as BI and c as R, with -Tlint:
  mandoc: ./q.1:2:14: WARNING: unknown font, skipping request: TS f	BI	.

If you change tbl_layout.c L171 to match L75:
-- >8 --
-	while (p[*pos] == ' ')
+	while (p[*pos] == ' ' || p[*pos] == '\t')
-- >8 --
and L187:
-- >8 --
-	if (strchr(" .", p[*pos + isz]) == NULL)
+	if (strchr(" \t.", p[*pos + isz]) == NULL)
-- >8 --
The document renders correctly.
Indeed, it looks like I got confused by the groff parsing
and thought it'd accumulate instead.
Yeah, save for the tab thing above, I haven't managed to fault it,
in tests or real pages.
They aren't, but either "наб" or "nabijaczleweli" is fine.

Best,
наб

#992002#39
Date:
2021-08-10 13:43:58 UTC
From:
To:
Hi Nab,

Nab wrote on Tue, Aug 10, 2021 at 01:08:31AM +0200:

Note that so far, everybody who contributed code to mandoc provided
their first and last names.  I'm not sure it is strictly required in
the legal sense, but i do consider it beneficial both for authors
and for users.  The benefit for authors is that it makes it easier
for them to exercise their rights under the Berne Convention, in
particular their moral rights under that Convention, for example
to protect themselves if somebody abuses their contribution for
slander.  The benefit for users of knowing who the Copyright holders
are is also obvious, even if the code is BSD or ISC licensed:
It makes Copyright and license audits easier and reduces the risk
of suddenly being sued by parties the users didn't even know existed.

In this case, it isn't needed because by mere chance, even though
several of your ideas remained in the committed patch, none of your
code did, because i switched from TBL_CELL_BOLD and TBL_CELL_ITALIC
to ESCAPE_FONT*.  Ideas aren't subject to Copyright, only text is,
and for crediting a person who provided bug reports, feature requests,
and ideas, a pseudonym is sufficient.

Note that i don't recommend using these fonts in manual pages.
Even with groff, typical installations don't prodide CB and CI
fonts for terminal output, which typically results in warnings
being thrown.  The details may vary among operating systems and
package managers even for the same version of groff.  Portability
to other formatters (like Heirloom, Plan 9, DWB, Solaris, neatroff)
is even more doubtful, but i don't know any details about that.

But as a rule, mandoc(1) even supports features if using them is
unwise, as long as that doesn't cause an undue burden.  One reason
to do so is making existing pages look better, no matter how good
or bad the style is that they are using.  Not supporting a feature
hurts end users - who aren't responsible for author's choices which
features to use.  But that a feature is supported by mandoc(1)
should not be misinterpreted by authors as a free pass to go on a
rampage and employ the most arcane and brittle features they manage
to find.

No, i'm not even convinced it is intentional, and relying on it in
any document would be a thoroughly bad idea.  But mandoc(1) aims to
be bug-compatible with groff unless there is a good reason to differ.

Done in the commit cited below, thanks for pointing out that quirk.

Thank you very much for testing.  That patch ended up growing
tentacles into quite a number of files, so the additional testing
is highly appreciated.

Here is the committed patch:

https://inbox.vuxu.org/mandoc-source/c2aa6365c21bfcca@mandoc.bsd.lv/

Yours,
  Ingo