* tbl issues with 1.11.1
@ 2011-04-14 1:45 Yuri Pankov
2011-04-16 21:34 ` Ingo Schwarze
2011-04-17 0:12 ` Joerg Sonnenberger
0 siblings, 2 replies; 9+ messages in thread
From: Yuri Pankov @ 2011-04-14 1:45 UTC (permalink / raw)
To: discuss
Hi,
I'm seeing issues with tbl rendering on FreeBSD/OpenIndiana with 1.11.1:
.TS
tab(:) box;
l | l .
HEAD1:HEAD2
=
cell1:cell2
.TE
results in:
+------+------+
|HEAD1 |HEAD2 |
|========+======|
|cell1 |cell2 |
+------+------+
while 1.10.9 and GNU's 'tbl | groff' seem to give nice looking table.
Thanks,
Yuri
--
To unsubscribe send an email to discuss+unsubscribe@mdocml.bsd.lv
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: tbl issues with 1.11.1
2011-04-14 1:45 tbl issues with 1.11.1 Yuri Pankov
@ 2011-04-16 21:34 ` Ingo Schwarze
2011-04-26 20:10 ` Joerg Sonnenberger
2011-04-17 0:12 ` Joerg Sonnenberger
1 sibling, 1 reply; 9+ messages in thread
From: Ingo Schwarze @ 2011-04-16 21:34 UTC (permalink / raw)
To: discuss; +Cc: Yuri Pankov
Hi Yuri,
Yuri Pankov wrote on Thu, Apr 14, 2011 at 05:45:26AM +0400:
> I'm seeing issues with tbl rendering on FreeBSD/OpenIndiana with 1.11.1:
>
> .TS
> tab(:) box;
> l | l .
> HEAD1:HEAD2
> =
> cell1:cell2
> .TE
>
> results in:
>
> +------+------+
> |HEAD1 |HEAD2 |
> |========+======|
> |cell1 |cell2 |
> +------+------+
>
> while 1.10.9 and GNU's 'tbl | groff' seem to give nice looking table.
Thanks for the very good report.
This does indeed look wrong.
The OpenBSD version is affected as well.
I have added this entry to the TODO list:
************************************************************************
* formatter bugs
************************************************************************
+- tbl(7): Horizontal and vertical lines are formatted badly:
+ With the box option, there is too much white space at the end of cells.
+ Horizontal lines from "=" lines are a bit too long.
+ yuri dot pankov at gmail dot com Thu, 14 Apr 2011 05:45:26 +0400
+
Yours,
Ingo
--
To unsubscribe send an email to discuss+unsubscribe@mdocml.bsd.lv
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: tbl issues with 1.11.1
2011-04-14 1:45 tbl issues with 1.11.1 Yuri Pankov
2011-04-16 21:34 ` Ingo Schwarze
@ 2011-04-17 0:12 ` Joerg Sonnenberger
1 sibling, 0 replies; 9+ messages in thread
From: Joerg Sonnenberger @ 2011-04-17 0:12 UTC (permalink / raw)
To: discuss
On Thu, Apr 14, 2011 at 05:45:26AM +0400, Yuri Pankov wrote:
> +------+------+
> |HEAD1 |HEAD2 |
> |========+======|
> |cell1 |cell2 |
> +------+------+
I see similar issues with the canonical queue(3) example. Haven't
investigated yet.
Joerg
--
To unsubscribe send an email to discuss+unsubscribe@mdocml.bsd.lv
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: tbl issues with 1.11.1
2011-04-16 21:34 ` Ingo Schwarze
@ 2011-04-26 20:10 ` Joerg Sonnenberger
2011-05-12 22:02 ` Joerg Sonnenberger
0 siblings, 1 reply; 9+ messages in thread
From: Joerg Sonnenberger @ 2011-04-26 20:10 UTC (permalink / raw)
To: discuss
[-- Attachment #1: Type: text/plain, Size: 572 bytes --]
On Sat, Apr 16, 2011 at 11:34:13PM +0200, Ingo Schwarze wrote:
> > +------+------+
> > |HEAD1 |HEAD2 |
> > |========+======|
> > |cell1 |cell2 |
> > +------+------+
> >
>
> Thanks for the very good report.
> This does indeed look wrong.
> The OpenBSD version is affected as well.
Ingo, that's your change from January. Care to check the attached patch?
It seems to compute the right column widths with one exception. If no |
is used, GNU tbl(1) separates columns by an implicit " ", e.g.
foo_ bar_ baz_
for left-aligned data. That's currently missing.
Joerg
[-- Attachment #2: tbl-layout.diff --]
[-- Type: text/x-diff, Size: 1577 bytes --]
Index: out.c
===================================================================
RCS file: /usr/vhosts/mdocml.bsd.lv/cvs/mdocml/out.c,v
retrieving revision 1.40
diff -u -p -r1.40 out.c
--- out.c 9 Apr 2011 15:29:40 -0000 1.40
+++ out.c 26 Apr 2011 20:07:03 -0000
@@ -293,7 +293,7 @@ tblcalc_literal(struct rofftbl *tbl, str
case (TBL_CELL_LONG):
/* FALLTHROUGH */
case (TBL_CELL_CENTRE):
- bufsz = (*tbl->len)(1, tbl->arg);
+ bufsz = (*tbl->len)(2, tbl->arg);
break;
default:
bufsz = (*tbl->len)(1, tbl->arg);
Index: tbl_term.c
===================================================================
RCS file: /usr/vhosts/mdocml.bsd.lv/cvs/mdocml/tbl_term.c,v
retrieving revision 1.19
diff -u -p -r1.19 tbl_term.c
--- tbl_term.c 25 Jan 2011 12:07:30 -0000 1.19
+++ tbl_term.c 26 Apr 2011 20:07:03 -0000
@@ -198,8 +198,6 @@ tbl_hrule(struct termp *tp, const struct
width = tp->tbl.cols[hp->ident].width;
switch (hp->pos) {
case (TBL_HEAD_DATA):
- if (hp->next)
- width += 2;
tbl_char(tp, c, width);
break;
case (TBL_HEAD_DVERT):
@@ -375,9 +373,9 @@ tbl_literal(struct termp *tp, const stru
break;
case (TBL_CELL_CENTRE):
padr = col->width - term_strlen(tp, dp->string);
- if (3 > padr)
+ if (2 > padr)
break;
- padl = (padr - 1) / 2;
+ padl = padr / 2;
padr -= padl;
break;
case (TBL_CELL_RIGHT):
@@ -390,7 +388,7 @@ tbl_literal(struct termp *tp, const stru
tbl_char(tp, ASCII_NBRSP, padl);
term_word(tp, dp->string);
- tbl_char(tp, ASCII_NBRSP, padr + 2);
+ tbl_char(tp, ASCII_NBRSP, padr);
}
static void
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: tbl issues with 1.11.1
2011-04-26 20:10 ` Joerg Sonnenberger
@ 2011-05-12 22:02 ` Joerg Sonnenberger
2011-05-13 21:03 ` Kristaps Dzonsons
0 siblings, 1 reply; 9+ messages in thread
From: Joerg Sonnenberger @ 2011-05-12 22:02 UTC (permalink / raw)
To: discuss
On Tue, Apr 26, 2011 at 10:10:50PM +0200, Joerg Sonnenberger wrote:
> Ingo, that's your change from January. Care to check the attached patch?
Anyone?
Joerg
--
To unsubscribe send an email to discuss+unsubscribe@mdocml.bsd.lv
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: tbl issues with 1.11.1
2011-05-12 22:02 ` Joerg Sonnenberger
@ 2011-05-13 21:03 ` Kristaps Dzonsons
2011-05-14 4:12 ` Ingo Schwarze
0 siblings, 1 reply; 9+ messages in thread
From: Kristaps Dzonsons @ 2011-05-13 21:03 UTC (permalink / raw)
To: discuss, Ingo Schwarze
>> Ingo, that's your change from January. Care to check the attached patch?
>
> Anyone?
Ingo? I remember you checking these in... care to take another look?
--
To unsubscribe send an email to discuss+unsubscribe@mdocml.bsd.lv
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: tbl issues with 1.11.1
2011-05-13 21:03 ` Kristaps Dzonsons
@ 2011-05-14 4:12 ` Ingo Schwarze
2011-09-12 20:11 ` Yuri Pankov
0 siblings, 1 reply; 9+ messages in thread
From: Ingo Schwarze @ 2011-05-14 4:12 UTC (permalink / raw)
To: discuss
Hi Kristaps and Joerg,
>>> Ingo, that's your change from January. Care to check the attached patch?
>> Anyone?
> Ingo? I remember you checking these in... care to take another look?
Er, jaja, i was a bit swamped...
Unfortunately, Joergs patch is not right, i breaks the horizontal
spacing in tables without vertical rulers almost completely, see
for example captoinfo(1) and compare the groff and mandoc output.
Getting this right in both cases - with and without vertical rulers -
requires rather large changes. The logic, as far as i understand it
so far, goes like this:
- box produces "|"
- no spacing at this point
- first cell, width is max(lengths)
- one space
- vertical ruler, if none, another space
- another space, unless there was a double ruler (!!)
- second cell
- and so on.
That is vastly different from what we have now, both regarding the
width calculations - which apparently must not include padding - and
regarding the output itself.
The work-in-progress patch below comes closer. However, it still
has at least two deficiencies, so it is not ready for commit:
- There still is a bogus space after double rulers, see (!!).
- The handling of explicit spacing options got sweeped away
by the reorg i was forced to do. Probably, i need to put
it back in at the right places. It was in the calc routine
but probably belongs in the output routines themselves.
Just sending this such that you finally get some feedback.
In case we need more discussion, we should probably move to tech@.
Yours,
Ingo
P.S.
The patch is large, but at least it shortens the code. :)
Index: out.c
===================================================================
RCS file: /cvs/src/usr.bin/mandoc/out.c,v
retrieving revision 1.13
diff -u -p -r1.13 out.c
--- out.c 21 Apr 2011 22:59:54 -0000 1.13
+++ out.c 14 May 2011 03:55:52 -0000
@@ -506,39 +506,12 @@ static void
tblcalc_literal(struct rofftbl *tbl, struct roffcol *col,
const struct tbl_dat *dp)
{
- size_t sz, bufsz, spsz;
+ size_t sz;
const char *str;
- /*
- * Calculate our width and use the spacing, with a minimum
- * spacing dictated by position (centre, e.g,. gets a space on
- * either side, while right/left get a single adjacent space).
- */
-
- bufsz = spsz = 0;
str = dp->string ? dp->string : "";
sz = (*tbl->slen)(str, tbl->arg);
- /* FIXME: TBL_DATA_HORIZ et al.? */
-
- assert(dp->layout);
- switch (dp->layout->pos) {
- case (TBL_CELL_LONG):
- /* FALLTHROUGH */
- case (TBL_CELL_CENTRE):
- bufsz = (*tbl->len)(1, tbl->arg);
- break;
- default:
- bufsz = (*tbl->len)(1, tbl->arg);
- break;
- }
-
- if (dp->layout->spacing) {
- spsz = (*tbl->len)(dp->layout->spacing, tbl->arg);
- bufsz = bufsz > spsz ? bufsz : spsz;
- }
-
- sz += bufsz;
if (col->width < sz)
col->width = sz;
}
@@ -555,7 +528,7 @@ tblcalc_number(struct rofftbl *tbl, stru
/*
* First calculate number width and decimal place (last + 1 for
- * no-decimal numbers). If the stored decimal is subsequent
+ * non-decimal numbers). If the stored decimal is subsequent to
* ours, make our size longer by that difference
* (right-"shifting"); similarly, if ours is subsequent the
* stored, then extend the stored size by the difference.
@@ -582,11 +555,6 @@ tblcalc_number(struct rofftbl *tbl, stru
} else
d = sz + psz;
- /* Padding. */
-
- sz += (*tbl->len)(2, tbl->arg);
- d += (*tbl->len)(1, tbl->arg);
-
/* Adjust the settings for this column. */
if (col->decimal > d) {
@@ -599,11 +567,4 @@ tblcalc_number(struct rofftbl *tbl, stru
col->width = sz;
if (d > col->decimal)
col->decimal = d;
-
- /* Adjust for stipulated width. */
-
- if (col->width < dp->layout->spacing)
- col->width = dp->layout->spacing;
}
-
-
Index: tbl_term.c
===================================================================
RCS file: /cvs/src/usr.bin/mandoc/tbl_term.c,v
retrieving revision 1.8
diff -u -p -r1.8 tbl_term.c
--- tbl_term.c 25 Jan 2011 12:07:26 -0000 1.8
+++ tbl_term.c 14 May 2011 03:55:52 -0000
@@ -37,7 +37,6 @@ static void tbl_number(struct termp *, c
const struct tbl_dat *,
const struct roffcol *);
static void tbl_hrule(struct termp *, const struct tbl_span *);
-static void tbl_vframe(struct termp *, const struct tbl *);
static void tbl_vrule(struct termp *, const struct tbl_head *);
@@ -96,7 +95,9 @@ term_tbl(struct termp *tp, const struct
/* Vertical frame at the start of each row. */
- tbl_vframe(tp, sp->tbl);
+ if (TBL_OPT_BOX & sp->tbl->opts || TBL_OPT_DBOX & sp->tbl->opts)
+ term_word(tp, TBL_SPAN_HORIZ == sp->pos ||
+ TBL_SPAN_DHORIZ == sp->pos ? "+" : "|");
/*
* Now print the actual data itself depending on the span type.
@@ -134,9 +135,28 @@ term_tbl(struct termp *tp, const struct
if (--spans >= 0)
continue;
+ /* All cells but the first get a leading blank. */
+
+ if (hp != sp->head)
+ tbl_char(tp, ASCII_NBRSP, 1);
+
col = &tp->tbl.cols[hp->ident];
tbl_data(tp, sp->tbl, dp, col);
+ /* No trailing blanks. */
+
+ if (NULL == hp->next)
+ break;
+
+ /*
+ * Add another blank between cells,
+ * or two when there is no vertical ruler.
+ */
+
+ tbl_char(tp, ASCII_NBRSP,
+ TBL_HEAD_VERT == hp->next->pos ||
+ TBL_HEAD_DVERT == hp->next->pos ? 1 : 2);
+
/*
* Go to the next data cell and assign the
* number of subsequent spans, if applicable.
@@ -150,7 +170,9 @@ term_tbl(struct termp *tp, const struct
break;
}
- tbl_vframe(tp, sp->tbl);
+ if (TBL_OPT_BOX & sp->tbl->opts || TBL_OPT_DBOX & sp->tbl->opts)
+ term_word(tp, TBL_SPAN_HORIZ == sp->pos ||
+ TBL_SPAN_DHORIZ == sp->pos ? "+" : " |");
term_flushln(tp);
/*
@@ -176,7 +198,7 @@ tbl_hrule(struct termp *tp, const struct
{
const struct tbl_head *hp;
char c;
- size_t width;
+ size_t width, spc;
/*
* An hrule extends across the entire table and is demarked by a
@@ -190,16 +212,14 @@ tbl_hrule(struct termp *tp, const struct
/* FIXME: don't use `+' between data and a spanner! */
+ spc = 1;
for (hp = sp->head; hp; hp = hp->next) {
width = tp->tbl.cols[hp->ident].width;
switch (hp->pos) {
case (TBL_HEAD_DATA):
- if (hp->next)
- width += 2;
- tbl_char(tp, c, width);
+ tbl_char(tp, c, width + spc);
break;
case (TBL_HEAD_DVERT):
- tbl_char(tp, '+', width);
/* FALLTHROUGH */
case (TBL_HEAD_VERT):
tbl_char(tp, '+', width);
@@ -208,6 +228,7 @@ tbl_hrule(struct termp *tp, const struct
abort();
/* NOTREACHED */
}
+ spc = 2;
}
}
@@ -215,7 +236,7 @@ static void
tbl_hframe(struct termp *tp, const struct tbl_span *sp)
{
const struct tbl_head *hp;
- size_t width;
+ size_t width, spc;
if ( ! (TBL_OPT_BOX & sp->tbl->opts ||
TBL_OPT_DBOX & sp->tbl->opts))
@@ -231,25 +252,29 @@ tbl_hframe(struct termp *tp, const struc
if (TBL_OPT_DBOX & sp->tbl->opts) {
term_word(tp, "+");
+ spc = 1;
for (hp = sp->head; hp; hp = hp->next) {
- width = tp->tbl.cols[hp->ident].width;
+ width = tp->tbl.cols[hp->ident].width + spc;
tbl_char(tp, '-', width);
+ spc = 2;
}
term_word(tp, "+");
term_flushln(tp);
}
term_word(tp, "+");
+ spc = 1;
for (hp = sp->head; hp; hp = hp->next) {
width = tp->tbl.cols[hp->ident].width;
switch (hp->pos) {
case (TBL_HEAD_DATA):
- tbl_char(tp, '-', width);
+ tbl_char(tp, '-', width + spc);
break;
default:
tbl_char(tp, '+', width);
break;
}
+ spc = 2;
}
term_word(tp, "+");
term_flushln(tp);
@@ -330,14 +355,6 @@ tbl_vrule(struct termp *tp, const struct
}
static void
-tbl_vframe(struct termp *tp, const struct tbl *tbl)
-{
-
- if (TBL_OPT_BOX & tbl->opts || TBL_OPT_DBOX & tbl->opts)
- term_word(tp, "|");
-}
-
-static void
tbl_char(struct termp *tp, char c, size_t len)
{
size_t i, sz;
@@ -356,37 +373,34 @@ static void
tbl_literal(struct termp *tp, const struct tbl_dat *dp,
const struct roffcol *col)
{
- size_t padl, padr, ssz;
-
- padl = padr = 0;
+ size_t padl, padr;
assert(dp->string);
-
- ssz = term_len(tp, 1);
+ padr = col->width - term_strlen(tp, dp->string);
+ padl = 0;
switch (dp->layout->pos) {
case (TBL_CELL_LONG):
- padl = ssz;
- padr = col->width - term_strlen(tp, dp->string) - ssz;
+ padl = term_len(tp, 1);
+ padr -= padl;
break;
case (TBL_CELL_CENTRE):
- padr = col->width - term_strlen(tp, dp->string);
- if (3 > padr)
+ if (2 > padr)
break;
- padl = (padr - 1) / 2;
+ padl = padr / 2;
padr -= padl;
break;
case (TBL_CELL_RIGHT):
- padl = col->width - term_strlen(tp, dp->string);
+ padl = padr;
+ padr = 0;
break;
default:
- padr = col->width - term_strlen(tp, dp->string);
break;
}
tbl_char(tp, ASCII_NBRSP, padl);
term_word(tp, dp->string);
- tbl_char(tp, ASCII_NBRSP, padr + 2);
+ tbl_char(tp, ASCII_NBRSP, padr);
}
static void
@@ -422,9 +436,6 @@ tbl_number(struct termp *tp, const struc
d = ssz + psz;
} else
d = sz + psz;
-
- sz += term_len(tp, 2);
- d += term_len(tp, 1);
padl = col->decimal - d;
--
To unsubscribe send an email to discuss+unsubscribe@mdocml.bsd.lv
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: tbl issues with 1.11.1
2011-05-14 4:12 ` Ingo Schwarze
@ 2011-09-12 20:11 ` Yuri Pankov
2011-09-18 17:25 ` Yuri Pankov
0 siblings, 1 reply; 9+ messages in thread
From: Yuri Pankov @ 2011-09-12 20:11 UTC (permalink / raw)
To: discuss
On Sat, May 14, 2011 at 06:12:28AM +0200, Ingo Schwarze wrote:
> Hi Kristaps and Joerg,
>
> >>> Ingo, that's your change from January. Care to check the attached patch?
> >> Anyone?
> > Ingo? I remember you checking these in... care to take another look?
>
> Er, jaja, i was a bit swamped...
>
> Unfortunately, Joergs patch is not right, i breaks the horizontal
> spacing in tables without vertical rulers almost completely, see
> for example captoinfo(1) and compare the groff and mandoc output.
>
> Getting this right in both cases - with and without vertical rulers -
> requires rather large changes. The logic, as far as i understand it
> so far, goes like this:
>
> - box produces "|"
> - no spacing at this point
> - first cell, width is max(lengths)
> - one space
> - vertical ruler, if none, another space
> - another space, unless there was a double ruler (!!)
> - second cell
> - and so on.
>
> That is vastly different from what we have now, both regarding the
> width calculations - which apparently must not include padding - and
> regarding the output itself.
>
> The work-in-progress patch below comes closer. However, it still
> has at least two deficiencies, so it is not ready for commit:
> - There still is a bogus space after double rulers, see (!!).
> - The handling of explicit spacing options got sweeped away
> by the reorg i was forced to do. Probably, i need to put
> it back in at the right places. It was in the calc routine
> but probably belongs in the output routines themselves.
>
> Just sending this such that you finally get some feedback.
> In case we need more discussion, we should probably move to tech@.
Sorry for bringing this up again, but I'm still seeing the same
behaviour in 1.11.6. Is there anything wrong with proposed patch?
Yuri
--
To unsubscribe send an email to discuss+unsubscribe@mdocml.bsd.lv
^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: tbl issues with 1.11.1
2011-09-12 20:11 ` Yuri Pankov
@ 2011-09-18 17:25 ` Yuri Pankov
0 siblings, 0 replies; 9+ messages in thread
From: Yuri Pankov @ 2011-09-18 17:25 UTC (permalink / raw)
To: discuss
On Tue, Sep 13, 2011 at 12:11:30AM +0400, Yuri Pankov wrote:
> On Sat, May 14, 2011 at 06:12:28AM +0200, Ingo Schwarze wrote:
> > Hi Kristaps and Joerg,
> >
> > >>> Ingo, that's your change from January. Care to check the attached patch?
> > >> Anyone?
> > > Ingo? I remember you checking these in... care to take another look?
> >
> > Er, jaja, i was a bit swamped...
> >
> > Unfortunately, Joergs patch is not right, i breaks the horizontal
> > spacing in tables without vertical rulers almost completely, see
> > for example captoinfo(1) and compare the groff and mandoc output.
> >
> > Getting this right in both cases - with and without vertical rulers -
> > requires rather large changes. The logic, as far as i understand it
> > so far, goes like this:
> >
> > - box produces "|"
> > - no spacing at this point
> > - first cell, width is max(lengths)
> > - one space
> > - vertical ruler, if none, another space
> > - another space, unless there was a double ruler (!!)
> > - second cell
> > - and so on.
> >
> > That is vastly different from what we have now, both regarding the
> > width calculations - which apparently must not include padding - and
> > regarding the output itself.
> >
> > The work-in-progress patch below comes closer. However, it still
> > has at least two deficiencies, so it is not ready for commit:
> > - There still is a bogus space after double rulers, see (!!).
> > - The handling of explicit spacing options got sweeped away
> > by the reorg i was forced to do. Probably, i need to put
> > it back in at the right places. It was in the calc routine
> > but probably belongs in the output routines themselves.
> >
> > Just sending this such that you finally get some feedback.
> > In case we need more discussion, we should probably move to tech@.
>
> Sorry for bringing this up again, but I'm still seeing the same
> behaviour in 1.11.6. Is there anything wrong with proposed patch?
And sorry for sending this again, but I'm not sure it got through due do
the site downtime...
Yuri
--
To unsubscribe send an email to discuss+unsubscribe@mdocml.bsd.lv
^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2011-09-18 17:25 UTC | newest]
Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-04-14 1:45 tbl issues with 1.11.1 Yuri Pankov
2011-04-16 21:34 ` Ingo Schwarze
2011-04-26 20:10 ` Joerg Sonnenberger
2011-05-12 22:02 ` Joerg Sonnenberger
2011-05-13 21:03 ` Kristaps Dzonsons
2011-05-14 4:12 ` Ingo Schwarze
2011-09-12 20:11 ` Yuri Pankov
2011-09-18 17:25 ` Yuri Pankov
2011-04-17 0:12 ` Joerg Sonnenberger
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).