New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support math font commands #132

Closed
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
10 participants
@kevinbarabash
Member

kevinbarabash commented Sep 27, 2014

This addresses issue #19 and partially addresses issue #42.

TODO:

  • \mathrm
  • \mathit
  • \mathbf
  • \mathsf
  • \mathtt
  • \mathbb
  • \mathfrak
  • \mathcal
  • \mathscr
  • test HTML output
  • test MathML output
  • get rid of fontStack
  • fallback to appropriate font, e.g. \mathbb{x} should look like \mathit{x}
  • ~~font aliases, e.g. \Bbb => \mathbb~~their behavior is different so I'd like to do this in a separate pull request after the other font commands land.
  • \imath \jmath
  • more unit tests
  • screenshotter tests for everything
  • rebase
  • accents use the correct fontrequires exporting shift metrics from mapping.pl and using the combining glyphs for each accent instead of the non-combining glyphs we're using right now.
@Hywan

This comment has been minimized.

Hywan commented Sep 30, 2014

👍

@kevinbarabash

This comment has been minimized.

Member

kevinbarabash commented Oct 1, 2014

I'll create some Huxley tests for this.

@kevinbarabash

This comment has been minimized.

Member

kevinbarabash commented Oct 2, 2014

I had to manually crop the images generated because the docker setup is producing images that 1024x767 as opposed to 1024x740 (size of images in the repo).

@spelufo

This comment has been minimized.

spelufo commented Feb 1, 2015

Very nice. I pulled your mathrm_mathbb branch just so I could use this. \mathbb{R} for real numbers is essential. Is there a reason why this isn't merged yet? Can I help in any way?

@martonbognar

This comment has been minimized.

martonbognar commented Mar 10, 2015

Are you considering implementing this? I think mathbb would be a great addition.

@underyx

This comment has been minimized.

underyx commented Mar 10, 2015

🔢 👍

@maxdumas

This comment has been minimized.

maxdumas commented Mar 12, 2015

I agree this should be merged. This feature is absolutely critical for me to use KaTeX as I work with sets a lot.

@akof1314

This comment has been minimized.

akof1314 commented Apr 29, 2015

I agree this should be merged.

@PeterBocan

This comment has been minimized.

PeterBocan commented Apr 29, 2015

@xymostech 👍

@justjanne

This comment has been minimized.

justjanne commented May 17, 2015

👍 Only found out KaTeX had no mathbb today, because I assumed it’d just have it – this is a must have!

@kevinbarabash kevinbarabash force-pushed the kevinbarabash:mathrm_mathbb branch 4 times, most recently from 8937825 to 52a8584 May 18, 2015

@@ -23,6 +23,16 @@ var makeText = function(text, mode) {
return new mathMLTree.TextNode(text);
};
// Keep track of the current font specified by /mathbb, /mathit, or /mathrm
var fontStack = [];

This comment has been minimized.

@kevinbarabash

kevinbarabash May 18, 2015

Member

Unfortunately, we can't use the same approach used for color b/c the mathvariant attribute is only valid on <mi>, <mn>, <mo>, <ms>, <mtext> so we have to set it on each of them individually. This means we have to keep track of the current font.

@kevinbarabash

This comment has been minimized.

Member

kevinbarabash commented May 18, 2015

I've rebased the code and added support for MathML. I need to update the screenshots though using the new screenshotter.

@kevinbarabash kevinbarabash force-pushed the kevinbarabash:mathrm_mathbb branch from 52a8584 to 07572d9 May 25, 2015

@kevinbarabash

This comment has been minimized.

Member

kevinbarabash commented May 25, 2015

The screenshots have been updated using the new screenshotter. @xymostech This is ready for review now.

@kevinbarabash kevinbarabash referenced this pull request May 27, 2015

Closed

Differential #238

@kevinbarabash kevinbarabash force-pushed the kevinbarabash:mathrm_mathbb branch from 07572d9 to ecc5170 May 27, 2015

@kevinbarabash kevinbarabash changed the title from Added support for \mathbb, \mathrm, and \mathit to Added support for \mathbb, \mathrm, \mathit, \mathcal, and \mathfrak May 27, 2015

"mathrm": "normal",
"mathcal": "script",
"mathfrak": "fraktur"
};

This comment has been minimized.

@kevinbarabash

kevinbarabash May 27, 2015

Member

These values are based on the "mathvariant" attribute in MathML produced by MathJax.

@gagern

This comment has been minimized.

Collaborator

gagern commented Jun 20, 2015

I should have looked at this before starting my own work on this issue, as I did in #258. Nice work already. Your MathML construction is very elegant with that math-variant thingy. It avoids those exotic unicode symbols which I had been using.

One thing that I think I got right is the list of supported characters for each font. With your code, if someone drops a lower-case letter in any of these functions (except for k in \mathbb), it will use the KaTeX_Main font and display a warning message about missing metrics. I think it would be better to fall back to normal italic symbols in those cases. That's what MathJax is doing there, and it feels right to me. LaTeX just renders whatever symbols happen to be in the AMS font in these locations, but I'd consider this a bug, not a feature. I haven't yet decided if the MathML should do such a fall-back as well, i.e. whether it should be closer to the input or closer to the output if these don't agree.

The global variable fontStack in buildMathML is really ugly. In particular, if one rendering run aborts due to an exception, subsequent runs may start with a non-empty stack. Perhaps we should add an options object, as it's used in buildHTML. Or perhaps we should make all of these functions methods of some object which can be used to hold such state. Or we should at least clean out the stack at the top level invocation for buildMathML.

Do you want to add \mathscr as well? Can this be distinguished from \mathcal in the MathML variant? How about the obsolete but shorter \Bbb synonym to \mathbb? I believe that there may be similar synonyms for the other fonts as well.

@kevinbarabash

This comment has been minimized.

Member

kevinbarabash commented Jun 20, 2015

It should probably fallback to whatever font is appropriate. We don't have double struck numbers so we should use KaTeX_Main for those and KaTeX_Math for lowercase letters (and Greek) as suggested. MathJax's MathML output matches its fallbacks.

I will do something about the fontStack. Thanks for pointing out the exception issue.

I can add \mathscr. Its mathvariant would also be script. MathJax adds an additional class attribute to make the distinction in its MathML rendering.

If \Bbb is obsolete what's the motivation in supporting it?

@gagern

This comment has been minimized.

Collaborator

gagern commented Jun 20, 2015

If \Bbb is obsolete what's the motivation in supporting it?

Because people are till using it. In my opinion it's the same reason which made you jump through hoops to get \over working despite the existence of \frac, except that there are no serious hoops involved for \Bbb. OK, \over is plain TeX, right, which hasn't really been “obsoleted” by LaTeX, but that's just a matter of terminology. Both have more modern replacements, both are being used nonetheless, and both are supported on the (La)TeX side, even if a notice gets displayed.

@kevinbarabash

This comment has been minimized.

Member

kevinbarabash commented Jun 20, 2015

Good enough for me.

@kevinbarabash kevinbarabash changed the title from Added support for \mathbb, \mathrm, \mathit, \mathcal, and \mathfrak to Support math font commands Jun 28, 2015

if zero_indexed:
info = self.char_info[char_num - self.start_char]
else:
info = self.char_info[char_num + self.start_char]

This comment has been minimized.

@kevinbarabash

kevinbarabash Jun 28, 2015

Member

The script font, rsfs, is zero_indexed meaning that although the start_char is 65, the first glyph index is 0. TODO: update the mapping so that we don't have to do - self.start_char here.

This comment has been minimized.

@kevinbarabash

kevinbarabash Jun 28, 2015

Member

I tried implementing the TODO, but I could get it to work. I'm probably misunderstanding something about how the mapping works. I'm going to leave this as is and add some comments.

@kevinbarabash kevinbarabash force-pushed the kevinbarabash:mathrm_mathbb branch from 35507f1 to c71c64e Jun 28, 2015

@kevinbarabash

This comment has been minimized.

Member

kevinbarabash commented Jun 28, 2015

Implemented \imath and \jmath as per #191.

module.exports = {
makeSymbol: makeSymbol,
fontMap: fontMap,

This comment has been minimized.

@kevinbarabash

kevinbarabash Jun 29, 2015

Member

the fontMap feels common because it's used in both buildHTML.js and builtMathML.js

mathtt: mathtt,
mathscr: mathscr,
mathsf: mathsf,
mathsym: mathsym,

This comment has been minimized.

@kevinbarabash

kevinbarabash Jun 29, 2015

Member

mathit, mathrm, etc. are not common b/c they're only used in buildHTML.js and should be moved into that file.

This comment has been minimized.

@gagern

gagern Jun 30, 2015

Collaborator

Then makeVList, makeSpan and so on should be moved as well, I think. Except if you consider these things common to different groupTypes, in which case the file name is just a bit confusing. In any case, I'd say that's a separate issue, and I'd leave mathit in here for now.

@kevinbarabash kevinbarabash force-pushed the kevinbarabash:mathrm_mathbb branch 2 times, most recently from 7339968 to db99cd5 Jun 29, 2015

"mathbf": {
variant: "bold",
test: function(value) {
return /[a-zA-Z0-9]/.test(value.charAt(0));

This comment has been minimized.

@gagern

gagern Jun 30, 2015

Collaborator

Why the charAt(0)? Shouldn't this be a single-symbol group in any case?

@gagern

This comment has been minimized.

Collaborator

gagern commented Jun 30, 2015

Personally, I'd combine all the screenshot tests into a single one, e.g. using the array environment for multi-line support. But that's just my taste, I guess, trying to keep the number of image files in the repository small.

I guess I'd also include \mathbb k in the mathbb screenshot. And perhaps the KaTeX symbol in one of them, to verify it's not affected by font changes.

@gagern

This comment has been minimized.

Collaborator

gagern commented Jun 30, 2015

The old font commands (those declared using \DeclareOldFontCommand) don't affect the single following group, but instead the current group to the end of that group, the way sizing commands work. So they are not exact synonyms. I guess that we generally should have a screenshot to verify the scope of each of these commands: affecting deep levels not just the top level, but ending after the argument (for the modern notation) resp. at the end of the current group (for the old notation).

@kevinbarabash

This comment has been minimized.

Member

kevinbarabash commented Jun 30, 2015

Personally, I'd combine all the screenshot tests into a single one, e.g. using the array environment for multi-line support.

I was thinking about doing this, but I'm worried about having dependencies between tests.

The old font commands (those declared using \DeclareOldFontCommand) don't affect the single following group, but instead the current group to the end of that group, the way sizing commands work.

Thanks for highlighting the distinction. I'll save those "alias" commands for another pull request.

I guess that we generally should have a screenshot to verify the scope of each of these commands

Agreed.

@kevinbarabash kevinbarabash referenced this pull request Jul 3, 2015

Open

Provide access to all symbols #286

6 of 9 tasks complete
@kevinbarabash

This comment has been minimized.

Member

kevinbarabash commented Jul 3, 2015

And perhaps the KaTeX symbol in one of them, to verify it's not affected by font changes.

Interestingly LaTeX doesn't support \LaTeX inside math mode.

@kevinbarabash

This comment has been minimized.

Member

kevinbarabash commented Jul 4, 2015

The plan is to break this PR into 3 separate PRs, namely:

  • font metrics + CSS changes
  • HTML implementation + screenshot tests
  • MathML implementation + unit tests
    Once each of these have been merged, I'll close this PR.

@kevinbarabash kevinbarabash force-pushed the kevinbarabash:mathrm_mathbb branch 2 times, most recently from 5363849 to 11f4e71 Jul 4, 2015

Support math font commands
implements \mathbb, \mathrm, \mathit, \mathcal, and \mathfrak

Added support for \mathbf, \mathtt, and \mathscr

added support for \mathsf, fixed output of \mathbf and \mathrm

implemented fallbacks for all fonts

added \imath and \jmath

created screenshots for font command and removed the old ones

Updated buildMathML.js so that mathvariant attributes are the same for the same value and font as is specified in buildCommon.js

fix lint

add my glyphs to mappings.pl, refactored to just check if an ord is in a given font

fix lint, remove commented code

updated test images to include accents and \KaTeX, removed aliases, removed redundant code in buildMathML.js

added more MathML tests

@kevinbarabash kevinbarabash force-pushed the kevinbarabash:mathrm_mathbb branch from 11f4e71 to 4e60d10 Jul 4, 2015

@kevinbarabash

This comment has been minimized.

Member

kevinbarabash commented Jul 4, 2015

Depends on:

@kevinbarabash

This comment has been minimized.

Member

kevinbarabash commented Sep 1, 2015

Closing because this has been implemented in #290, #291, and #292.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment