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

Use mpadded for \raisebox MathML #876

Merged
merged 3 commits into from
Sep 14, 2017
Merged

Use mpadded for \raisebox MathML #876

merged 3 commits into from
Sep 14, 2017

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

@kevinbarabash
Copy link
Member

@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.

@kevinbarabash
Copy link
Member

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)

@kevinbarabash
Copy link
Member

@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.

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

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

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.

None yet

2 participants