Lint feature: macro arguments to No

Lint feature: macro arguments to No

[Anthony J. Bentley]

Hi,

When working on a manual today, I accidentally wrote:

    .Ar foo No Ns bar

instead of

    .Ar foo Ns No bar

mandoc -Tlint didn't warn me at all, but groff did:

    mdoc warning: Using a macro as first argument cancels effect of .No

This seems like an easy-to-miss mistake that would be useful to catch
with mandoc -Tlint.

-- 
Anthony J. Bentley
--
 To unsubscribe send an email to discuss+unsubscribe@mdocml.bsd.lv

Re: Lint feature: macro arguments to No

[Ingo Schwarze]

Hi Anthony,

Anthony J. Bentley wrote on Sun, Nov 16, 2014 at 04:13:02PM -0700:

> When working on a manual today, I accidentally wrote:
> 
>     .Ar foo No Ns bar
> 
> instead of
> 
>     .Ar foo Ns No bar
> 
> mandoc -Tlint didn't warn me at all, but groff did:
> 
>     mdoc warning: Using a macro as first argument cancels effect of .No
> 
> This seems like an easy-to-miss mistake that would be useful to catch
> with mandoc -Tlint.

Granted.

While investigating, i ran into a handful of bugs in the area,
and after fixing all of them, it turned out that the problem
with the missing warning went away without further changes.
It now warns like this:

  WARNING: skipping empty macro: No

Can you confirm that it works for you, too?

Thanks for the suggestion,
  Ingo


Log Message:
-----------
Multiple fixes with respect to in-line macros:
* .No selects the default font; relevant e.g. in .Bf blocks
* no need to force empty .Li elements
* closing delimiters as leading macro arguments do not suppress space
* opening delimiters at the end of a macro line do not suppress space
* correctly handle delimiter spacing in -Tman
As a side effect, these fixes let mandoc warn about empty .No macros
as requested by bentley@.

Modified Files:
--------------
    mdocml:
        libmdoc.h
        mdoc_html.c
        mdoc_macro.c
        mdoc_man.c
        mdoc_term.c
        mdoc_validate.c

Revision Data
-------------
Index: mdoc_man.c
===================================================================
RCS file: /usr/vhosts/mdocml.bsd.lv/cvs/mdocml/mdoc_man.c,v
retrieving revision 1.72
retrieving revision 1.73
diff -Lmdoc_man.c -Lmdoc_man.c -u -p -r1.72 -r1.73
--- mdoc_man.c
+++ mdoc_man.c
@@ -594,7 +594,11 @@ print_node(DECL_ARGS)
 			printf("\\&");
 			outflags &= ~MMAN_spc;
 		}
+		if (outflags & MMAN_Sm && ! (n->flags & MDOC_DELIMC))
+			outflags |= MMAN_spc_force;
 		print_word(n->string);
+		if (outflags & MMAN_Sm && ! (n->flags & MDOC_DELIMO))
+			outflags |= MMAN_spc;
 	} else {
 		/*
 		 * Conditionally run the pre-node action handler for a
Index: mdoc_validate.c
===================================================================
RCS file: /usr/vhosts/mdocml.bsd.lv/cvs/mdocml/mdoc_validate.c,v
retrieving revision 1.254
retrieving revision 1.255
diff -Lmdoc_validate.c -Lmdoc_validate.c -u -p -r1.254 -r1.255
--- mdoc_validate.c
+++ mdoc_validate.c
@@ -209,7 +209,7 @@ static	const struct valids mdoc_valids[M
 	{ NULL, NULL },				/* Eo */
 	{ NULL, NULL },				/* Fx */
 	{ NULL, NULL },				/* Ms */
-	{ NULL, ewarn_eq0 },			/* No */
+	{ NULL, NULL },				/* No */
 	{ NULL, post_ns },			/* Ns */
 	{ NULL, NULL },				/* Nx */
 	{ NULL, NULL },				/* Ox */
@@ -353,6 +353,20 @@ mdoc_valid_post(struct mdoc *mdoc)
 	case MDOC_ROOT:
 		return(post_root(mdoc));
 	default:
+
+		/*
+		 * Closing delimiters are not special at the
+		 * beginning of a block, opening delimiters
+		 * are not special at the end.
+		 */
+
+		if (n->child != NULL)
+			n->child->flags &= ~MDOC_DELIMC;
+		if (n->last != NULL)
+			n->last->flags &= ~MDOC_DELIMO;
+
+		/* Call the macro's postprocessor. */
+
 		p = mdoc_valids[n->tok].post;
 		return(*p ? (*p)(mdoc) : 1);
 	}
@@ -1160,10 +1174,6 @@ post_defaults(POST_ARGS)
 		if ( ! mdoc_word_alloc(mdoc, nn->line, nn->pos, "file"))
 			return(0);
 		if ( ! mdoc_word_alloc(mdoc, nn->line, nn->pos, "..."))
-			return(0);
-		break;
-	case MDOC_Li:
-		if ( ! mdoc_word_alloc(mdoc, nn->line, nn->pos, ""))
 			return(0);
 		break;
 	case MDOC_Pa:
Index: mdoc_macro.c
===================================================================
RCS file: /usr/vhosts/mdocml.bsd.lv/cvs/mdocml/mdoc_macro.c,v
retrieving revision 1.143
retrieving revision 1.144
diff -Lmdoc_macro.c -Lmdoc_macro.c -u -p -r1.143 -r1.144
--- mdoc_macro.c
+++ mdoc_macro.c
@@ -147,8 +147,7 @@ const	struct mdoc_macro __mdoc_macros[MD
 	{ blk_part_exp, MDOC_CALLABLE | MDOC_PARSED | MDOC_EXPLICIT }, /* Eo */
 	{ in_line_argn, MDOC_CALLABLE | MDOC_PARSED }, /* Fx */
 	{ in_line, MDOC_CALLABLE | MDOC_PARSED }, /* Ms */
-	{ in_line_argn, MDOC_CALLABLE | MDOC_PARSED |
-			MDOC_IGNDELIM | MDOC_JOIN }, /* No */
+	{ in_line, MDOC_CALLABLE | MDOC_PARSED | MDOC_JOIN }, /* No */
 	{ in_line_argn, MDOC_CALLABLE | MDOC_PARSED |
 			MDOC_IGNDELIM | MDOC_JOIN }, /* Ns */
 	{ in_line_argn, MDOC_CALLABLE | MDOC_PARSED }, /* Nx */
@@ -627,24 +626,22 @@ dword(struct mdoc *mdoc, int line, int c
 	if ( ! mdoc_word_alloc(mdoc, line, col, p))
 		return(0);
 
-	if (DELIM_OPEN == d)
-		mdoc->last->flags |= MDOC_DELIMO;
-
 	/*
-	 * Closing delimiters only suppress the preceding space
-	 * when they follow something, not when they start a new
-	 * block or element, and not when they follow `No'.
-	 *
-	 * XXX	Explicitly special-casing MDOC_No here feels
-	 *	like a layering violation.  Find a better way
-	 *	and solve this in the code related to `No'!
+	 * If the word consists of a bare delimiter,
+	 * flag the new node accordingly,
+	 * unless doing so was vetoed by the invoking macro.
+	 * Always clear the veto, it is only valid for one word.
 	 */
 
-	else if (DELIM_CLOSE == d && mdoc->last->prev &&
-	    mdoc->last->prev->tok != MDOC_No &&
+	if (d == DELIM_OPEN)
+		mdoc->last->flags |= MDOC_DELIMO;
+	else if (d == DELIM_CLOSE &&
+	    ! (mdoc->flags & MDOC_NODELIMC) &&
 	    mdoc->last->parent->tok != MDOC_Fd)
 		mdoc->last->flags |= MDOC_DELIMC;
 
+	mdoc->flags &= ~MDOC_NODELIMC;
+
 	return(1);
 }
 
@@ -843,7 +840,7 @@ blk_exp_close(MACRO_PROT_ARGS)
 static int
 in_line(MACRO_PROT_ARGS)
 {
-	int		 la, scope, cnt, mayopen, nc, nl;
+	int		 la, scope, cnt, firstarg, mayopen, nc, nl;
 	enum margverr	 av;
 	enum mdoct	 ntok;
 	enum margserr	 ac;
@@ -894,17 +891,40 @@ in_line(MACRO_PROT_ARGS)
 		return(0);
 	}
 
+	d = DELIM_NONE;
+	firstarg = 1;
 	mayopen = 1;
 	for (cnt = scope = 0;; ) {
 		la = *pos;
 		ac = mdoc_args(mdoc, line, pos, buf, tok, &p);
 
-		if (ARGS_ERROR == ac)
+		if (ac == ARGS_ERROR)
 			return(0);
-		if (ARGS_EOLN == ac)
+
+		/*
+		 * At the end of a macro line,
+		 * opening delimiters do not suppress spacing.
+		 */
+
+		if (ac == ARGS_EOLN) {
+			if (d == DELIM_OPEN)
+				mdoc->last->flags &= ~MDOC_DELIMO;
 			break;
-		if (ARGS_PUNCT == ac)
+		}
+
+		/*
+		 * The rest of the macro line is only punctuation,
+		 * to be handled by append_delims().
+		 * If there were no other arguments,
+		 * do not allow the first one to suppress spacing,
+		 * even if it turns out to be a closing one.
+		 */
+
+		if (ac == ARGS_PUNCT) {
+			if (cnt == 0 && nc == 0)
+				mdoc->flags |= MDOC_NODELIMC;
 			break;
+		}
 
 		ntok = ARGS_QWORD == ac ? MDOC_MAX : lookup(tok, p);
 
@@ -949,20 +969,19 @@ in_line(MACRO_PROT_ARGS)
 		if (DELIM_NONE != d) {
 			/*
 			 * If we encounter closing punctuation, no word
-			 * has been omitted, no scope is open, and we're
+			 * has been emitted, no scope is open, and we're
 			 * allowed to have an empty element, then start
 			 * a new scope.
 			 */
 			if ((d == DELIM_CLOSE ||
 			     (d == DELIM_MIDDLE && tok == MDOC_Fl)) &&
-			    (nc || tok == MDOC_Li) &&
-			    !scope && !cnt && mayopen) {
+			    !cnt && !scope && nc && mayopen) {
 				if ( ! mdoc_elem_alloc(mdoc,
 				    line, ppos, tok, arg))
 					return(0);
 				scope = 1;
 				cnt++;
-				if (MDOC_Li == tok || MDOC_Nm == tok)
+				if (MDOC_Nm == tok)
 					mayopen = 0;
 			}
 			/*
@@ -984,6 +1003,15 @@ in_line(MACRO_PROT_ARGS)
 			return(0);
 
 		/*
+		 * If the first argument is a closing delimiter,
+		 * do not suppress spacing before it.
+		 */
+
+		if (firstarg && d == DELIM_CLOSE && !nc)
+			mdoc->last->flags &= ~MDOC_DELIMC;
+		firstarg = 0;
+
+		/*
 		 * `Fl' macros have their scope re-opened with each new
 		 * word so that the `-' can be added to each one without
 		 * having to parse out spaces.
@@ -1532,8 +1560,6 @@ in_line_argn(MACRO_PROT_ARGS)
 
 	switch (tok) {
 	case MDOC_Ap:
-		/* FALLTHROUGH */
-	case MDOC_No:
 		/* FALLTHROUGH */
 	case MDOC_Ns:
 		/* FALLTHROUGH */
Index: mdoc_html.c
===================================================================
RCS file: /usr/vhosts/mdocml.bsd.lv/cvs/mdocml/mdoc_html.c,v
retrieving revision 1.209
retrieving revision 1.210
diff -Lmdoc_html.c -Lmdoc_html.c -u -p -r1.209 -r1.210
--- mdoc_html.c
+++ mdoc_html.c
@@ -98,6 +98,7 @@ static	int		  mdoc_mt_pre(MDOC_ARGS);
 static	int		  mdoc_ms_pre(MDOC_ARGS);
 static	int		  mdoc_nd_pre(MDOC_ARGS);
 static	int		  mdoc_nm_pre(MDOC_ARGS);
+static	int		  mdoc_no_pre(MDOC_ARGS);
 static	int		  mdoc_ns_pre(MDOC_ARGS);
 static	int		  mdoc_pa_pre(MDOC_ARGS);
 static	void		  mdoc_pf_post(MDOC_ARGS);
@@ -192,7 +193,7 @@ static	const struct htmlmdoc mdocs[MDOC_
 	{mdoc_quote_pre, mdoc_quote_post}, /* Eo */
 	{mdoc_xx_pre, NULL}, /* Fx */
 	{mdoc_ms_pre, NULL}, /* Ms */
-	{mdoc_igndelim_pre, NULL}, /* No */
+	{mdoc_no_pre, NULL}, /* No */
 	{mdoc_ns_pre, NULL}, /* Ns */
 	{mdoc_xx_pre, NULL}, /* Nx */
 	{mdoc_xx_pre, NULL}, /* Ox */
@@ -1880,6 +1881,16 @@ mdoc_rs_pre(MDOC_ARGS)
 
 	PAIR_CLASS_INIT(&tag, "ref");
 	print_otag(h, TAG_SPAN, 1, &tag);
+	return(1);
+}
+
+static int
+mdoc_no_pre(MDOC_ARGS)
+{
+	struct htmlpair	tag;
+
+	PAIR_CLASS_INIT(&tag, "none");
+	print_otag(h, TAG_CODE, 1, &tag);
 	return(1);
 }
 
Index: libmdoc.h
===================================================================
RCS file: /usr/vhosts/mdocml.bsd.lv/cvs/mdocml/libmdoc.h,v
retrieving revision 1.88
retrieving revision 1.89
diff -Llibmdoc.h -Llibmdoc.h -u -p -r1.88 -r1.89
--- libmdoc.h
+++ libmdoc.h
@@ -1,7 +1,7 @@
 /*	$Id$ */
 /*
  * Copyright (c) 2008, 2009, 2010, 2011 Kristaps Dzonsons <kristaps@bsd.lv>
- * Copyright (c) 2013 Ingo Schwarze <schwarze@openbsd.org>
+ * Copyright (c) 2013, 2014 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
@@ -37,6 +37,7 @@ struct	mdoc {
 #define	MDOC_SYNOPSIS	 (1 << 7) /* SYNOPSIS-style formatting */
 #define	MDOC_KEEP	 (1 << 8) /* in a word keep */
 #define	MDOC_SMOFF	 (1 << 9) /* spacing is off */
+#define	MDOC_NODELIMC	 (1 << 10) /* disable closing delimiter handling */
 	enum mdoc_next	  next; /* where to put the next node */
 	struct mdoc_node *last; /* the last node parsed */
 	struct mdoc_node *first; /* the first node parsed */
Index: mdoc_term.c
===================================================================
RCS file: /usr/vhosts/mdocml.bsd.lv/cvs/mdocml/mdoc_term.c,v
retrieving revision 1.289
retrieving revision 1.290
diff -Lmdoc_term.c -Lmdoc_term.c -u -p -r1.289 -r1.290
--- mdoc_term.c
+++ mdoc_term.c
@@ -193,7 +193,7 @@ static	const struct termact termacts[MDO
 	{ termp_quote_pre, termp_quote_post }, /* Eo */
 	{ termp_xx_pre, NULL }, /* Fx */
 	{ termp_bold_pre, NULL }, /* Ms */
-	{ NULL, NULL }, /* No */
+	{ termp_li_pre, NULL }, /* No */
 	{ termp_ns_pre, NULL }, /* Ns */
 	{ termp_xx_pre, NULL }, /* Nx */
 	{ termp_xx_pre, NULL }, /* Ox */
--
 To unsubscribe send an email to discuss+unsubscribe@mdocml.bsd.lv

Re: Lint feature: macro arguments to No

[Anthony J. Bentley]

Hi Ingo,

Ingo Schwarze writes:
> While investigating, i ran into a handful of bugs in the area,
> and after fixing all of them, it turned out that the problem
> with the missing warning went away without further changes.
> It now warns like this:
> 
>   WARNING: skipping empty macro: No
> 
> Can you confirm that it works for you, too?

Yes, the latest revision of mandoc from OpenBSD works correctly. Thanks!

-- 
Anthony J. Bentley
--
 To unsubscribe send an email to discuss+unsubscribe@mdocml.bsd.lv