PATCH: fix vertical spacing in nested lists

PATCH: fix vertical spacing in nested lists

[Ingo Schwarze]

[-- Attachment #1: Type: text/plain, Size: 6259 bytes --]

Hi Joerg and Kristaps,

OK, so now i'm producing stacked diffs.

The following will only apply *after* you have applied
05.tabbing.patch.  It touches more or less the same lines
of the same files.

There is no clear corresponding commit in OpenBSD, it needed
several iterations to get this right.  In particular,

term.c 1.23:

> When the last field on an output line is empty, break the line
> if and only if something was printed on that line.
> This avoids double line breaks after nested lists
> while still breaking lines after items with empty body.

term.c 1.28, mdoc_term.c 1.74:

> Partial revert of term.c rev. 1.23
> because jmc@ noticed that it broke blank lines in literal displays.
>
> The original idea was to suppress stray blank lines.
> But we don't want to suppress *all* blank lines,
> instead just those caused by nested lists.
> So do the check whether there was any output on this line,
> i.e. whether or not to break the line,
> at the right place, which is after processing the .It body.

term.c 1.29, term.h 1.15:

> Fix rendering of multiple successive .It macros without intervening text;
> another problem reported by jmc@.
>
> The physical output line may contain output from more than one buffer.
> Thus, to decide whether a line break is needed, it's insufficient to
> only look at the number of bytes in the current output buffer.
> Keep track of the number of characters already written, too.


Joerg, you may remeber we discussed this while walking along the
southern and western edge of the village of Nienhagen just before
reaching the Baltic sea.  From a layering point of view, it may
be better to do this half a layer upwards, that is not in term_flushln(),
but in bufferc(), which would require a trivial rewrite of buffera()
in terms of bufferc().  I doubt it can conveniently be done a full
layer upwards, that is on the layer of mdoc(7) macros, because
it is not at all obvious on that layer whether a macro will or will
not generate non-whitespace output with the given output device
settings at the end of the day.

So, it would be useful to try and write an implementation setting the
flag in bufferc() instead of term_flushln().  If this gets done quickly
and turns out to be better than this one, i will happily throw this one
away and also replace it in the OpenBSD tree.  If this kind of
replacement needs more thought or time, no problem either, then we
can come back to it later when we are not in a hurry.  In that case,
i propose to use this one for now:  It works and is only mildly ugly,
and it will get us in sync.

For now, personally, i'm focussing on getting my other patches ready
for commit, so we can reach synced state asap.

This diff is also available from
  /usr/vhosts/mdocml.bsd.lv/patch/schwarze/08.viscol.patch

A test page is attached, covering only the nesting case.
Testing the empty .It case in .Bl -tag is easy anyway.

Thoughts?

Yours,
  Ingo


--- term.h	Mon May 24 16:54:52 2010
+++ term.h	Sat May 15 23:09:53 2010
@@ -39,6 +39,7 @@ struct	termp {
 	size_t		  offset;	/* Margin offest. */
 	size_t		  tabwidth;	/* Distance of tab positions. */
 	size_t		  col;		/* Bytes in buf. */
+	size_t		  viscol;	/* Chars on current line. */
 	int		  overstep;	/* See termp_flushln(). */
 	int		  flags;
 #define	TERMP_SENTENCE	 (1 << 1)	/* Space before a sentence. */
--- term.c	Mon May 24 16:54:52 2010
+++ term.c	Mon May 24 00:45:01 2010
@@ -190,27 +187,29 @@ term_flushln(struct termp *p)
 		 */
 
 		/* LINTED */
 		for ( ; j < (int)p->col; j++) {
 			if ((j && ' ' == p->buf[j]) || '\t' == p->buf[j])
 				break;
 			if (8 == p->buf[j])
 				vend--;
 			else
 				vend++;
 		}
 
 		/*
 		 * Find out whether we would exceed the right margin.
 		 * If so, break to the next line.
 		 */
 		if (vend > bp && vis > 0) {
 			vend -= vis;
 			putchar('\n');
 			if (TERMP_NOBREAK & p->flags) {
+				p->viscol = p->rmargin;
 				for (j = 0; j < (int)p->rmargin; j++)
 					putchar(' ');
 				vend += p->rmargin - p->offset;
 			} else {
+				p->viscol = 0;
 				vbl = p->offset;
 			}
 
@@ -251,9 +256,11 @@ term_flushln(struct termp *p)
 			if (vbl) {
 				for (j = 0; j < (int)vbl; j++)
 					putchar(' ');
+				p->viscol += vbl;
 				vbl = 0;
 			}
 			putchar(p->buf[i]);
+			p->viscol += 1;
 		}
 		vend += vbl;
 		vis = vend;
@@ -263,6 +270,7 @@ term_flushln(struct termp *p)
 	p->overstep = 0;
 
 	if ( ! (TERMP_NOBREAK & p->flags)) {
+		p->viscol = 0;
 		putchar('\n');
 		return;
 	}
@@ -294,11 +302,13 @@ term_flushln(struct termp *p)
 
 	/* Right-pad. */
 	if (maxvis > vis + /* LINTED */
-			((TERMP_TWOSPACE & p->flags) ? 1 : 0))  
+			((TERMP_TWOSPACE & p->flags) ? 1 : 0)) {
+		p->viscol += maxvis - vis;
 		for ( ; vis < maxvis; vis++)
 			putchar(' ');
-	else {	/* ...or newline break. */
+	} else {	/* ...or newline break. */
 		putchar('\n');
+		p->viscol = p->rmargin;
 		for (i = 0; i < (int)p->rmargin; i++)
 			putchar(' ');
 	}
@@ -315,7 +325,7 @@ term_newln(struct termp *p)
 {
 
 	p->flags |= TERMP_NOSPACE;
-	if (0 == p->col) {
+	if (0 == p->col && 0 == p->viscol) {
 		p->flags &= ~TERMP_NOLPAD;
 		return;
 	}
@@ -335,6 +345,7 @@ term_vspace(struct termp *p)
 {
 
 	term_newln(p);
+	p->viscol = 0;
 	putchar('\n');
 }
 
--- mdoc_term.c	Mon May 24 16:54:52 2010
+++ mdoc_term.c	Mon May 24 19:10:03 2010
@@ -1014,14 +1010,14 @@ termp_it_post(DECL_ARGS)
 		/* FALLTHROUGH */
 	case (LIST_inset):
 		if (MDOC_BODY == n->type)
-			term_flushln(p);
+			term_newln(p);
 		break;
 	case (LIST_column):
 		if (MDOC_HEAD == n->type)
 			term_flushln(p);
 		break;
 	default:
-		term_flushln(p);
+		term_newln(p);
 		break;
 	}
 
@@ -1633,12 +1629,10 @@ termp_bd_pre(DECL_ARGS)
 	for (nn = n->child; nn; nn = nn->next) {
 		p->flags |= TERMP_NOSPACE;
 		print_mdoc_node(p, pair, m, nn);
-		if (NULL == nn->next)
-			continue;
-		if (nn->prev && nn->prev->line < nn->line)
+		if (NULL == nn->prev ||
+		    nn->prev->line < nn->line ||
+		    NULL == nn->next)
 			term_flushln(p);
-		else if (NULL == nn->prev)
-			term_flushln(p);
 	}
 	p->tabwidth = tabwidth;
 
@@ -1668,7 +1662,7 @@ termp_bd_post(DECL_ARGS)
 		p->rmargin = p->maxrmargin = TERM_MAXMARGIN;
 
 	p->flags |= TERMP_NOSPACE;
-	term_flushln(p);
+	term_newln(p);
 
 	p->rmargin = rm;
 	p->maxrmargin = rmax;

[-- Attachment #2: nested.in --]
[-- Type: text/plain, Size: 432 bytes --]

.Dd $Mdocdate: April 12 2010 $
.Dt BL-NESTED 1
.Os
.Sh NAME
.Nm Bl-nested
.Nd nested lists
.Sh DESCRIPTION
inset:
.Bl -inset
.It outer
list
.Bl -inset
.It inner
list
.El
.El
inset compact:
.Bl -inset -compact
.It outer
list
.Bl -inset -compact
.It inner
list
.El
.El
dash:
.Bl -dash
.It
outer list
.Bl -dash
.It
inner list
.El
.El
dash compact:
.Bl -dash -compact
.It
outer list
.Bl -dash -compact
.It
inner list
.El
.El
final text

Re: PATCH: fix vertical spacing in nested lists

[Kristaps Dzonsons]

> The following will only apply *after* you have applied
> 05.tabbing.patch.  It touches more or less the same lines
> of the same files.
> 
> There is no clear corresponding commit in OpenBSD, it needed
> several iterations to get this right.  In particular,
> 
> term.c 1.23:
> 
>> When the last field on an output line is empty, break the line
>> if and only if something was printed on that line.
>> This avoids double line breaks after nested lists
>> while still breaking lines after items with empty body.
> 
> term.c 1.28, mdoc_term.c 1.74:
> 
>> Partial revert of term.c rev. 1.23
>> because jmc@ noticed that it broke blank lines in literal displays.
>>
>> The original idea was to suppress stray blank lines.
>> But we don't want to suppress *all* blank lines,
>> instead just those caused by nested lists.
>> So do the check whether there was any output on this line,
>> i.e. whether or not to break the line,
>> at the right place, which is after processing the .It body.
> 
> term.c 1.29, term.h 1.15:
> 
>> Fix rendering of multiple successive .It macros without intervening text;
>> another problem reported by jmc@.
>>
>> The physical output line may contain output from more than one buffer.
>> Thus, to decide whether a line break is needed, it's insufficient to
>> only look at the number of bytes in the current output buffer.
>> Keep track of the number of characters already written, too.
> 
> 
> Joerg, you may remeber we discussed this while walking along the
> southern and western edge of the village of Nienhagen just before
> reaching the Baltic sea.  From a layering point of view, it may
> be better to do this half a layer upwards, that is not in term_flushln(),
> but in bufferc(), which would require a trivial rewrite of buffera()
> in terms of bufferc().  I doubt it can conveniently be done a full
> layer upwards, that is on the layer of mdoc(7) macros, because
> it is not at all obvious on that layer whether a macro will or will
> not generate non-whitespace output with the given output device
> settings at the end of the day.
> 
> So, it would be useful to try and write an implementation setting the
> flag in bufferc() instead of term_flushln().  If this gets done quickly
> and turns out to be better than this one, i will happily throw this one
> away and also replace it in the OpenBSD tree.  If this kind of
> replacement needs more thought or time, no problem either, then we
> can come back to it later when we are not in a hurry.  In that case,
> i propose to use this one for now:  It works and is only mildly ugly,
> and it will get us in sync.
> 
> For now, personally, i'm focussing on getting my other patches ready
> for commit, so we can reach synced state asap.
> 
> This diff is also available from
>   /usr/vhosts/mdocml.bsd.lv/patch/schwarze/08.viscol.patch
> 
> A test page is attached, covering only the nesting case.
> Testing the empty .It case in .Bl -tag is easy anyway.

I haven't tested these and will wait until it's committed to do fallout 
tests, but the logic looks fine by me, so go ahead and commit.  The more 
in-code documentation (and regression tests) in these places the better, 
as I for one get a bit lost in flushln().
--
 To unsubscribe send an email to tech+unsubscribe@mdocml.bsd.lv