fix blank line handling in .if

fix blank line handling in .if

[Ingo Schwarze]

OK to commit to bsd.lv as well?

----- Forwarded message from Ingo Schwarze <schwarze@cvs.openbsd.org> -----

From: Ingo Schwarze <schwarze@cvs.openbsd.org>
Date: Wed, 30 May 2012 19:36:56 -0600 (MDT)
To: source-changes@cvs.openbsd.org

CVSROOT:	/cvs
Module name:	src
Changes by:	schwarze@cvs.openbsd.org	2012/05/30 19:36:56

Modified files:
	usr.bin/mandoc : roff.c 
	regress/usr.bin/mandoc/roff/cond: if.in if.out_ascii 

Log message:
Fix blank line handling in .if.

In particular, two cases were wrong:
- single-line .if with trailing whitespace gave no blank line
- multiline .if with \{ but without \{\ gave no blank line

While here, simplify roff_cond() by partially reordering the code.

----- End forwarded message -----

Index: usr.bin/mandoc/roff.c
===================================================================
RCS file: /cvs/src/usr.bin/mandoc/roff.c,v
retrieving revision 1.45
diff -u -p -r1.45 roff.c
--- usr.bin/mandoc/roff.c	24 Oct 2011 21:38:56 -0000	1.45
+++ usr.bin/mandoc/roff.c	31 May 2012 01:30:16 -0000
@@ -1,7 +1,7 @@
 /*	$Id: roff.c,v 1.45 2011/10/24 21:38:56 schwarze Exp $ */
 /*
  * Copyright (c) 2010, 2011 Kristaps Dzonsons <kristaps@bsd.lv>
- * Copyright (c) 2010, 2011 Ingo Schwarze <schwarze@openbsd.org>
+ * Copyright (c) 2010, 2011, 2012 Ingo Schwarze <schwarze@openbsd.org>
  *
  * Permission to use, copy, modify, and distribute this software for any
  * purpose with or without fee is hereby granted, provided that the above
@@ -774,7 +774,7 @@ roffnode_cleanscope(struct roff *r)
 {
 
 	while (r->last) {
-		if (--r->last->endspan < 0)
+		if (--r->last->endspan != 0)
 			break;
 		roffnode_pop(r);
 	}
@@ -1094,8 +1094,8 @@ roff_line_ignore(ROFF_ARGS)
 static enum rofferr
 roff_cond(ROFF_ARGS)
 {
-	int		 sv;
-	enum roffrule	 rule;
+
+	roffnode_push(r, tok, NULL, ln, ppos);
 
 	/* 
 	 * An `.el' has no conditional body: it will consume the value
@@ -1105,31 +1105,11 @@ roff_cond(ROFF_ARGS)
 	 * If we're not an `el', however, then evaluate the conditional.
 	 */
 
-	rule = ROFF_el == tok ?
+	r->last->rule = ROFF_el == tok ?
 		(r->rstackpos < 0 ? 
 		 ROFFRULE_DENY : r->rstack[r->rstackpos--]) :
 		roff_evalcond(*bufp, &pos);
 
-	sv = pos;
-	while (' ' == (*bufp)[pos])
-		pos++;
-
-	/*
-	 * Roff is weird.  If we have just white-space after the
-	 * conditional, it's considered the BODY and we exit without
-	 * really doing anything.  Warn about this.  It's probably
-	 * wrong.
-	 */
-
-	if ('\0' == (*bufp)[pos] && sv != pos) {
-		mandoc_msg(MANDOCERR_NOARGS, r->parse, ln, ppos, NULL);
-		return(ROFF_IGN);
-	}
-
-	roffnode_push(r, tok, NULL, ln, ppos);
-
-	r->last->rule = rule;
-
 	/*
 	 * An if-else will put the NEGATION of the current evaluated
 	 * conditional into the stack of rules.
@@ -1152,28 +1132,39 @@ roff_cond(ROFF_ARGS)
 		r->last->rule = ROFFRULE_DENY;
 
 	/*
-	 * Determine scope.  If we're invoked with "\{" trailing the
-	 * conditional, then we're in a multiline scope.  Else our scope
-	 * expires on the next line.
+	 * Determine scope.
+	 * If there is nothing on the line after the conditional,
+	 * not even whitespace, use next-line scope.
 	 */
 
-	r->last->endspan = 1;
+	if ('\0' == (*bufp)[pos]) {
+		r->last->endspan = 2;
+		goto out;
+	}
+
+	while (' ' == (*bufp)[pos])
+		pos++;
+
+	/* An opening brace requests multiline scope. */
 
 	if ('\\' == (*bufp)[pos] && '{' == (*bufp)[pos + 1]) {
 		r->last->endspan = -1;
 		pos += 2;
+		goto out;
 	} 
 
 	/*
-	 * If there are no arguments on the line, the next-line scope is
-	 * assumed.
+	 * Anything else following the conditional causes
+	 * single-line scope.  Warn if the scope contains
+	 * nothing but trailing whitespace.
 	 */
 
 	if ('\0' == (*bufp)[pos])
-		return(ROFF_IGN);
+		mandoc_msg(MANDOCERR_NOARGS, r->parse, ln, ppos, NULL);
 
-	/* Otherwise re-run the roff parser after recalculating. */
+	r->last->endspan = 1;
 
+out:
 	*offs = pos;
 	return(ROFF_RERUN);
 }
Index: regress/usr.bin/mandoc/roff/cond/if.in
===================================================================
RCS file: /cvs/src/regress/usr.bin/mandoc/roff/cond/if.in,v
retrieving revision 1.2
diff -u -p -r1.2 if.in
--- regress/usr.bin/mandoc/roff/cond/if.in	17 Nov 2011 16:28:46 -0000	1.2
+++ regress/usr.bin/mandoc/roff/cond/if.in	31 May 2012 01:30:16 -0000
@@ -1,9 +1,20 @@
-.TH IF 1 "April 25, 2010" OpenBSD
+.TH IF 1 "May 30, 2012" OpenBSD
 .SH NAME
 if \- the roff conditional instruction
 .SH DESCRIPTION
 .if n One-line true condition.
 .if t One-line false condition.
+.if n
+Next-line empty true condition.
+.if t
+Next-line empty false condition.
+.br
+False condition with whitespace:
+.if t 
+True condition with whitespace:
+.if n 
+.if n \{One-line true condition with block.\}
+.if t \{One-line false condition with block.\}
 .br
 .if n \{Two-line true condition
 without continuation and macros.\}
@@ -18,6 +29,11 @@ with continuation and macros.
 Two-line false condition
 with continuation and macros.
 .\}
+.br
+.if n \{
+Two-line true condition with blank line.\}
+.if t \{
+Two-line false condition with blank line.\}
 .br
 .if n \{Outer true condition.
 .br
Index: regress/usr.bin/mandoc/roff/cond/if.out_ascii
===================================================================
RCS file: /cvs/src/regress/usr.bin/mandoc/roff/cond/if.out_ascii,v
retrieving revision 1.4
diff -u -p -r1.4 if.out_ascii
--- regress/usr.bin/mandoc/roff/cond/if.out_ascii	2 Dec 2011 01:50:46 -0000	1.4
+++ regress/usr.bin/mandoc/roff/cond/if.out_ascii	31 May 2012 01:30:16 -0000
@@ -7,8 +7,15 @@ N\bNA\bAM\bME\bE
 
 D\bDE\bES\bSC\bCR\bRI\bIP\bPT\bTI\bIO\bON\bN
        One-line true condition.
+
+       Next-line empty true condition.
+       False condition with whitespace: True condition with whitespace:
+
+       One-line true condition with block.
        Two-line true condition without continuation and macros.
        Two-line true condition with continuation and macros.
+
+       Two-line true condition with blank line.
        Outer true condition.
        Inner one-line true condition.
        Inner two-line true condition without continuation and macros.
@@ -16,4 +23,4 @@ D\bDE\bES\bSC\bCR\bRI\bIP\bPT\bTI\bIO\bON\bN
 
 
 
-OpenBSD                         April 25, 2010                           IF(1)
+OpenBSD                          May 30, 2012                            IF(1)

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

Re: fix blank line handling in .if

[Kristaps Dzonsons]

On 05/31/12 03:42, Ingo Schwarze wrote:
> OK to commit to bsd.lv as well?

Good one!  As usual, do just commit and I'll look over the patches in 
hindsight and see if anything makes me sad.  I'm particularly fond of 
this fix---the trailing space/newline has always been annoying.

I anticipate finally (finally!) having some free time by this weekend 
and next week (projects rolling over), so I'll polish the SQLite fixes 
(this will initially come with a hit to apropos functionality--only OR 
searches, for example, but is worthwhile given the massive reduction of 
code complexity).

Given Jesse's mention of many roff macros he'd like to implement---few 
of them possible without layer violations---I'd like to throw together a 
concept, as we've discussed before (with fear and awe in our voices), of 
all macros thrown together and parsed in a common way.
--
 To unsubscribe send an email to tech+unsubscribe@mdocml.bsd.lv

Re: fix blank line handling in .if

[Ingo Schwarze]

Hi Kristaps,

Kristaps Dzonsons wrote on Thu, May 31, 2012 at 11:49:56AM +0200:

> Good one!  As usual, do just commit and I'll look over the patches
> in hindsight and see if anything makes me sad.

Thanks, will do so tonight.

> I'm particularly fond of this fix---the trailing space/newline has
> always been annoying.
> 
> I anticipate finally (finally!) having some free time by this
> weekend and next week (projects rolling over), so I'll polish the
> SQLite fixes (this will initially come with a hit to apropos
> functionality--only OR searches, for example, but is worthwhile
> given the massive reduction of code complexity).

OK, so i will try to get that in and tested, too!
(Or at least to nag... ;-)

One thing i have to do is send a revert patch for the WARNING macro.
A variable number of arguments for a macro is not portable.

> Given Jesse's mention of many roff macros he'd like to
> implement---few of them possible without layer violations---I'd like
> to throw together a concept, as we've discussed before (with fear
> and awe in our voices), of all macros thrown together and parsed in
> a common way.

I'm not sure it would be fair with respect to Jesse to do that right
now; well, he can't get clean earth to till, but at least he should
have a stable platform.  I don't think you will get that project to
a stable state until his time is up, so it won't help him.
From your perspective, it's probably less work, too, to start that
after he has moved on to diff(1) - less conflicts, in case he manages
to produce any patches good enough for commit...

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

Re: fix blank line handling in .if

[Kristaps Dzonsons]

On 05/31/12 12:28, Ingo Schwarze wrote:
> Hi Kristaps,
>
> Kristaps Dzonsons wrote on Thu, May 31, 2012 at 11:49:56AM +0200:
>
>> Good one!  As usual, do just commit and I'll look over the patches
>> in hindsight and see if anything makes me sad.
>
> Thanks, will do so tonight.
>
>> I'm particularly fond of this fix---the trailing space/newline has
>> always been annoying.
>>
>> I anticipate finally (finally!) having some free time by this
>> weekend and next week (projects rolling over), so I'll polish the
>> SQLite fixes (this will initially come with a hit to apropos
>> functionality--only OR searches, for example, but is worthwhile
>> given the massive reduction of code complexity).
>
> OK, so i will try to get that in and tested, too!
> (Or at least to nag... ;-)
>
> One thing i have to do is send a revert patch for the WARNING macro.
> A variable number of arguments for a macro is not portable.
>
>> Given Jesse's mention of many roff macros he'd like to
>> implement---few of them possible without layer violations---I'd like
>> to throw together a concept, as we've discussed before (with fear
>> and awe in our voices), of all macros thrown together and parsed in
>> a common way.
>
> I'm not sure it would be fair with respect to Jesse to do that right
> now; well, he can't get clean earth to till, but at least he should
> have a stable platform.  I don't think you will get that project to
> a stable state until his time is up, so it won't help him.
>  From your perspective, it's probably less work, too, to start that
> after he has moved on to diff(1) - less conflicts, in case he manages
> to produce any patches good enough for commit...

Ingo,

Oh, don't get me wrong---I fully agree.  As mentioned in the last mail, 
a greater priority for me is to clean out the dratted tab-handling code 
in -mdoc column lists.  With this cleaned up, and argument parsing 
unified, there'd be a much clearer basis from which to consider a Grand 
Unification of Macros.

I also want to generalise the [x]html frontend to use macro names as 
classes, which would remove a lot of superfluous code.  But that's an 
entirely different story.

Of course, after such a long caesura, it may take a few hours simply to 
remember what's where in the code-base!  My local tree is a mess of 
sqlite benchmarks and tests.

Best,

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