update fonts and metrics so that accents are positioned correctly #1094
Conversation
src/symbols.js
Outdated
| defineSymbol(text, main, accent, "\u02ca", "\\'"); // acute | ||
| defineSymbol(text, main, accent, "\u02cb", "\\`"); // grave | ||
| defineSymbol(text, main, accent, "\u00b4", "\\'"); // acute | ||
| defineSymbol(text, main, accent, "\u0060", "\\`"); // grave |
There was a problem hiding this comment.
It would be nice if all of these were in the \u02c0 to \u02cf range.
| @@ -666,7 +666,7 @@ defineSymbol(text, main, accent, "\u02d9", "\\."); // dot above | |||
| defineSymbol(text, main, accent, "\u02da", "\\r"); // ring above | |||
| defineSymbol(text, main, accent, "\u02c7", "\\v"); // caron | |||
| defineSymbol(text, main, accent, "\u00a8", '\\"'); // diaresis | |||
|
It looks like I broke |
| \text{\'\i} & \text{\.\i} & \text{\`\i} & \text{\"\i} & \text{\H\i} & \text{\r\i} \\ | ||
| \text{\'\j} & \text{\.\j} & \text{\`\j} & \text{\"\j} & \text{\H\j} & \text{\r\j} \\ | ||
| \text{\'a} & \text{\.a} & \text{\`a} & \text{\"a} & \text{\H{a}} & \text{\r{a}} \\ | ||
| \text{\'A} & \text{\.A} & \text{\`A} & \text{\"A} & \text{\H{A}} & \text{\r{A}} \\ |
There was a problem hiding this comment.
KA has specific bug reports from translators about e with a double acute accent and turkish dotless i and dotted captial I, so if you're willing to add those here as well that would be great.
| @@ -42,7 +42,7 @@ exports[`A MathML builder accents turn into <mover accent="true"> in MathML 1`] | |||
| e | |||
| </mi> | |||
| <mo> | |||
| ´ | |||
There was a problem hiding this comment.
This changed b/c we switched some of the accents to be in the "Spacing Modifier Characters" Unicode block. See https://en.wikipedia.org/wiki/Spacing_Modifier_Letters.
test/screenshotter/ss_data.yaml
Outdated
| \text{\'\j} & \text{\.\j} & \text{\`\j} & \text{\"\j} & \text{\H\j} & \text{\r\j} \\ | ||
| \text{\'a} & \text{\.a} & \text{\`a} & \text{\"a} & \text{\H{a}} & \text{\r{a}} \\ | ||
| \text{\'A} & \text{\.A} & \text{\`A} & \text{\"A} & \text{\H{A}} & \text{\r{A}} \\ | ||
| \text{\.I} & \text{\H e} & \text{\i ı} |
There was a problem hiding this comment.
@davidflanagan would you like me to change \text{\H e} to \text{\H ee̋ }?
There was a problem hiding this comment.
That would be great, if you don't mind. And maybe add an explicit dotted capital I as well to make the whole row the same. Thanks!
There was a problem hiding this comment.
Thanks for fixing this, Kevin! (And thanks for talking me through how fonts work in KaTeX so I could understand the patch)
It is pretty confusing that when we've got an input character like á, we map that (in unicodeSymbols) to \u0061\u0301 (using a combining acute accent) and then (in Parser.js) break those apart and map (from unicodeAccents.js) \u301 to ' or \acute, and then somewhere map those TeX functions to the non-combining \u02ca form of the acute accent.
It works though, so I'm not complaining. Maybe file an issue to write some documentation about how this is all handled though.
Also, src/domTree.js has some special cases for forms of lowercase letter i. I'm not sure why those are there, but they also use the combining form of the accents rather than the non-combining form. Please check that code still makes sense with this change before merging.
| "176": [0, 0.69444, 0, 0, 0.86944], | ||
| "177": [0.13333, 0.63333, 0, 0, 0.89444], | ||
| "180": [0, 0.69444, 0, 0, 0.575], | ||
| "198": [0, 0.68611, 0, 0, 1.04166], |
There was a problem hiding this comment.
feature request for the future: it would be nice if the script that generates this file included comments up at the top saying that it is an autogenerated file and explaining how to regenerate it.
There was a problem hiding this comment.
Good call, I've opened an issue for this.
I've opened an issue for this. |
I had a look at the PR for that and it sounds like I wasn't doing the right thing at the time anyways. Also, we don't allow any other unicode chars outside of text mode. I'm okay with removing that behavior. We'll want to mention it in the release notes as a breaking change. Fixing that probably makes sense as its own PR though. |
|
@kevinbarabash What does this do regarding |
|
@ronkok I haven't fixed the |
|
@kevinbarabash Thanks for the update. I will take no action. |
This depends on KaTeX/katex-fonts#5 which changed almost every combining glyph in the fonts to be non-combining. The ones that are left don't appear to be used. I'd like to remove unused glyphs from the fonts eventually but in a separate PR.