r87934 MediaWiki - Code Review archive

Repository:	MediaWiki
Revision:	< r87933‎ \| r87934 \| r87935 >
Date:	21:46, 12 May 2011
Author:	thenub314
Status:	reverted (Comments)
Tags:	brion, math
Comment:	This updates improves the MathML support of texvc. New new tags have been added, but the parser now recognizes numbers containing decimal points as being renderable using <mn>. The delimiters (,[,{ now render using <mo> tags, this allows for equations such as "y=f(x)" to be rendered to MathML. Finally for standard functions such as \sin, now render as <mi>\sin</mi>⁡, etc so standard function names are supported.
Modified paths:	/trunk/extensions/Math/math/html.ml (modified) (history) /trunk/extensions/Math/math/lexer.mll (modified) (history) /trunk/extensions/Math/math/mathml.ml (modified) (history) /trunk/extensions/Math/math/parser.mly (modified) (history) /trunk/extensions/Math/math/render_info.mli (modified) (history) /trunk/extensions/Math/math/texutil.ml (modified) (history)

Diff [purge]

Index: trunk/extensions/Math/math/mathml.ml
—	—	@@ -1,13 +1,15 @@
2	2	open Tex
3	3	open Render_info
4	4
5		~~-type t = TREE_MN of string \| TREE_MO of string \| TREE_MI of string~~
	5	+type t = TREE_MN of string \| TREE_MO of string \| TREE_MI of string \| TREE_MF of string \| TREE_MFB of string * string
6	6
7	7	let rec make_mathml_tree = function
8	8	TREE_MN a::otr,TEX_LITERAL(MHTMLABLEC(_,_,_,MN,b))::itr -> make_mathml_tree(TREE_MN (a^b)::otr,itr)
9	9	\| otr,TEX_LITERAL(MHTMLABLEC(_,_,_,MN,a))::itr -> make_mathml_tree(TREE_MN a::otr,itr)
10	10	\| otr,TEX_LITERAL(MHTMLABLEC(_,_,_,MO,a))::itr -> make_mathml_tree(TREE_MO a::otr,itr)
11	11	\| otr,TEX_LITERAL(MHTMLABLEC(_,_,_,MI,a))::itr -> make_mathml_tree(TREE_MI a::otr,itr)
	12	+ \| otr,TEX_LITERAL(MHTMLABLEC(_,_,_,MF,a))::itr -> make_mathml_tree(TREE_MF a::otr,itr)
	13	+ \| otr,TEX_LITERAL(MHTMLABLEFC(_,_,_,MF,a,b))::itr -> make_mathml_tree(TREE_MFB (a,b)::otr,itr)
12	14	\| otr,TEX_CURLY(crl)::itr -> make_mathml_tree(otr,crl@itr)
13	15	\| otr,[] -> List.rev otr
14	16	\| _ -> failwith "failed to render mathml"
—	—	@@ -16,5 +18,7 @@
17	19	TREE_MN s -> "<mn>"^s^"</mn>"
18	20	\| TREE_MI s -> "<mi>"^s^"</mi>"
19	21	\| TREE_MO s -> "<mo>"^s^"</mo>"
	22	+ \| TREE_MF s -> "<mi>"^s^"</mi><mo>⁡</mo>"
	23	+ \| TREE_MFB (s,b) -> "<mi>"^s^"</mi><mo>⁡</mo>"^"<mo>"^b^"</mo>"
20	24
21	25	let render tree = try Some (Util.mapjoin render_mathml_tree (make_mathml_tree ([],tree))) with _ -> None
Index: trunk/extensions/Math/math/parser.mly
—	—	@@ -2,7 +2,7 @@
3	3	open Tex
4	4	open Render_info
5	5
6		~~- let sq_close_ri = HTMLABLEC(FONT_UFH,"]", "]")~~
	6	+ let sq_close_ri = MHTMLABLEC(FONT_UFH,"]", "]",MO,"]")
7	7	%}
8	8	%token <Render_info.t> LITERAL DELIMITER
9	9	%token <string> FUN_AR2 FUN_INFIX FUN_AR1 DECL FUN_AR1opt BIG FUN_AR2nb
Index: trunk/extensions/Math/math/lexer.mll
—	—	@@ -6,9 +6,9 @@
7	7	let space = [' ' '\t' '\n' '\r']
8	8	let alpha = ['a'-'z' 'A'-'Z']
9	9	let literal_id = ['a'-'z' 'A'-'Z']
10		~~-let literal_mn = ['0'-'9']~~
	10	+let literal_mn = ['0'-'9' '.']
11	11	let literal_uf_lt = [',' ':' ';' '?' '!' '\'']
12		~~-let delimiter_uf_lt = ['(' ')' '.']~~
	12	+let delimiter_uf_lt = ['(' ')']
13	13	let literal_uf_op = ['+' '-' '*' '=']
14	14	let delimiter_uf_op = ['/' '\|']
15	15	let boxchars = ['0'-'9' 'a'-'z' 'A'-'Z' '+' '-' '*' ',' '=' '(' ')' ':' '/' ';' '?' '.' '!' '\'' '`' ' ' '\128'-'\255']
—	—	@@ -52,7 +52,7 @@
53	53	\| literal_id { let str = Lexing.lexeme lexbuf in LITERAL (MHTMLABLEC (FONT_IT, str,str,MI,str)) }
54	54	\| literal_mn { let str = Lexing.lexeme lexbuf in LITERAL (MHTMLABLEC (FONT_RM, str,str,MN,str)) }
55	55	\| literal_uf_lt { let str = Lexing.lexeme lexbuf in LITERAL (HTMLABLEC (FONT_UFH, str,str)) }
56		~~- \| delimiter_uf_lt { let str = Lexing.lexeme lexbuf in DELIMITER (HTMLABLEC (FONT_UFH, str,str)) }~~
	56	+ \| delimiter_uf_lt { let str = Lexing.lexeme lexbuf in DELIMITER (MHTMLABLEC (FONT_UFH, str,str,MO,str)) }
57	57	\| "-" { let str = Lexing.lexeme lexbuf in LITERAL (MHTMLABLEC (FONT_UFH,"-"," − ",MO,str))}
58	58	\| literal_uf_op { let str = Lexing.lexeme lexbuf in LITERAL (MHTMLABLEC (FONT_UFH, str," "^str^" ",MO,str)) }
59	59	\| delimiter_uf_op { let str = Lexing.lexeme lexbuf in DELIMITER (MHTMLABLEC (FONT_UFH, str," "^str^" ",MO,str)) }
—	—	@@ -60,20 +60,31 @@
61	61	\| "\\sqrt" space * "[" { FUN_AR1opt "\\sqrt" }
62	62	\| "\\xleftarrow" space * "[" { Texutil.tex_use_ams(); FUN_AR1opt "\\xleftarrow" }
63	63	\| "\\xrightarrow" space * "[" { Texutil.tex_use_ams(); FUN_AR1opt "\\xrightarrow" }
64		~~- \| "\\" (latex_function_names as name) space * "(" { LITERAL (HTMLABLEC(FONT_UFH,"\\" ^ name ^ "(", name ^ "(")) }~~
65		~~- \| "\\" (latex_function_names as name) space * "[" { LITERAL (HTMLABLEC(FONT_UFH,"\\" ^ name ^ "[", name ^ "[")) }~~
66		~~- \| "\\" (latex_function_names as name) space * "\\{" { LITERAL (HTMLABLEC(FONT_UFH,"\\" ^ name ^ "\\{", name ^ "{")) }~~
67		~~- \| "\\" (latex_function_names as name) space * { LITERAL (HTMLABLEC(FONT_UFH,"\\" ^ name ^ " ", name ^ " ")) }~~
68		~~- \| "\\" (mediawiki_function_names as name) space * "(" { (Texutil.tex_use_ams(); LITERAL (HTMLABLEC(FONT_UFH,"\\operatorname{" ^ name ^ "}(", name^ "("))) }~~
69		~~- \| "\\" (mediawiki_function_names as name) space * "[" { (Texutil.tex_use_ams(); LITERAL (HTMLABLEC(FONT_UFH,"\\operatorname{" ^ name ^ "}[", name^ "["))) }~~
70		~~- \| "\\" (mediawiki_function_names as name) space * "\\{" { (Texutil.tex_use_ams(); LITERAL (HTMLABLEC(FONT_UFH,"\\operatorname{" ^ name ^ "}\\{", name^ "{"))) }~~
71		~~- \| "\\" (mediawiki_function_names as name) space * { (Texutil.tex_use_ams(); LITERAL (HTMLABLEC(FONT_UFH,"\\operatorname{" ^ name ^ "}", name ^ " "))) }~~
	64	+ \| "\\" (latex_function_names as name) space * "("
	65	+ { LITERAL (MHTMLABLEFC(FONT_UFH,"\\" ^ name ^ "(", name ^ "(", MF, name, "(")) }
	66	+ \| "\\" (latex_function_names as name) space * "["
	67	+ { LITERAL (MHTMLABLEFC(FONT_UFH,"\\" ^ name ^ "[", name ^ "[", MF, name, "[")) }
	68	+ \| "\\" (latex_function_names as name) space * "\\{"
	69	+ { LITERAL (MHTMLABLEFC(FONT_UFH, "\\" ^ name ^ "\\{", name ^ "{", MF, name, "{")) }
	70	+ \| "\\" (latex_function_names as name) space *
	71	+ { LITERAL (MHTMLABLEC(FONT_UFH,"\\" ^ name, name ^ " ", MF, name)) }
	72	+ \| "\\" (mediawiki_function_names as name) space * "("
	73	+ { (Texutil.tex_use_ams(); LITERAL (MHTMLABLEFC(FONT_UFH,
	74	+ "\\operatorname{" ^ name ^ "}(", name ^ "(", MF, name, "("))) }
	75	+ \| "\\" (mediawiki_function_names as name) space * "["
	76	+ { (Texutil.tex_use_ams(); LITERAL (MHTMLABLEFC(FONT_UFH,
	77	+ "\\operatorname{" ^ name ^ "}[", name ^ "[", MF, name, "["))) }
	78	+ \| "\\" (mediawiki_function_names as name) space * "\\{"
	79	+ { (Texutil.tex_use_ams(); LITERAL (MHTMLABLEFC(FONT_UFH,
	80	+ "\\operatorname{" ^ name ^ "}\\{", name ^ "{", MF, name, "{"))) }
	81	+ \| "\\" (mediawiki_function_names as name) space *
	82	+ { (Texutil.tex_use_ams(); LITERAL (MHTMLABLEC(FONT_UFH,"\\operatorname{" ^ name ^ "}", name ^ " ", MF, name))) }
72	83	\| "\\," { LITERAL (HTMLABLE (FONT_UF, "\\,"," ")) }
73	84	\| "\\ " { LITERAL (HTMLABLE (FONT_UF, "\\ "," ")) }
74	85	\| "\\;" { LITERAL (HTMLABLE (FONT_UF, "\\;"," ")) }
75	86	\| "\\!" { LITERAL (TEX_ONLY "\\!") }
76		~~- \| "\\{" { DELIMITER (HTMLABLEC(FONT_UFH,"\\{","{")) }~~
77		~~- \| "\\}" { DELIMITER (HTMLABLEC(FONT_UFH,"\\}","}")) }~~
	87	+ \| "\\{" { DELIMITER (MHTMLABLEC(FONT_UFH,"\\{","{",MO,"{")) }
	88	+ \| "\\}" { DELIMITER (MHTMLABLEC(FONT_UFH,"\\}","}",MO,"}")) }
78	89	\| "\\\|" { DELIMITER (HTMLABLE (FONT_UFH,"\\\|","\|\|")) }
79	90	\| "\\_" { LITERAL (HTMLABLEC(FONT_UFH,"\\_","_")) }
80	91	\| "\\#" { LITERAL (HTMLABLE (FONT_UFH,"\\#","#")) }
—	—	@@ -109,7 +120,7 @@
110	121	\| '%' { LITERAL (HTMLABLEC(FONT_UFH,"\\%","%")) }
111	122	\| '$' { LITERAL (HTMLABLEC(FONT_UFH,"\\$","$")) }
112	123	\| '~' { LITERAL (HTMLABLE (FONT_UF, "~"," ")) }
113		~~- \| '[' { DELIMITER (HTMLABLEC(FONT_UFH,"[","[")) }~~
	124	+ \| '[' { DELIMITER (MHTMLABLEC(FONT_UFH,"[","[",MO,"[")) }
114	125	\| ']' { SQ_CLOSE }
115	126	\| '{' { CURLY_OPEN }
116	127	\| '}' { CURLY_CLOSE }
Index: trunk/extensions/Math/math/texutil.ml
—	—	@@ -9,6 +9,7 @@
10	10	\| HTMLABLEM (_,t,_) -> t
11	11	\| HTMLABLEC (_,t,_) -> t
12	12	\| MHTMLABLEC (_,t,_,_,_) -> t
	13	+ \| MHTMLABLEFC (_,t,_,_,_,_) -> t
13	14	\| HTMLABLE_BIG (t,_) -> t
14	15	\| TEX_ONLY t -> t
15	16
Index: trunk/extensions/Math/math/render_info.mli
—	—	@@ -11,10 +11,13 @@
12	12	MN
13	13	\| MI
14	14	\| MO
	15	+ \| MF
	16	+
15	17	type t =
16	18	HTMLABLEC of font_class * string * string
17	19	\| HTMLABLEM of font_class * string * string
18	20	\| HTMLABLE of font_class * string * string
19	21	\| MHTMLABLEC of font_class * string * string * math_class * string
	22	+ \| MHTMLABLEFC of font_class * string * string * math_class * string * string
20	23	\| HTMLABLE_BIG of string * string
21	24	\| TEX_ONLY of string
Index: trunk/extensions/Math/math/html.ml
—	—	@@ -28,6 +28,7 @@
29	29	TEX_LITERAL (HTMLABLE (ft,_,sh))::r -> (html_liberal (); (font_render sh (ctx,ft))^html_render_flat ctx r)
30	30	\| TEX_LITERAL (HTMLABLEC(ft,_,sh))::r -> (font_render sh (ctx,ft))^html_render_flat ctx r
31	31	\| TEX_LITERAL (MHTMLABLEC(ft,_,sh,_,_))::r -> (font_render sh (ctx,ft))^html_render_flat ctx r
	32	+ \| TEX_LITERAL (MHTMLABLEFC(ft,_,sh,_,_,_))::r -> (font_render sh (ctx,ft))^html_render_flat ctx r
32	33	\| TEX_LITERAL (HTMLABLEM(ft,_,sh))::r -> (html_moderate(); (font_render sh (ctx,ft))^html_render_flat ctx r)
33	34	\| TEX_LITERAL (HTMLABLE_BIG (_,sh))::r -> (html_liberal (); sh^html_render_flat ctx r)
34	35	\| TEX_FUN1hl (_,(f1,f2),a)::r -> f1^(html_render_flat ctx [a])^f2^html_render_flat ctx r
—	—	@@ -75,6 +76,7 @@
76	77	\| TEX_LITERAL (HTMLABLEM(ft,_,sh))::r -> (html_moderate(); ("",(font_render sh (ctx,ft)),"")::html_render_deep ctx r)
77	78	\| TEX_LITERAL (HTMLABLEC(ft,_,sh))::r -> ("",(font_render sh (ctx,ft)),"")::html_render_deep ctx r
78	79	\| TEX_LITERAL (MHTMLABLEC(ft,_,sh,_,_))::r -> ("",(font_render sh (ctx,ft)),"")::html_render_deep ctx r
	80	+ \| TEX_LITERAL (MHTMLABLEFC(ft,_,sh,_,_,_))::r -> ("",(font_render sh (ctx,ft)),"")::html_render_deep ctx r
79	81	\| TEX_LITERAL (HTMLABLE_BIG (_,sh))::r -> (html_liberal (); ("",sh,"")::html_render_deep ctx r)
80	82	\| TEX_FUN2h (_,f,a,b)::r -> (html_liberal (); (f a b)::html_render_deep ctx r)
81	83	\| TEX_INFIXh (_,f,a,b)::r -> (html_liberal (); (f a b)::html_render_deep ctx r)

Follow-up revisions

Revision	Commit summary	Author	Date
r96990	Revert changes to texvc that provide no test cases or examples of what they'r...	brion	19:00, 13 September 2011
r96993	MFT r96990: provisional revert of texvc changes that don't come with any test...	brion	19:07, 13 September 2011

Comments

#Comment by Thelema314 (talk | contribs) 14:00, 29 August 2011

As far as I can see from the diff, this is a well written change that does what it says. If it actually works when tested, keep the change.

#Comment by Brion VIBBER (talk | contribs) 00:41, 8 September 2011

What on earth do the parameters on these MHTMLABLE, MHTMLABLEFC, HTMLABLEM, etc mean? When a new one's added in like on MHTMLABLEFC, what does it mean? How can we tell whether parameter values are correct in future during maintenance?

#Comment by Brion VIBBER (talk | contribs) 01:07, 8 September 2011

Ok I think this is starting to make sense to me, but I'll want to add some doc comments for next time. :D

#Comment by Thelema314 (talk | contribs) 01:28, 8 September 2011

As far as I can tell, these parameters get used for each different things scattered around the code. I totally agree that this code should could be better commented with explanation of what's going on. If the values should be labeled, ocaml provides records, which work like the tuples being used but have named access to fields (instead of positional access). The first step to making this change would be to replace

+ | MHTMLABLEFC of font_class * string * string * math_class * string * string

with

type mhtmlablefc_t = { font_class: font_class;

                    string1: string;
                    string2: string;
                    math_class: math_class;
                    string3: string;
                    string4: string; }

... + | MHTMLABLEFC of mhtmlablefc_t

#Comment by Brion VIBBER (talk | contribs) 18:49, 13 September 2011

Note that MathML rendering will be removed since it's so poor that it's useless. Probably no point to committing and testing this.

#Comment by Brion VIBBER (talk | contribs) 19:04, 13 September 2011

Provisionally reverted in r96990 pending test cases.

Status & tagging log

19:04, 13 September 2011 Brion VIBBER (talk | contribs) changed the status of r87934 [removed: new added: reverted]
15:53, 13 September 2011 MarkAHershberger (talk | contribs) changed the tags for r87934 [added: brion]
23:14, 22 August 2011 Catrope (talk | contribs) changed the tags for r87934 [added: math]