Bugs and regressions in .Bk

Bugs and regressions in .Bk

[Ingo Schwarze]

Hi Kristaps,

> Log Message:
> -----------
> Arguments to `Bsx' and friends are separated by a non-breaking space.
> This removes a TODO raised by schwarze@.

While trying to merge this patch, i ran into a regression:
When called inside .Bk or in the SYNOPSIS, this patch ends
keep mode.

Trying to fix that (meanwhile, i have a working fix, so don't
waste your time searching for it), i ran into another .Bk bug:
For text generated in the pre-handler, setting up keep state
comes too late to take effect.

So, my plan is to

 1) Get the following patch in, fixing the latter bug - OK?
    The code is unchanged, it is just moved around.

 2) Once that's in, merge your patch to OpenBSD, together
    with my fix for the regression.

 3) Finally, merge back my regression fix to bsd.lv.

Yours,
  Ingo


Index: usr.bin/mandoc/mdoc_term.c
===================================================================
RCS file: /cvs/src/usr.bin/mandoc/mdoc_term.c,v
retrieving revision 1.127
diff -u -p -r1.127 mdoc_term.c
--- usr.bin/mandoc/mdoc_term.c	6 Feb 2011 17:33:20 -0000	1.127
+++ usr.bin/mandoc/mdoc_term.c	6 Feb 2011 20:32:04 -0000
@@ -308,22 +308,6 @@ print_mdoc_node(DECL_ARGS)
 
 	memset(&npair, 0, sizeof(struct termpair));
 	npair.ppair = pair;
-	
-	switch (n->type) {
-	case (MDOC_TEXT):
-		if (' ' == *n->string && MDOC_LINE & n->flags)
-			term_newln(p);
-		term_word(p, n->string);
-		break;
-	case (MDOC_TBL):
-		term_tbl(p, n->span);
-		break;
-	default:
-		if (termacts[n->tok].pre && ENDBODY_NOT == n->end)
-			chld = (*termacts[n->tok].pre)
-				(p, &npair, m, n);
-		break;
-	}
 
 	/*
 	 * Keeps only work until the end of a line.  If a keep was
@@ -354,6 +338,22 @@ print_mdoc_node(DECL_ARGS)
 	    ((n->prev   && MDOC_SYNPRETTY & n->prev->flags) ||
 	     (n->parent && MDOC_SYNPRETTY & n->parent->flags)))
 		p->flags &= ~(TERMP_KEEP | TERMP_PREKEEP);
+
+	switch (n->type) {
+	case (MDOC_TEXT):
+		if (' ' == *n->string && MDOC_LINE & n->flags)
+			term_newln(p);
+		term_word(p, n->string);
+		break;
+	case (MDOC_TBL):
+		term_tbl(p, n->span);
+		break;
+	default:
+		if (termacts[n->tok].pre && ENDBODY_NOT == n->end)
+			chld = (*termacts[n->tok].pre)
+				(p, &npair, m, n);
+		break;
+	}
 
 	if (chld && n->child)
 		print_mdoc_nodelist(p, &npair, m, n->child);
Index: regress/usr.bin/mandoc/mdoc/Bk/break.in
===================================================================
RCS file: /cvs/src/regress/usr.bin/mandoc/mdoc/Bk/break.in,v
retrieving revision 1.1
diff -u -p -r1.1 break.in
--- regress/usr.bin/mandoc/mdoc/Bk/break.in	19 Dec 2010 12:10:33 -0000	1.1
+++ regress/usr.bin/mandoc/mdoc/Bk/break.in	6 Feb 2011 20:32:04 -0000
@@ -5,6 +5,24 @@
 .Nm Bk-break
 .Nd handling of word keeps
 .Sh DESCRIPTION
+Even though it is called a word keep,
+.Bk -words
+it will not keep words together in free-form text.
+.Ek
+However, even the noop macro
+.Bk -words
+.No is sufficient to let the keep take effect .
+.Ek
+Even text generated in pre-handlers must be kept together with
+the text following it:
+.Bk -words
+.Xr one 1 No and Xr two 2
+.Ek
+should be on the same line.
+.Pp
+Including the whole output line into the keep is not required,
+including just the macros to be kept together is sufficient:
+.Pp
 .Nm
 .Ar x x x x x x x x
 .Ar x x x x x x x x
Index: regress/usr.bin/mandoc/mdoc/Bk/break.out_ascii
===================================================================
RCS file: /cvs/src/regress/usr.bin/mandoc/mdoc/Bk/break.out_ascii,v
retrieving revision 1.1
diff -u -p -r1.1 break.out_ascii
--- regress/usr.bin/mandoc/mdoc/Bk/break.out_ascii	19 Dec 2010 12:10:33 -0000	1.1
+++ regress/usr.bin/mandoc/mdoc/Bk/break.out_ascii	6 Feb 2011 20:32:04 -0000
@@ -4,6 +4,15 @@ N\bNA\bAM\bME\bE
      B\bBk\bk-\b-b\bbr\bre\bea\bak\bk - handling of word keeps
 
 D\bDE\bES\bSC\bCR\bRI\bIP\bPT\bTI\bIO\bON\bN
+     Even though it is called a word keep, it will not keep words together in
+     free-form text.  However, even the noop macro
+     is sufficient to let the keep take effect.  Even text generated in pre-
+     handlers must be kept together with the text following it:
+     one(1) and two(2) should be on the same line.
+
+     Including the whole output line into the keep is not required, including
+     just the macros to be kept together is sufficient:
+
      B\bBk\bk-\b-b\bbr\bre\bea\bak\bk _\bx _\bx _\bx _\bx _\bx _\bx _\bx _\bx _\bx _\bx _\bx _\bx _\bx _\bx _\bx _\bx _\bx _\bx _\bx _\bx _\bx _\bx _\bx _\bx _\bx _\bx _\bx _\bx _\bx _\bx
      [o _\ba]
 
--
 To unsubscribe send an email to tech+unsubscribe@mdocml.bsd.lv

Re: Bugs and regressions in .Bk

[Kristaps Dzonsons]

On 06/02/2011 21:39, Ingo Schwarze wrote:
> Hi Kristaps,
>
>> Log Message:
>> -----------
>> Arguments to `Bsx' and friends are separated by a non-breaking space.
>> This removes a TODO raised by schwarze@.
>
> While trying to merge this patch, i ran into a regression:
> When called inside .Bk or in the SYNOPSIS, this patch ends
> keep mode.
>
> Trying to fix that (meanwhile, i have a working fix, so don't
> waste your time searching for it), i ran into another .Bk bug:
> For text generated in the pre-handler, setting up keep state
> comes too late to take effect.
>
> So, my plan is to
>
>   1) Get the following patch in, fixing the latter bug - OK?
>      The code is unchanged, it is just moved around.
>
>   2) Once that's in, merge your patch to OpenBSD, together
>      with my fix for the regression.
>
>   3) Finally, merge back my regression fix to bsd.lv.

Ingo,

That's fine by me.  I haven't closely examined the behaviour, but it 
looks reasonable.  Please add in the necessary comments in the code as 
to why the order is important, though...

Thanks!

Kristaps

--
 To unsubscribe send an email to tech+unsubscribe@mdocml.bsd.lv

Re: Bugs and regressions in .Bk

[Ingo Schwarze]

Hi Kristaps,

Kristaps Dzonsons wrote on Sun, Feb 06, 2011 at 10:46:04PM +0100:
> On 06/02/2011 21:39, Ingo Schwarze wrote:

>> So, my plan is to
>>
>>  1) Get the following patch in, fixing the latter bug - OK?
>>     The code is unchanged, it is just moved around.
>>
>>  2) Once that's in, merge your patch to OpenBSD, together
>>     with my fix for the regression.
>>
>>  3) Finally, merge back my regression fix to bsd.lv.

> That's fine by me.  I haven't closely examined the behaviour, but it
> looks reasonable.

Thanks for having a look!
All three steps are now done in both repositories.

> Please add in the necessary comments in the code
> as to why the order is important, though...

That comment is in place, too.

Yours,
  Ingo
--
 To unsubscribe send an email to tech+unsubscribe@mdocml.bsd.lv