TERMP_NOLPAD r.i.p.

[Ingo Schwarze <schwarze@usta.de>]

Hi,

Kristaps drew my attention to the fact that the popt(3) manual
is badly broken:

> .HP
> .nf
> .BI "int poptReadDefaultConfig(poptContext " con ", int " flags ");"
> .fi
> 
> This looks fine in `ul', but on the raw screen it prints out lots of
> blank space due to the flush.  Of course, it's because rmargin is
> set to TERM_MAXMARGIN and the TERMP_NOBREAK goes nuts (in
> print_man_node()). This can be hacked around by NOT setting rmargin
> and only setting maxrmargin, but it still looks crappy.  Any
> thoughts?

Yes, i have come up with the following diff which might be one of the
most beautiful i have written for mandoc so far:  I suggest to
enhance functionality and squash bugs - by removing a flag!

Specifically, as long as we have TERMP_NOLPAD, the problem at hand
is very hard to solve, in particular in the following variant:

  .nf
  .HP 12n
  This is a very long tag.
  .fi
  This is the indented text; it is of considerable length as well.

With TERMP_NOLPAD, while rendering the first line of the .HP,
term_flushln() - yes, once again my favourite function -
overruns the column, breaks the line, and pads to the indent.
After that, the .fi breaks the already padded line - ouch.

To improve this, we should not pad until we know that we want
to write actual content to a line, but let the next call to
term_flushln() do the padding instead; which means that we don't
need TERMP_NOLPAD any longer.

Some of the chunks are non-obvious:

 * term.c chunk @@ -130,9 +126,10 @@:
   Always calculate the padding before starting a new line.
   No more TERMP_NOLPAD.
 * term.c chunk @@ -232,10 +229,14 @@:
   An independent bugfix.  Backspace has negative length.
   Without this, indentation does not get right.
 * term.c chunk @@ -244,7 +245,8 @@:
   Handle the edge case of an empty input string:
   In that case, vis is still zero, and there is no need to go back.
   This occurs e.g. in man(7) page headers and footers.
 * term.c chunk @@ -269,25 +271,18 @@:
   This is the main point.
   No more padding at the end of term_flushln().
 * Most other chunks just drop the obsolete flag.
 * man_term.c chunk @@ -241,6 +241,18 @@:
   That's the main fix for .HP indentation containing .fi/.nf,
   and the only ugly chunk in this diff.
   See the comment in the code what it does.
 * man_term.c chunk @@ -452,8 +464,11 @@:
   This adjusts the indentation width to what groff does.
 * man_term.c chunk @@ -878,7 +889,7 @@:
   The right margin must not be unlimited in literal mode
   when TERMP_NOBREAK is in effect.  That avoids the long string
   of blank characters.

This patch is -65 +50 :).

It reduces groff/mandoc differences in base by nearly 20%,
from 89k to 72k lines of diffs.

I see no regressions.

OK?
  Ingo


Index: term.h
===================================================================
RCS file: /cvs/src/usr.bin/mandoc/term.h,v
retrieving revision 1.31
diff -u -p -r1.31 term.h
--- term.h	18 Sep 2011 10:25:28 -0000	1.31
+++ term.h	19 Sep 2011 21:29:38 -0000
@@ -64,7 +64,6 @@ struct	termp {
 	int		  flags;
 #define	TERMP_SENTENCE	 (1 << 1)	/* Space before a sentence. */
 #define	TERMP_NOSPACE	 (1 << 2)	/* No space before words. */
-#define	TERMP_NOLPAD	 (1 << 3)	/* See term_flushln(). */
 #define	TERMP_NOBREAK	 (1 << 4)	/* See term_flushln(). */
 #define	TERMP_IGNDELIM	 (1 << 6)	/* Delims like regulars. */
 #define	TERMP_NONOSPACE	 (1 << 7)	/* No space (no autounset). */
Index: term.c
===================================================================
RCS file: /cvs/src/usr.bin/mandoc/term.c,v
retrieving revision 1.60
diff -u -p -r1.60 term.c
--- term.c	18 Sep 2011 20:38:02 -0000	1.60
+++ term.c	19 Sep 2011 21:29:38 -0000
@@ -75,22 +75,18 @@ term_end(struct termp *p)
  *
  * The following flags may be specified:
  *
- *  - TERMP_NOLPAD: when beginning to write the line, don't left-pad the
- *    offset value.  This is useful when doing columnar lists where the
- *    prior column has right-padded.
- *
  *  - TERMP_NOBREAK: this is the most important and is used when making
- *    columns.  In short: don't print a newline and instead pad to the
- *    right margin.  Used in conjunction with TERMP_NOLPAD.
+ *    columns.  In short: don't print a newline and instead expect the
+ *    next call to do the padding up to the start of the next column.
  *
- *  - TERMP_TWOSPACE: when padding, make sure there are at least two
- *    space characters of padding.  Otherwise, rather break the line.
+ *  - TERMP_TWOSPACE: make sure there is room for at least two space
+ *    characters of padding.  Otherwise, rather break the line.
  *
  *  - TERMP_DANGLE: don't newline when TERMP_NOBREAK is specified and
  *    the line is overrun, and don't pad-right if it's underrun.
  *
  *  - TERMP_HANG: like TERMP_DANGLE, but doesn't newline when
- *    overruning, instead save the position and continue at that point
+ *    overrunning, instead save the position and continue at that point
  *    when the next invocation.
  *
  *  In-line line breaking:
@@ -130,9 +126,10 @@ term_flushln(struct termp *p)
 	bp = TERMP_NOBREAK & p->flags ? mmax : maxvis;
 
 	/*
-	 * Indent the first line of a paragraph.
+	 * Calculate the required amount of padding.
 	 */
-	vbl = p->flags & TERMP_NOLPAD ? (size_t)0 : p->offset;
+	vbl = p->offset + p->overstep > p->viscol ?
+	      p->offset + p->overstep - p->viscol : 0;
 
 	vis = vend = 0;
 	i = 0;
@@ -232,10 +229,14 @@ term_flushln(struct termp *p)
 			if (ASCII_HYPH == p->buf[i]) {
 				(*p->letter)(p, '-');
 				p->viscol += (*p->width)(p, '-');
-			} else {
-				(*p->letter)(p, p->buf[i]);
-				p->viscol += (*p->width)(p, p->buf[i]);
+				continue;
 			}
+
+			(*p->letter)(p, p->buf[i]);
+			if (8 == p->buf[i])
+				p->viscol -= (*p->width)(p, p->buf[i-1]);
+			else 
+				p->viscol += (*p->width)(p, p->buf[i]);
 		}
 		vis = vend;
 	}
@@ -244,7 +245,8 @@ term_flushln(struct termp *p)
 	 * If there was trailing white space, it was not printed;
 	 * so reset the cursor position accordingly.
 	 */
-	vis -= vbl;
+	if (vis)
+		vis -= vbl;
 
 	p->col = 0;
 	p->overstep = 0;
@@ -269,25 +271,18 @@ term_flushln(struct termp *p)
 		 * move it one step LEFT and flag the rest of the line
 		 * to be longer.
 		 */
-		if (p->overstep >= -1) {
-			assert((int)maxvis + p->overstep >= 0);
-			maxvis += (size_t)p->overstep;
-		} else
+		if (p->overstep < -1)
 			p->overstep = 0;
+		return;
 
 	} else if (TERMP_DANGLE & p->flags)
 		return;
 
-	/* Right-pad. */
-	if (maxvis > vis +
+	/* If the column was overrun, break the line. */
+	if (maxvis <= vis +
 	    ((TERMP_TWOSPACE & p->flags) ? (*p->width)(p, ' ') : 0)) {
-		p->viscol += maxvis - vis;
-		(*p->advance)(p, maxvis - vis);
-		vis += (maxvis - vis);
-	} else {	/* ...or newline break. */
 		(*p->endline)(p);
-		p->viscol = p->rmargin;
-		(*p->advance)(p, p->rmargin);
+		p->viscol = 0;
 	}
 }
 
@@ -302,12 +297,8 @@ term_newln(struct termp *p)
 {
 
 	p->flags |= TERMP_NOSPACE;
-	if (0 == p->col && 0 == p->viscol) {
-		p->flags &= ~TERMP_NOLPAD;
-		return;
-	}
-	term_flushln(p);
-	p->flags &= ~TERMP_NOLPAD;
+	if (p->col || p->viscol)
+		term_flushln(p);
 }
 
 
Index: mdoc_term.c
===================================================================
RCS file: /cvs/src/usr.bin/mandoc/mdoc_term.c,v
retrieving revision 1.135
diff -u -p -r1.135 mdoc_term.c
--- mdoc_term.c	18 Sep 2011 10:25:28 -0000	1.135
+++ mdoc_term.c	19 Sep 2011 21:29:38 -0000
@@ -431,7 +431,7 @@ print_mdoc_foot(struct termp *p, const v
 
 	p->offset = p->rmargin;
 	p->rmargin = p->maxrmargin - term_strlen(p, m->os);
-	p->flags |= TERMP_NOLPAD | TERMP_NOSPACE;
+	p->flags |= TERMP_NOSPACE;
 
 	term_word(p, m->date);
 	term_flushln(p);
@@ -439,7 +439,7 @@ print_mdoc_foot(struct termp *p, const v
 	p->offset = p->rmargin;
 	p->rmargin = p->maxrmargin;
 	p->flags &= ~TERMP_NOBREAK;
-	p->flags |= TERMP_NOLPAD | TERMP_NOSPACE;
+	p->flags |= TERMP_NOSPACE;
 
 	term_word(p, m->os);
 	term_flushln(p);
@@ -495,7 +495,7 @@ print_mdoc_head(struct termp *p, const v
 
 	p->offset = p->rmargin;
 	p->rmargin = p->maxrmargin - term_strlen(p, title);
-	p->flags |= TERMP_NOLPAD | TERMP_NOSPACE;
+	p->flags |= TERMP_NOSPACE;
 
 	term_word(p, buf);
 	term_flushln(p);
@@ -503,7 +503,7 @@ print_mdoc_head(struct termp *p, const v
 	p->offset = p->rmargin;
 	p->rmargin = p->maxrmargin;
 	p->flags &= ~TERMP_NOBREAK;
-	p->flags |= TERMP_NOLPAD | TERMP_NOSPACE;
+	p->flags |= TERMP_NOSPACE;
 
 	term_word(p, title);
 	term_flushln(p);
@@ -783,16 +783,11 @@ termp_it_pre(DECL_ARGS)
 	case (LIST_hyphen):
 		if (MDOC_HEAD == n->type)
 			p->flags |= TERMP_NOBREAK;
-		else
-			p->flags |= TERMP_NOLPAD;
 		break;
 	case (LIST_hang):
 		if (MDOC_HEAD == n->type)
 			p->flags |= TERMP_NOBREAK;
 		else
-			p->flags |= TERMP_NOLPAD;
-
-		if (MDOC_HEAD != n->type)
 			break;
 
 		/*
@@ -803,17 +798,14 @@ termp_it_pre(DECL_ARGS)
 		 */
 		if (n->next->child && 
 				(MDOC_Bl == n->next->child->tok ||
-				 MDOC_Bd == n->next->child->tok)) {
+				 MDOC_Bd == n->next->child->tok))
 			p->flags &= ~TERMP_NOBREAK;
-			p->flags &= ~TERMP_NOLPAD;
-		} else
+		else
 			p->flags |= TERMP_HANG;
 		break;
 	case (LIST_tag):
 		if (MDOC_HEAD == n->type)
 			p->flags |= TERMP_NOBREAK | TERMP_TWOSPACE;
-		else
-			p->flags |= TERMP_NOLPAD;
 
 		if (MDOC_HEAD != n->type)
 			break;
@@ -829,10 +821,6 @@ termp_it_pre(DECL_ARGS)
 		else
 			p->flags |= TERMP_NOBREAK;
 
-		assert(n->prev);
-		if (MDOC_BODY == n->prev->type) 
-			p->flags |= TERMP_NOLPAD;
-
 		break;
 	case (LIST_diag):
 		if (MDOC_HEAD == n->type)
@@ -989,7 +977,6 @@ termp_it_post(DECL_ARGS)
 	p->flags &= ~TERMP_DANGLE;
 	p->flags &= ~TERMP_NOBREAK;
 	p->flags &= ~TERMP_TWOSPACE;
-	p->flags &= ~TERMP_NOLPAD;
 	p->flags &= ~TERMP_HANG;
 }
 
@@ -1005,7 +992,7 @@ termp_nm_pre(DECL_ARGS)
 	if (MDOC_BODY == n->type) {
 		if (NULL == n->child)
 			return(0);
-		p->flags |= TERMP_NOLPAD | TERMP_NOSPACE;
+		p->flags |= TERMP_NOSPACE;
 		p->offset += term_len(p, 1) +
 		    (NULL == n->prev->child ? term_strlen(p, m->name) :
 		     MDOC_TEXT == n->prev->child->type ?
@@ -1050,10 +1037,8 @@ termp_nm_post(DECL_ARGS)
 	if (MDOC_HEAD == n->type && n->next->child) {
 		term_flushln(p);
 		p->flags &= ~(TERMP_NOBREAK | TERMP_HANG);
-	} else if (MDOC_BODY == n->type && n->child) {
+	} else if (MDOC_BODY == n->type && n->child)
 		term_flushln(p);
-		p->flags &= ~TERMP_NOLPAD;
-	}
 }
 
 		
Index: man_term.c
===================================================================
RCS file: /cvs/src/usr.bin/mandoc/man_term.c,v
retrieving revision 1.71
diff -u -p -r1.71 man_term.c
--- man_term.c	18 Sep 2011 10:25:28 -0000	1.71
+++ man_term.c	19 Sep 2011 21:29:38 -0000
@@ -241,6 +241,18 @@ pre_literal(DECL_ARGS)
 	else
 		mt->fl &= ~MANT_LITERAL;
 
+	/*
+	 * Unlike .IP and .TP, .HP does not have a HEAD.
+	 * So in case a second call to term_flushln() is needed,
+	 * indentation has to be set up explicitly.
+	 */
+	if (MAN_HP == n->parent->tok && p->rmargin < p->maxrmargin) {
+		p->offset = p->rmargin + 1;
+		p->rmargin = p->maxrmargin;
+		p->flags &= ~(TERMP_NOBREAK | TERMP_TWOSPACE);
+		p->flags |= TERMP_NOSPACE;
+	}
+
 	return(0);
 }
 
@@ -427,7 +439,7 @@ pre_sp(DECL_ARGS)
 static int
 pre_HP(DECL_ARGS)
 {
-	size_t			 len;
+	size_t			 len, one;
 	int			 ival;
 	const struct man_node	*nn;
 
@@ -452,8 +464,11 @@ pre_HP(DECL_ARGS)
 		if ((ival = a2width(p, nn->string)) >= 0)
 			len = (size_t)ival;
 
-	if (0 == len)
-		len = term_len(p, 1);
+	one = term_len(p, 1);
+	if (len > one)
+		len -= one;
+	else
+		len = one;
 
 	p->offset = mt->offset;
 	p->rmargin = mt->offset + len;
@@ -516,7 +531,6 @@ pre_IP(DECL_ARGS)
 
 	switch (n->type) {
 	case (MAN_BODY):
-		p->flags |= TERMP_NOLPAD;
 		p->flags |= TERMP_NOSPACE;
 		break;
 	case (MAN_HEAD):
@@ -587,7 +601,6 @@ post_IP(DECL_ARGS)
 		break;
 	case (MAN_BODY):
 		term_newln(p);
-		p->flags &= ~TERMP_NOLPAD;
 		break;
 	default:
 		break;
@@ -608,7 +621,6 @@ pre_TP(DECL_ARGS)
 		p->flags |= TERMP_NOBREAK;
 		break;
 	case (MAN_BODY):
-		p->flags |= TERMP_NOLPAD;
 		p->flags |= TERMP_NOSPACE;
 		break;
 	case (MAN_BLOCK):
@@ -677,7 +689,6 @@ post_TP(DECL_ARGS)
 		break;
 	case (MAN_BODY):
 		term_newln(p);
-		p->flags &= ~TERMP_NOLPAD;
 		break;
 	default:
 		break;
@@ -878,7 +889,7 @@ print_man_node(DECL_ARGS)
 		 * -man doesn't have nested macros, we don't need to be
 		 * more specific than this.
 		 */
-		if (MANT_LITERAL & mt->fl && 
+		if (MANT_LITERAL & mt->fl && ! (TERMP_NOBREAK & p->flags) &&
 				(NULL == n->next || 
 				 n->next->line > n->line)) {
 			rm = p->rmargin;
@@ -886,7 +897,6 @@ print_man_node(DECL_ARGS)
 			p->rmargin = p->maxrmargin = TERM_MAXMARGIN;
 			p->flags |= TERMP_NOSPACE;
 			term_flushln(p);
-			p->flags &= ~TERMP_NOLPAD;
 			p->rmargin = rm;
 			p->maxrmargin = rmax;
 		}
@@ -968,7 +978,7 @@ print_man_foot(struct termp *p, const vo
 		term_word(p, "");
 	term_flushln(p);
 
-	p->flags |= TERMP_NOLPAD | TERMP_NOSPACE;
+	p->flags |= TERMP_NOSPACE;
 	p->offset = p->rmargin;
 	p->rmargin = p->maxrmargin;
 	p->flags &= ~TERMP_NOBREAK;
@@ -1015,7 +1025,7 @@ print_man_head(struct termp *p, const vo
 	term_word(p, title);
 	term_flushln(p);
 
-	p->flags |= TERMP_NOLPAD | TERMP_NOSPACE;
+	p->flags |= TERMP_NOSPACE;
 	p->offset = p->rmargin;
 	p->rmargin = p->offset + buflen + titlen < p->maxrmargin ?
 	    p->maxrmargin - titlen : p->maxrmargin;
@@ -1025,7 +1035,7 @@ print_man_head(struct termp *p, const vo
 
 	p->flags &= ~TERMP_NOBREAK;
 	if (p->rmargin + titlen <= p->maxrmargin) {
-		p->flags |= TERMP_NOLPAD | TERMP_NOSPACE;
+		p->flags |= TERMP_NOSPACE;
 		p->offset = p->rmargin;
 		p->rmargin = p->maxrmargin;
 		term_word(p, title);
--
 To unsubscribe send an email to tech+unsubscribe@mdocml.bsd.lv