tech@mandoc.bsd.lv
 help / color / mirror / Atom feed
* Re: mdocml: Specifying both %T and %J in an `Rs' block causes the title to
       [not found] <201012251350.oBPDob7Q026196@krisdoz.my.domain>
@ 2010-12-25 17:40 ` Ingo Schwarze
  2010-12-25 23:26   ` Kristaps Dzonsons
  0 siblings, 1 reply; 2+ messages in thread
From: Ingo Schwarze @ 2010-12-25 17:40 UTC (permalink / raw)
  To: tech

Hi Kristaps,

kristaps@mdocml.bsd.lv wrote on Sat, Dec 25, 2010 at 08:50:37AM -0500:

> Log Message:
> -----------
> Specifying both %T and %J in an `Rs' block causes the title to be quoted
> instead of underlined.  This only happens in -Tascii, as -T[x]html both
> underlines and italicises.

Thanks for fixing this!
Your approach mostly works and helped a lot to resolve this issue
in OpenBSD; however, it can be simplified, see below for details.


A patch against OpenBSD is appended at the end.

However, we are now so badly out of sync that we can't push simple
fixes back and forth any longer, it won't apply at all to bsd.lv.
In a nutshell, all the %T code needs to know whether there is a %J
node as well, so there is no need to do any calculations.
It is sufficent (and more flexible) to just save a pointer.

See inline for more details.

In case you agree with my changes, we should probably delay the
merge until we are back in sync, or just merge this as part of the
re-sync.

> Modified Files:
> --------------
>     mdocml:
>         TODO
>         mdoc.c
>         mdoc.h
>         mdoc_term.c
>         mdoc_validate.c
> 
> Revision Data
> -------------
> Index: mdoc_validate.c
> ===================================================================
> RCS file: /usr/vhosts/mdocml.bsd.lv/cvs/mdocml/mdoc_validate.c,v
> retrieving revision 1.147
> retrieving revision 1.148
> diff -Lmdoc_validate.c -Lmdoc_validate.c -u -p -r1.147 -r1.148
> --- mdoc_validate.c
> +++ mdoc_validate.c
> @@ -1649,8 +1649,19 @@ post_rs(POST_ARGS)
>  {
>  	struct mdoc_node *nn, *next, *prev;
>  	int		  i, j;
> +	int		 *tj;
> +#define	RS_JOURNAL	 (1 << 0)
> +#define	RS_TITLE	 (1 << 1)
>  
> -	if (MDOC_BODY != mdoc->last->type)
> +	/* Mark whether we're carrying both a %T and %J. */
> +
> +	tj = &mdoc->last->norm->Rs.titlejournal;
> +
> +	if (MDOC_BLOCK == mdoc->last->type) {
> +		if ( ! (RS_JOURNAL & *tj && RS_TITLE & *tj))
> +			*tj = 0;
> +		return(1);
> +	} else if (MDOC_BODY != mdoc->last->type)
>  		return(1);
>  
>  	/*
> @@ -1666,6 +1677,10 @@ post_rs(POST_ARGS)
>  				break;
>  
>  		if (i < RSORD_MAX) {
> +			if (MDOC__T == rsord[i])
> +				*tj |= RS_TITLE;
> +			else if (MDOC__J == rsord[i])
> +				*tj |= RS_JOURNAL;
>  			next = nn->next;
>  			continue;
>  		}

All this can be replaced by a one-line if
and a one-line pointer assignment.

Besides, this part of the patch contains "norm",
so it does not apply to OpenBSD.

> Index: mdoc_term.c
> ===================================================================
> RCS file: /usr/vhosts/mdocml.bsd.lv/cvs/mdocml/mdoc_term.c,v
> retrieving revision 1.202
> retrieving revision 1.203
> diff -Lmdoc_term.c -Lmdoc_term.c -u -p -r1.202 -r1.203
> --- mdoc_term.c
> +++ mdoc_term.c
> @@ -68,6 +68,7 @@ static	void	  synopsis_pre(struct termp 
>  			const struct mdoc_node *);
>  
>  static	void	  termp____post(DECL_ARGS);
> +static	void	  termp__t_post(DECL_ARGS);
>  static	void	  termp_an_post(DECL_ARGS);
>  static	void	  termp_bd_post(DECL_ARGS);
>  static	void	  termp_bk_post(DECL_ARGS);
> @@ -85,6 +86,7 @@ static	void	  termp_sh_post(DECL_ARGS);
>  static	void	  termp_ss_post(DECL_ARGS);
>  
>  static	int	  termp__a_pre(DECL_ARGS);
> +static	int	  termp__t_pre(DECL_ARGS);
>  static	int	  termp_an_pre(DECL_ARGS);
>  static	int	  termp_ap_pre(DECL_ARGS);
>  static	int	  termp_bd_pre(DECL_ARGS);
> @@ -174,7 +176,7 @@ static	const struct termact termacts[MDO
>  	{ NULL, termp____post }, /* %O */
>  	{ NULL, termp____post }, /* %P */
>  	{ NULL, termp____post }, /* %R */
> -	{ termp_under_pre, termp____post }, /* %T */
> +	{ termp__t_pre, termp__t_post }, /* %T */
>  	{ NULL, termp____post }, /* %V */
>  	{ NULL, NULL }, /* Ac */
>  	{ termp_quote_pre, termp_quote_post }, /* Ao */
> @@ -1830,7 +1832,7 @@ static int
>  termp_quote_pre(DECL_ARGS)
>  {
>  
> -	if (MDOC_BODY != n->type)
> +	if (MDOC_BODY != n->type && MDOC_ELEM != n->type)
>  		return(1);
>  
>  	switch (n->tok) {
> @@ -1853,6 +1855,8 @@ termp_quote_pre(DECL_ARGS)
>  	case (MDOC_Bq):
>  		term_word(p, "[");
>  		break;
> +	case (MDOC__T):
> +		/* FALLTHROUGH */

Wrong position; both old and new groff handle this like .Qo,
not like .Do.

>  	case (MDOC_Do):
>  		/* FALLTHROUGH */
>  	case (MDOC_Dq):
> @@ -1890,7 +1894,7 @@ static void
>  termp_quote_post(DECL_ARGS)
>  {
>  
> -	if (MDOC_BODY != n->type)
> +	if (MDOC_BODY != n->type && MDOC_ELEM != n->type)
>  		return;
>  
>  	p->flags |= TERMP_NOSPACE;
> @@ -1915,6 +1919,8 @@ termp_quote_post(DECL_ARGS)
>  	case (MDOC_Bq):
>  		term_word(p, "]");
>  		break;
> +	case (MDOC__T):
> +		/* FALLTHROUGH */

Again, we want .Qo.

>  	case (MDOC_Do):
>  		/* FALLTHROUGH */
>  	case (MDOC_Dq):
> @@ -2131,6 +2137,39 @@ termp_bk_post(DECL_ARGS)
>  
>  	if (MDOC_BODY == n->type)
>  		p->flags &= ~(TERMP_KEEP | TERMP_PREKEEP);
> +}
> +
> +/* ARGSUSED */
> +static void
> +termp__t_post(DECL_ARGS)
> +{
> +
> +	/*
> +	 * If we're in an `Rs' and there's a journal present, then quote
> +	 * us instead of underlining us (for disambiguation).
> +	 */
> +	if (n->parent && MDOC_Rs == n->parent->tok &&
> +			n->parent->norm->Rs.titlejournal)
> +		termp_quote_post(p, pair, m, n);
> +
> +	termp____post(p, pair, m, n);
> +}
> +
> +/* ARGSUSED */
> +static int
> +termp__t_pre(DECL_ARGS)
> +{
> +
> +	/*
> +	 * If we're in an `Rs' and there's a journal present, then quote
> +	 * us instead of underlining us (for disambiguation).
> +	 */
> +	if (n->parent && MDOC_Rs == n->parent->tok &&
> +			n->parent->norm->Rs.titlejournal)
> +		return(termp_quote_pre(p, pair, m, n));
> +
> +	term_fontpush(p, TERMFONT_UNDER);
> +	return(1);
>  }

Again, this doesn't apply to OpenBSD ("norm").

>  /* ARGSUSED */
> Index: mdoc.h
> ===================================================================
> RCS file: /usr/vhosts/mdocml.bsd.lv/cvs/mdocml/mdoc.h,v
> retrieving revision 1.110
> retrieving revision 1.111
> diff -Lmdoc.h -Lmdoc.h -u -p -r1.110 -r1.111
> --- mdoc.h
> +++ mdoc.h
> @@ -353,6 +353,10 @@ struct	mdoc_an {
>  	enum mdoc_auth	  auth; /* -split, etc. */
>  };
>  
> +struct	mdoc_rs {
> +	int		  titlejournal; /* whether %T and %J */
> +};
> +

Just saving child_J is more flexible.

>  /*
>   * Consists of normalised node arguments.  These should be used instead
>   * of iterating through the mdoc_arg pointers of a node: defaults are
> @@ -363,6 +367,7 @@ union	mdoc_data {
>  	struct mdoc_bd	  Bd;
>  	struct mdoc_bf	  Bf;
>  	struct mdoc_bl	  Bl;
> +	struct mdoc_rs	  Rs;
>  };
>  
>  /* 
> Index: mdoc.c
> ===================================================================
> RCS file: /usr/vhosts/mdocml.bsd.lv/cvs/mdocml/mdoc.c,v
> retrieving revision 1.172
> retrieving revision 1.173
> diff -Lmdoc.c -Lmdoc.c -u -p -r1.172 -r1.173
> --- mdoc.c
> +++ mdoc.c
> @@ -484,6 +484,8 @@ mdoc_block_alloc(struct mdoc *m, int lin
>  	case (MDOC_Bf):
>  		/* FALLTHROUGH */
>  	case (MDOC_Bl):
> +		/* FALLTHROUGH */
> +	case (MDOC_Rs):

Still looks a bit different in OpenBSD.

>  		p->norm = mandoc_calloc(1, sizeof(union mdoc_data));
>  		break;
>  	default:

So, here is my version of the patch:

OK to commit to OpenBSD?

Yours,
  Ingo


Index: mdoc.c
===================================================================
RCS file: /cvs/src/usr.bin/mandoc/mdoc.c,v
retrieving revision 1.73
diff -u -p -r1.73 mdoc.c
--- mdoc.c	21 Dec 2010 23:57:31 -0000	1.73
+++ mdoc.c	25 Dec 2010 17:23:38 -0000
@@ -541,6 +541,9 @@ mdoc_node_free(struct mdoc_node *p)
 	if (MDOC_An == p->tok)
 		if (p->data.An)
 			free(p->data.An);
+	if (MDOC_Rs == p->tok && MDOC_BLOCK == p->type)
+		if (p->data.Rs)
+			free(p->data.Rs);
 	if (MDOC_TS == p->tok && MDOC_BLOCK == p->type)
 		if (p->data.TS)
 			tbl_free(p->data.TS);
Index: mdoc.h
===================================================================
RCS file: /cvs/src/usr.bin/mandoc/mdoc.h,v
retrieving revision 1.38
diff -u -p -r1.38 mdoc.h
--- mdoc.h	21 Dec 2010 23:57:31 -0000	1.38
+++ mdoc.h	25 Dec 2010 17:23:38 -0000
@@ -355,6 +355,10 @@ struct	mdoc_an {
 	enum mdoc_auth	  auth; /* -split, etc. */
 };
 
+struct	mdoc_rs {
+	struct mdoc_node *child_J;
+};
+
 /*
  * Consists of normalised node arguments.  These should be used instead
  * of iterating through the mdoc_arg pointers of a node: defaults are
@@ -365,6 +369,7 @@ union mdoc_data {
 	struct mdoc_bd	 *Bd;
 	struct mdoc_bf	 *Bf;
 	struct mdoc_bl	 *Bl;
+	struct mdoc_rs	 *Rs;
 	struct tbl	 *TS;
 };
 
Index: mdoc_term.c
===================================================================
RCS file: /cvs/src/usr.bin/mandoc/mdoc_term.c,v
retrieving revision 1.119
diff -u -p -r1.119 mdoc_term.c
--- mdoc_term.c	21 Dec 2010 23:57:31 -0000	1.119
+++ mdoc_term.c	25 Dec 2010 17:23:38 -0000
@@ -65,6 +65,7 @@ static	void	  synopsis_pre(struct termp 
 			const struct mdoc_node *);
 
 static	void	  termp____post(DECL_ARGS);
+static	void	  termp__t_post(DECL_ARGS);
 static	void	  termp_an_post(DECL_ARGS);
 static	void	  termp_bd_post(DECL_ARGS);
 static	void	  termp_bk_post(DECL_ARGS);
@@ -82,6 +83,7 @@ static	void	  termp_sh_post(DECL_ARGS);
 static	void	  termp_ss_post(DECL_ARGS);
 
 static	int	  termp__a_pre(DECL_ARGS);
+static	int	  termp__t_pre(DECL_ARGS);
 static	int	  termp_an_pre(DECL_ARGS);
 static	int	  termp_ap_pre(DECL_ARGS);
 static	int	  termp_bd_pre(DECL_ARGS);
@@ -172,7 +174,7 @@ static	const struct termact termacts[MDO
 	{ NULL, termp____post }, /* %O */
 	{ NULL, termp____post }, /* %P */
 	{ NULL, termp____post }, /* %R */
-	{ termp_under_pre, termp____post }, /* %T */
+	{ termp__t_pre, termp__t_post }, /* %T */
 	{ NULL, termp____post }, /* %V */
 	{ NULL, NULL }, /* Ac */
 	{ termp_quote_pre, termp_quote_post }, /* Ao */
@@ -1833,7 +1835,7 @@ static int
 termp_quote_pre(DECL_ARGS)
 {
 
-	if (MDOC_BODY != n->type)
+	if (MDOC_BODY != n->type && MDOC_ELEM != n->type)
 		return(1);
 
 	switch (n->tok) {
@@ -1866,6 +1868,8 @@ termp_quote_pre(DECL_ARGS)
 	case (MDOC_Pq):
 		term_word(p, "(");
 		break;
+	case (MDOC__T):
+		/* FALLTHROUGH */
 	case (MDOC_Qo):
 		/* FALLTHROUGH */
 	case (MDOC_Qq):
@@ -1893,7 +1897,7 @@ static void
 termp_quote_post(DECL_ARGS)
 {
 
-	if (MDOC_BODY != n->type)
+	if (MDOC_BODY != n->type && MDOC_ELEM != n->type)
 		return;
 
 	p->flags |= TERMP_NOSPACE;
@@ -1928,6 +1932,8 @@ termp_quote_post(DECL_ARGS)
 	case (MDOC_Pq):
 		term_word(p, ")");
 		break;
+	case (MDOC__T):
+		/* FALLTHROUGH */
 	case (MDOC_Qo):
 		/* FALLTHROUGH */
 	case (MDOC_Qq):
@@ -2152,6 +2158,41 @@ termp_bk_post(DECL_ARGS)
 	if (MDOC_BODY == n->type)
 		p->flags &= ~(TERMP_KEEP | TERMP_PREKEEP);
 }
+
+/* ARGSUSED */
+static void
+termp__t_post(DECL_ARGS)
+{
+	struct mdoc_node *nn;
+
+	/*
+	 * If we're in an `Rs' and there's a journal present, then quote
+	 * us instead of underlining us (for disambiguation).
+	 */
+	nn = n->parent->parent;
+	if (nn && MDOC_Rs == nn->tok && nn->data.Rs->child_J)
+		termp_quote_post(p, pair, m, n);
+
+	termp____post(p, pair, m, n);
+}
+
+/* ARGSUSED */
+static int
+termp__t_pre(DECL_ARGS)
+{
+	struct mdoc_node *nn;
+
+	/*
+	 * If we're in an `Rs' and there's a journal present, then quote
+	 * us instead of underlining us (for disambiguation).
+	 */
+	nn = n->parent->parent;
+	if (nn && MDOC_Rs == nn->tok && nn->data.Rs->child_J)
+		return(termp_quote_pre(p, pair, m, n));
+
+	term_fontpush(p, TERMFONT_UNDER);
+	return(1);
+ }
 
 /* ARGSUSED */
 static int
Index: mdoc_validate.c
===================================================================
RCS file: /cvs/src/usr.bin/mandoc/mdoc_validate.c,v
retrieving revision 1.80
diff -u -p -r1.80 mdoc_validate.c
--- mdoc_validate.c	21 Dec 2010 23:57:31 -0000	1.80
+++ mdoc_validate.c	25 Dec 2010 17:23:38 -0000
@@ -127,6 +127,7 @@ static	int	 pre_it(PRE_ARGS);
 static	int	 pre_literal(PRE_ARGS);
 static	int	 pre_os(PRE_ARGS);
 static	int	 pre_par(PRE_ARGS);
+static	int	 pre_rs(PRE_ARGS);
 static	int	 pre_sh(PRE_ARGS);
 static	int	 pre_ss(PRE_ARGS);
 static	int	 pre_std(PRE_ARGS);
@@ -174,6 +175,7 @@ static	v_pre	 pres_fd[] = { NULL, NULL }
 static	v_pre	 pres_it[] = { pre_it, pre_par, NULL };
 static	v_pre	 pres_os[] = { pre_os, NULL };
 static	v_pre	 pres_pp[] = { pre_par, NULL };
+static	v_pre	 pres_rs[] = { pre_rs, NULL };
 static	v_pre	 pres_sh[] = { pre_sh, NULL };
 static	v_pre	 pres_ss[] = { pre_ss, NULL };
 static	v_pre	 pres_std[] = { pre_std, NULL };
@@ -265,7 +267,7 @@ const	struct valids mdoc_valids[MDOC_MAX
 	{ NULL, NULL },				/* Qo */
 	{ NULL, NULL },				/* Qq */
 	{ NULL, NULL },				/* Re */
-	{ NULL, posts_rs },			/* Rs */
+	{ pres_rs, posts_rs },			/* Rs */
 	{ NULL, NULL },				/* Sc */
 	{ NULL, NULL },				/* So */
 	{ NULL, NULL },				/* Sq */
@@ -979,6 +981,16 @@ pre_dd(PRE_ARGS)
 	return(1);
 }
 
+static int
+pre_rs(PRE_ARGS)
+{
+
+	assert(NULL == n->data.Rs);
+	n->data.Rs = mandoc_calloc(1, sizeof(struct mdoc_rs));
+
+	return(1);
+}
+
 
 static int
 post_bf(POST_ARGS)
@@ -1704,6 +1716,14 @@ post_rs(POST_ARGS)
 
 	next = NULL;
 	for (nn = mdoc->last->child->next; nn; nn = next) {
+
+		/*
+		 * Specifically note journal nodes because
+		 * they influence the formatting of the title.
+		 */
+		if (MDOC__J == nn->tok)
+			mdoc->last->parent->data.Rs->child_J = nn;
+
 		/* Determine order of `nn'. */
 		for (i = 0; i < RSORD_MAX; i++)
 			if (rsord[i] == nn->tok)

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

^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: mdocml: Specifying both %T and %J in an `Rs' block causes the title to
  2010-12-25 17:40 ` mdocml: Specifying both %T and %J in an `Rs' block causes the title to Ingo Schwarze
@ 2010-12-25 23:26   ` Kristaps Dzonsons
  0 siblings, 0 replies; 2+ messages in thread
From: Kristaps Dzonsons @ 2010-12-25 23:26 UTC (permalink / raw)
  To: tech

On 25/12/2010 19:40, Ingo Schwarze wrote:
> Hi Kristaps,
>
> kristaps@mdocml.bsd.lv wrote on Sat, Dec 25, 2010 at 08:50:37AM -0500:
>
>> Log Message:
>> -----------
>> Specifying both %T and %J in an `Rs' block causes the title to be quoted
>> instead of underlined.  This only happens in -Tascii, as -T[x]html both
>> underlines and italicises.
>
> Thanks for fixing this!
> Your approach mostly works and helped a lot to resolve this issue
> in OpenBSD; however, it can be simplified, see below for details.
>
>
> A patch against OpenBSD is appended at the end.
>
> However, we are now so badly out of sync that we can't push simple
> fixes back and forth any longer, it won't apply at all to bsd.lv.
> In a nutshell, all the %T code needs to know whether there is a %J
> node as well, so there is no need to do any calculations.
> It is sufficent (and more flexible) to just save a pointer.

Much nicer!  I checked in the same.

Thanks,

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

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2010-12-25 23:26 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
     [not found] <201012251350.oBPDob7Q026196@krisdoz.my.domain>
2010-12-25 17:40 ` mdocml: Specifying both %T and %J in an `Rs' block causes the title to Ingo Schwarze
2010-12-25 23:26   ` Kristaps Dzonsons

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).