r99741 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r99740‎ | r99741 | r99742 >
Date:23:04, 13 October 2011
Author:thenub314
Status:ok (Comments)
Tags:math 
Comment:
The following patches restore how texvc added braces for all commands
but \operatorname to fix Bug 31442. More specifically, TEX_FUN1 now
adds braces as it did in previous versions. The result is texvc will
once again correctly sanitize multiple math accents even when no braces are
given (e.g. \dot \vec B) and it will allow math accents to be used with
symbols whose font as been changed (e.g. \tilde \mathcal{M}). Parser
tests are created to ensure these continue to work.

The handling of \operatorname is now handled specially by the
function TEX_FUN1nb (nb stands for no braces and the name is taken to
be consistent with the existing function TEX_FUN2nb). The addition of
braces causes this command to LaTeX incorrectly. The parser test for
\operatorname has been updated to reflect the fact the hash of the
images involved has changed.
Modified paths:
  • /trunk/extensions/Math/math/html.ml (modified) (history)
  • /trunk/extensions/Math/math/lexer.mll (modified) (history)
  • /trunk/extensions/Math/math/parser.mly (modified) (history)
  • /trunk/extensions/Math/math/tex.mli (modified) (history)
  • /trunk/extensions/Math/math/texutil.ml (modified) (history)
  • /trunk/extensions/Math/mathParserTests.txt (modified) (history)

Diff [purge]

Index: trunk/extensions/Math/mathParserTests.txt
——@@ -144,6 +144,24 @@
145145 !! input
146146 <math>\operatorname{sen}</math>
147147 !! result
148 -<p><img class="tex" alt="\operatorname{sen}" src="/images/math/3/e/7/3e71e75d9aac1bf5107271ad89ca7c3b.png" />
 148+<p><img class="tex" alt="\operatorname{sen}" src="/images/math/f/a/9/fa9660c7eb053ca8d3c9a87fa86635d9.png" />
149149 </p>
150150 !! end
 151+
 152+!! test
 153+BUG 31442: Multiple math accents without braces fails to parse
 154+!! input
 155+<math>\dot \vec B</math>
 156+!! result
 157+<p><img class="tex" alt="\dot \vec B" src="/images/math/e/6/4/e64939568ecb506a86a392373cec0458.png" />
 158+</p>
 159+!! end
 160+
 161+!! test
 162+BUG 31442: Math accents with math font fail to parse if braces not used
 163+!! input
 164+<math>\tilde \mathcal{M}</math>
 165+!! result
 166+<p><img class="tex" alt="\tilde \mathcal{M}" src="/images/math/5/5/0/55072ce6ef8c840c4b7687bd8a028bde.png" />
 167+</p>
 168+!! end
Index: trunk/extensions/Math/math/tex.mli
——@@ -11,6 +11,7 @@
1212 | TEX_BOX of string * string
1313 | TEX_BIG of string * Render_info.t
1414 | TEX_FUN1 of string * t
 15+ | TEX_FUN1nb of string * t
1516 | TEX_FUN2 of string * t * t
1617 | TEX_FUN2nb of string * t * t
1718 | TEX_INFIX of string * t list * t list
Index: trunk/extensions/Math/math/parser.mly
——@@ -5,7 +5,7 @@
66 let sq_close_ri = HTMLABLEC(FONT_UFH,"]", "]")
77 %}
88 %token <Render_info.t> LITERAL DELIMITER
9 -%token <string> FUN_AR2 FUN_INFIX FUN_AR1 DECL FUN_AR1opt BIG FUN_AR2nb
 9+%token <string> FUN_AR2 FUN_INFIX FUN_AR1 DECL FUN_AR1nb FUN_AR1opt BIG FUN_AR2nb
1010 %token <string*string> BOX
1111 %token <string*(string*string)> FUN_AR1hl
1212 %token <string*Render_info.font_force> FUN_AR1hf DECLh
——@@ -85,6 +85,7 @@
8686 | BIG SQ_CLOSE { TEX_BIG ($1,sq_close_ri) }
8787 | left expr right { TEX_LR ($1,$3,$2) }
8888 | FUN_AR1 lit { TEX_FUN1($1,$2) }
 89+ | FUN_AR1nb lit { TEX_FUN1nb($1,$2) }
8990 | FUN_AR1hl lit { let t,h=$1 in TEX_FUN1hl(t,h,$2) }
9091 | FUN_AR1hf lit { let t,h=$1 in TEX_FUN1hf(t,h,$2) }
9192 | FUN_AR1opt expr_nosqc SQ_CLOSE lit { TEX_FUN2sq($1,TEX_CURLY $2,$4) }
Index: trunk/extensions/Math/math/lexer.mll
——@@ -56,6 +56,7 @@
5757 | "-" { let str = Lexing.lexeme lexbuf in LITERAL (MHTMLABLEC (FONT_UFH,"-"," &minus; ",MO,str))}
5858 | literal_uf_op { let str = Lexing.lexeme lexbuf in LITERAL (MHTMLABLEC (FONT_UFH, str," "^str^" ",MO,str)) }
5959 | delimiter_uf_op { let str = Lexing.lexeme lexbuf in DELIMITER (MHTMLABLEC (FONT_UFH, str," "^str^" ",MO,str)) }
 60+ | "\\operatorname" { Texutil.tex_use_ams(); FUN_AR1nb "\\operatorname" }
6061 | "\\sqrt" space * "[" { FUN_AR1opt "\\sqrt" }
6162 | "\\xleftarrow" space * "[" { Texutil.tex_use_ams(); FUN_AR1opt "\\xleftarrow" }
6263 | "\\xrightarrow" space * "[" { Texutil.tex_use_ams(); FUN_AR1opt "\\xrightarrow" }
Index: trunk/extensions/Math/math/texutil.ml
——@@ -20,7 +20,8 @@
2121 | TEX_DQN (a) -> "_{" ^ (render_tex a) ^ "}"
2222 | TEX_UQN (a) -> "^{" ^ (render_tex a) ^ "}"
2323 | TEX_LITERAL s -> tex_part s
24 - | TEX_FUN1 (f,a) -> f ^ " " ^ (render_tex a)
 24+ | TEX_FUN1 (f,a) -> "{" ^ f ^ " " ^ (render_tex a) ^ "}"
 25+ | TEX_FUN1nb (f,a) -> f ^ " " ^ (render_tex a)
2526 | TEX_FUN1hl (f,_,a) -> "{" ^ f ^ " " ^ (render_tex a) ^ "}"
2627 | TEX_FUN1hf (f,_,a) -> "{" ^ f ^ " " ^ (render_tex a) ^ "}"
2728 | TEX_DECLh (f,_,a) -> "{" ^ f ^ "{" ^ (mapjoin render_tex a) ^ "}}"
Index: trunk/extensions/Math/math/html.ml
——@@ -56,6 +56,7 @@
5757 | TEX_BOX (_,s)::r -> s^html_render_flat ctx r
5858 | TEX_LITERAL (TEX_ONLY _)::_ -> raise Too_difficult_for_html
5959 | TEX_FUN1 _::_ -> raise Too_difficult_for_html
 60+ | TEX_FUN1nb _::_ -> raise Too_difficult_for_html
6061 | TEX_FUN2 _::_ -> raise Too_difficult_for_html
6162 | TEX_FUN2nb _::_ -> raise Too_difficult_for_html
6263 | TEX_FUN2h _::_ -> raise Too_difficult_for_html

Follow-up revisions

RevisionCommit summaryAuthorDate
r103197missing TEX_FUN1nb in html_render_deep()...hashar17:26, 15 November 2011
r103240REL1_18 MFT r98560, r99741, r101031, r101404, r102438, r102563, r102953, r103197reedy22:08, 15 November 2011
r1032421.18wmf1 MFT r98560, r99741, r101031, r101404, r102438, r102563, r102953, r10...reedy22:10, 15 November 2011

Comments

#Comment by Sumanah (talk | contribs)   16:54, 15 November 2011

I asked some OCaml people to comment on this. One said, "I don't see anything wrong in this commit, although, again, I am not aware of the whole project." Another said, "I guess I don't see anything problematic in the changelog there (though I can't comment much on the .mly stuff - I haven't used ocamlyacc) - are there any specific concerns?"

So that's somewhat reassuring.

#Comment by Hashar (talk | contribs)   19:25, 15 November 2011

This revision is not blocking the 1.18 tarball.

#Comment by Hashar (talk | contribs)   22:09, 15 November 2011

This one is good. i have reviewed the various tests cases on bug 31442 and they seems to render correctly on my Debian.

This can be merged to 1.18wmf1 and scap will recompile the math (thanks Aaron for that tip).

We probably want to add r103197 too although it will need to be reviewed.

#Comment by Hashar (talk | contribs)   22:37, 15 November 2011

Change deployed live! Thanks thenub314 :-)

Status & tagging log