Skip to content
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

Add \smash, laps, spaces, and phantoms #833

Merged
merged 14 commits into from
Sep 2, 2017
Merged

Add \smash, laps, spaces, and phantoms #833

merged 14 commits into from
Sep 2, 2017

Conversation

ronkok
Copy link
Collaborator

@ronkok ronkok commented Aug 29, 2017

  1. Support \smash, including the optional argument from AMS.

  2. Change \llap and \rlap so that they render in text style. Repeat: This changes KaTeX behavior.

  3. Add \mathllap and \mathrlap. These will act as they do in mathtools and as in previous KaTeX versions of \llap and \rlap.

  4. Add \mathclap and \clap.

  5. Add \hphantom and \vphantom.

  6. Add \thinspace, \medspace, \thickspace

  7. Add \hspace.

This work will resolve issue #270 and parts of #50 and #164.

A. Perlis has written a concise description of items 1 thru 5. Except for \smash's optional argument. It's described in the AMS User's Guide.

Item 6 also comes from the AMS User's Guide.

1. Support `\smash`, including the optional argument from AMS.

2. Change `\llap` and `\rlap` so that they render in text style. Repeat: This *changes* KaTeX behavior.

3. Add `\mathllap` and `\mathrlap`. These will act as they do  in `mathtools` and as in previous KaTeX versions of `\llap` and `\rlap`.

4. Add `\mathclap` and `\clap`.

5. Add `\hphantom` and \vphantom`.

6. Add `\thinspace`, `\medspace`, `\thickspace`

7. Add `\hspace`.

This work will resolve issue #270 and parts of #50 and #164.

A. Perlis has written a [concise description](https://www.math.lsu.edu/~aperlis/publications/mathclap/perlis_mathclap_24Jun2003.pdf) of items 1 thru 5. Except for `\smash`'s optional argument. It's described in the [AMS User's Guide](http://texdoc.net/texmf-dist/doc/latex/amsmath/amsldoc.pdf).

Item 6 also comes from the AMS User's Guide.
@ronkok
Copy link
Collaborator Author

ronkok commented Aug 29, 2017

My apologies for the disconnectedness in this PR. And @edemaine thank you for the help in getting restarted.

@kohler I haven't yet looked into your comment about calling makeVList from groupTypes.smash. I'll do that later this evening.

@ronkok
Copy link
Collaborator Author

ronkok commented Aug 29, 2017

@kohler, I've taken your advice and exploited makeVList. I've applied it to \smash and \hphantom. That should be enough shouldn't it?

"\\thickspace": {
size: "0.277778em",
className: "thickspace",
},
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are these separate commands or would it make sense to use defineMacro to define \thickspace in terms of \;?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using defineMacro would work very well. I'll change it.

.clap > .inner > span {
margin-left: -50%;
margin-right: 50%;
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like there might be an easier way to center things. I'll check out the branch and experiment.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I never got around to experimenting. This does the trick and anything else would be marginally smaller so I'm going to merge this as is.

@@ -715,7 715,7 @@ describe("A text parser", function() {
const textExpression = "\\text{a b}";
const noBraceTextExpression = "\\text x";
const nestedTextExpression =
"\\text{a {b} \\blue{c} \\textcolor{#fff}{x} \\llap{x}}";
"\\text{a {b} \\blue{c} \\textcolor{#fff}{x} \\mathllap{x}}";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why change all \laps in this file to \mathlaps?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seemed to me that \mathllap is a core command. I found \llap to be less interesting. But your point is well taken. This line is supposed to be testing a text parser. I'll change it back.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, now I have a better answer to your question about the test. Since \lap is a test mode function, we should not expect it to work properly when given math mode arguments, such as \left. So the tests really should be revised to test \mathllap and not \lllap.

@@ -107,7 107,7 @@ KaTeX: \KaTeX
Kern:
tex: \frac{a\kern{1em}b}{c}a\kern{1em}b\kern{1ex}c\kern{-0.25em}d
nolatex: LaTeX fails to typeset this, “Missing number, treated as zero.”
Lap: ab\llap{f}cd\rlap{g}h
Lap: ab\mathllap{f}cd\mathrlap{g}h
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should probably also verify the changed behavior of llap et al.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll add some tests.

src/functions.js Outdated
numArgs: 1,
allowedInText: true,
}, function(context, args) {
const body = args[0];
return {
type: context.funcName.slice(1),
type: "lap",
className: context.funcName.slice(5),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd like to avoid including CSS specifics at this level, maybe this could be called alignment.

const inner = buildExpression(group.value.value, options);
const node = new mathMLTree.MathNode("mphantom", inner);
node.setAttribute("width", "0px");
return node;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your attention to detail with this.

src/buildHTML.js Outdated
// ref: amsmath: \renewcommand{\smash}[1][tb]{%
// def\mb@t{\ht}\def\mb@b{\dp}\def\mb@tb{\ht\z@\z@\dp}%
smashHeight = /t/.test(group.value.tb);
smashDepth = /b/.test(group.value.tb);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, the tb optional param is used to "smash" the top and/or the bottom of the group.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These tests could be moved into functions.js so that we don't have repeat the here and in buildMathML.js.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll do that.

src/functions.js Outdated
};
});

// smash, with optional [bt], as in AMS
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: the comment uses bt but everywhere else it's tb

src/functions.js Outdated
body: body,
tb: tb,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

smashHeight: /t/.test(tbArg),
smashDepth: /b/.test(tbArg),

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we care if there are other values in tbArg?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I suppose we do care. I just checked and AMS LaTeX ignores the \smash if I put another character in there. Here's an example and the first exponent has \smash[tab]{2}

I'll change the code to act in the same way.

ronkok and others added 7 commits August 31, 2017 10:43
\llap is fundamentally a text-mode function. We should not expect it to work correctly when given math-mode arguments. So test \mathllap instead.
A correction. The previous macro returned an error if given an argument with math-mode content, such as x^2.

The corrected macro will not return an error. It will instead return well rendered math, but letters are in `\mathrm` font.
@kevinbarabash
Copy link
Member

@ronkok I've updated the \llap, \clap, and \rlap to use textrm instead of mathrm. The unit tests are passing now. I'll regenerate the Lap screenshots.

Copy link
Member

@kevinbarabash kevinbarabash left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Thanks for making changes.

@kevinbarabash kevinbarabash merged commit 092aa0c into KaTeX:master Sep 2, 2017
@ronkok ronkok mentioned this pull request Sep 7, 2017
@ronkok
Copy link
Collaborator Author

ronkok commented Oct 15, 2017

@kevinbarabash, It just occurred to me that there is another breaking change in release 0.9.0-alpha, one that is not yet listed in the release notes.

The \llap and \rlap functions now render in text style, which follows LaTeX behavior. Also, KaTeX now has functions \mathllap and \mathrlap. These will act as they do in mathtools and as in previous KaTeX versions of \llap and \rlap.

My current draft of the function support page has been so updated. But I did not emphasize the change as much as I did when \color changed. If you want that draft revised, let me know.

@kevinbarabash
Copy link
Member

Good call. I can up date the release notes.

@kevinbarabash
Copy link
Member

I've updated the release notes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants