Can we keep these tests but only check for Çç? Eventually we'll want to test for œ, Œ, æ, Æ, and ß when they get added.

This PR actually removes support for Çç. I believe they were added prematurely when forcing support for Latin-1. I just verified that none of the Latin-1 glyphs, Çç included, are in the KaTeX fonts. (I believe this actually makes sense, and the purpose of this PR is to map them into the accent macros, like LaTeX would.) Here's an example of incorrect looking Çç vs Cc in the current KaTeX:

I opened https://github.com/KaTeX/katex-fonts/issues/2 to track the cedilla issue.

I guess expanding to \grave A makes sense. Can you add a MathML fixture test? I'm curious to see if we recombine the accent with the letter when we render to MathML.

They render exactly how accent commands do, which I learned is via <mover accent="true">. I think this makes sense... Test added.

This contains many more characters than we support. Where do we generate an error if someone uses an unsupported combining mark? Maybe add a test for this.

I added an error test (in error-spec.js because I wanted toFailWithParseError). Incidentally, most of my tests are in katex-spec.js because I wanted toParseLike, but they would make more sense in unicode-spec.js... Not sure how to best share code between these test modules.

I've created #1021 to extract helper functions into test-utils.js. I can do that this weekend and then we could rebase this with the tests in the proper suite.

This is super cool. I wish I would've known about this sooner. There's no support for this in IE and Safari only got this in version 10. We might consider using https://github.com/walling/unorm instead.

Ah, I should have checked the browser compatibility matrix for String.normalize. The only problem with a polyfill is that it increases the katex.min.js build by 60%, from 226,836 bytes to 363,641 bytes (as any polyfill requires a huge table of Unicode symbols).

So I think this raises a serious question of whether we should:

1. Include the polyfill.
2. Not include the polyfill, and support Unicode in all supported browsers except IE.
3. Rewrite the PR to avoid String.normalize, at least in the built form. We actually only want symbols that expand to accents within our supported set, so the symbol table we could generate at build time could be smaller than unorm's.
4. Drop this PR altogether (though this is probably a desirable feature, and IE probably isn't evolving).

I think option 2 or 3 is best. Thoughts? I can look into option 3.

It's in Edge so my vote would be for option 2. If people need to support IE then they can add the polyfill themselves. We should document this requirement.

Sounds good. That's the easiest! I'll add a fallback when normalize isn't implemented, maybe with a warning, and add documentation.

If we bailing here do we need to include the combiningDiacriticalMarkString with surrogate pairs in the regex in Lexer.js?

combiningDiacriticalMarkString is already included in Lexer's tokenRegex both after a single codepoint and after a surrogate pair.

We have to lex the tokens before we can test the tokens against the cjkRegex here, d'oh.

Good enough for me.

Nice clean up!

Are there any unicode glyphs that include multiple accents? If so, we should test that this works correctly.

I believe the answer is "no" -- all the examples I've seen (e.g. here) show up to one accent in the first character, and the rest obtained with combining characters. But given the combiningDiacriticalMarkString in the Lexer, there could be arbitrarily many combining characters at this point in the code.

Nice cleanup, but I think this may actually cause the bundle size to increase quite a bit b/c it requires a polyfill for the string iterator which I believe uses .codePointAt() which also ends up be polyfilled. Can you generate a build and see how much the bundle size has increased?

I don't think there are polyfills involved, but this is what it expands to:

var _iteratorNormalCompletion = true;
var _didIteratorError = false;
var _iteratorError = undefined;

try {
    for (var _iterator = (0, _getIterator3.default)("0123456789/@.\""), _step; !(_iteratorNormalCompletion = (_step = _iterator.next()).done); _iteratorNormalCompletion = true) {
        var _ch = _step.value;

        defineSymbol(math, main, textord, _ch, _ch);
    }

    // All of these are textords in text mode
} catch (err) {
    _didIteratorError = true;
    _iteratorError = err;
} finally {
    try {
        if (!_iteratorNormalCompletion && _iterator.return) {
            _iterator.return();
        }
    } finally {
        if (_didIteratorError) {
            throw _iteratorError;
        }
    }
}

Or minified:

var M=true;var _=false;var S=undefined;try{for(var z=(0,n.default)('0123456789/@."'),T;!(M=(T=z.next()).done);M=true){var A=T.value;u(o,f,k,A,A)}}catch(e){_=true;S=e}finally{try{if(!M&&z.return){z.return()}}finally{if(_){throw S}}}var C=true;var O=false;var N=undefined;try{for(var j=(0,n.default)('0123456789!@*()-=+[]<>|";:?/.,'),L;!(C=(L=j.next()).done);C=true){var E=L.value;u(s,f,k,E,E)}}catch(e){O=true;N=e}finally{try{if(!C&&j.return){j.return()}}finally{if(O){throw N}}}

So it does add a few hundred bytes. Shall I switch back?

(To clarify, _getIterator3 gets defined even without these lines.)

The code that it generates is pretty awful. We should experiment with loose mode for the for-of plugin for babel sometime, see https://babeljs.io/docs/plugins/transform-es2015-for-of. Interestingly, when iterating over an array literal it will output a basic for loop. Maybe we could do for (const letter of "abc...".split()) { ... } to force this optimization.

How about the following, which Babel seems to leave as-is?

"0123456789/@.\"".split().forEach(function () {
    defineSymbol(math, main, textord, ch, ch);
});

Wow, this is actually fastest! jsperf benchmark

I wasn't expecting that. Awesome!

Oops, that's because I was using split wrong. It's now the slowest... So perhaps I should go back to a regular for loop.

right, b/c it still has to call charCodeAt... yeah, let's go back to regular for loops.

Cool!

I love the comments.

Do we need this in addition to the export default?

I'm still somewhat new to ES6, so I'm not sure what's normal here. I still find it weird that export default const is forbidden. I believe we could just do export default {...} if we don't need to name the object, so maybe that's best? The current system allows importing in two different ways (default or nondefault), but that's probably not necessary.

I find the lack of support for export default const weird as well. You can do:

export { foo as default };

I guess it's a moot point now since there no longer seems to be an export default in this file.

Maybe we could have Parser.js export accents instead.

I should add a comment about this. I originally tried it that way, but Node doesn't support import, and I'm running unicodeMake.js in Node, so Babel would have to get involved, and the accents are not available in built katex.js. I'm open to better suggestions! We could, for example, put accents here in unicodeMake.js and/or unicodeSymbols.js and import them in Parser.js -- perhaps that's best.

OK, I tried moving them. It's not especially pretty -- I still evald the code in unicodeMake, which led to extra \ escaping in the accents definition... Let me know if you see a cleaner way! But at least I'm no longer parsing Parser.js with a regex!

This is super cool.

I see, these got moved down to unicode-spec.js.

\\\" caught me off-guard until I realized that the " need to be escaped as well.

This change isn't really related to accents and should probably be in a separate PR.

It's relevant in that this PR rewrites how we parse Unicode. The relevant line change is from if (this.mode === "text" && cjkRegex.test(nucleus.text)) to if (cjkRegex.test(text)). I can't make a separate PR now because they'd both change that line, but if you like, I can remove that change from here and add a second PR once this one is accepted.

FWIW, this one-line change would fix #895.

I can remove that change from here and add a second PR once this one is accepted.

I think that makes the most sense. Thanks. :)

Removed.

Can you include when this tool should be run?

Added.

I wonder if we can run this as part of the build and then not even bother with committing the result. That we don't have to worry about whether people are running it when they should or not.

My motivation for committing it is so that people wouldn't worry about when to run it. (You need to run it whenever we add support for a new accent, or when the Unicode spec changes, neither of which I imagine being very common.) Another advantage is that we can directly see what characters are supported, so it's easy to complain about a missing one, and for PRs to show what additional characters are being supported.

If we do it during build, would we also have to do it when running the test server...? This approach felt more complicated to me, but I'm happy to have a specific suggestion.

Let's leave it like this for now. I definitely like it committing it from a documentation point of view. I don't think we have much work left to do on accents so there shouldn't be too many PRs which is affected by this.

I'm confused why the eval is necessary, can't we just console.log(accentsCode); this as is?

A better solution might be to do:

const {accents} = require('./unicodeSymbols.js');

const encodedAccents = {};
for (const [key, value] of Object.values(accents)) {
    const encodedEntry = {};
    encodedAccents[encode(key)] = encodedEntry;
    if (value.text) {
        encodedEntry.text = encode(value.text);
    }
    if (value.math) {
        encodedEntry.math = encode(value.math);
    }
}

This way we have one source of truths for the accents.

First, the eval is necessary to load this data into the current module. console.log is what outputs the same code to unicodeSymbols.js. In this way, there is already just one source, accentsCode in unicodeMake.js, which generates unicodeSymbols.js which has a copy of the data, but that file should never be modified...

Your bootstrapping approach above is neat, but feels awkward. You're asking unicodeMake.js to first import unicodeSymbols.js, but then modify unicodeSymbols.js... In particular, it won't work with the > piping as is, and is fragile if e.g. unicodeSymbols.js ever got corrupted (e.g. from a bug in unicodeMake.js), then you'd lose the accents table.

I have a third approach: make a unicodeAccents.js that just gives the accents list, and is written with Node-style modules (module.export = ... instead of export). Then it should be both requireable in unicodeMake.js and (I think) importable by the rest of KaTeX.