Skip to content

Comments

Use mpadded for \raisebox MathML#876

Merged
k4b7 merged 3 commits intoKaTeX:masterfrom
ronkok:master
Sep 14, 2017
Merged

Use mpadded for \raisebox MathML#876
k4b7 merged 3 commits intoKaTeX:masterfrom
ronkok:master

Conversation

@ronkok
Copy link
Collaborator

@ronkok ronkok commented Sep 12, 2017

No description provided.

const dy = group.value.dy.value.number + group.value.dy.value.unit;
node.setAttribute("voffset", dy);
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.

Can you add a snapshot test for this?

@ronkok
Copy link
Collaborator Author

ronkok commented Sep 12, 2017

You know, I'm not sure I know how to reliably document MathML outcomes. For what it's worth, here's a screenshot.
raiseboxml

@k4b7
Copy link
Member

k4b7 commented Sep 12, 2017

@ronkok see https://github.com/Khan/KaTeX/blob/master/test/mathml-spec.js for examples of snapshot tests. After you add a test mathml-spec.js that uses .toMatchSnapshot(), run npm test and jest add a new snapshot to __snapshots__/mathml-spec.js.snap.

@ronkok
Copy link
Collaborator Author

ronkok commented Sep 12, 2017

Can you add a snapshot test for this?

It turns out that the answer to that question is no, I can't. My attempt to npm test failed on the make lint flow test.

@k4b7
Copy link
Member

k4b7 commented Sep 12, 2017

It turns out that the answer to that question is no, I can't. My attempt to npm test failed on the make lint flow test.

I can send you a PR for the snapshot test. Can you post the console output of the failure from npm test?

@ronkok
Copy link
Collaborator Author

ronkok commented Sep 12, 2017

Output as requested:

Windows PowerShell
Copyright (C) 2016 Microsoft Corporation. All rights reserved.

PS C:\Users\ronko> cd C:\Users\ronko\OneDrive\Documents\GitHub\KaTeX
PS C:\Users\ronko\OneDrive\Documents\GitHub\KaTeX> npm test

katex@0.8.4-pre test C:\Users\ronko\OneDrive\Documents\GitHub\KaTeX
make lint flow test

make i info Invoking lint target
(node:8744) UnhandledPromiseRejectionWarning: Unhandled promise rejection (rejection id: 1): undefined
make × ERR not found: make-$(NIS)

@k4b7
Copy link
Member

k4b7 commented Sep 13, 2017

@ronkok thanks. It looks like make isn't found. Does that sounds right? I think we should be able to modify the scripts in package.json to run commands in node_modules/.bin directly. The benefit of the Makefile at the moment is that it ensures that dependencies are up to date. I wonder if there's a way to do without using make.

@ronkok
Copy link
Collaborator Author

ronkok commented Sep 13, 2017

I think that's right. make is not, I believe, part of Windows. There are ways to get equivalents running in Windows and no doubt someone like @edemaine knows all about them. The problem here is that you are dealing with an amateur.

Go ahead and search for a make alternative if that is something with long term value. Don't do that on my account. I have 3 or 4 small PRs left to submit and then my work here will be done.

@ronkok
Copy link
Collaborator Author

ronkok commented Sep 14, 2017

One small step forward.

@k4b7 k4b7 merged commit a358c5a into KaTeX:master Sep 14, 2017
@k4b7
Copy link
Member

k4b7 commented Sep 14, 2017

@ronkok thanks for PR and for working through difficulties generating the snapshot.

@k4b7 k4b7 mentioned this pull request Sep 14, 2017
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