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

[WIP] Fix argument order of embed #200

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jwmerrill
Copy link
Member

@jwmerrill jwmerrill commented Feb 3, 2022

In LaTeX, optional arguments are enclosed in square brackets, and always come before required arguments. That's why e.g. a cube root is written as

\sqrt[3]{1+x}

and not

\sqrt{1+x}[3]

which is actually also valid latex that displays a square root followed by a 3 wrapped in square brackets.

The embed feature got this backwards. Fix it to use the standard order.

UPDATE: I don't think this parser is actually working correctly yet.

In LaTeX, optional arguments are enclosed in square brackets, and always
come _before_ required arguments. That's why e.g. a cube root is written
as
```
\sqrt[3]{1+x}
```
and not
```
\sqrt{1+x}[3]
```
which is actually also valid latex that displays a square root followed
by a 3 wrapped in square brackets.

The embed feature got this backwards. Fix it to use the standard order.
@jwmerrill jwmerrill changed the title Fix argument order of embed [WIP] Fix argument order of embed Feb 3, 2022
@@ -1200,7 +1200,7 @@ suite('Public API', function () {
assert.equal(mq.text(), 'sqrt(embedded text)');
assert.equal(mq.latex(), '\\sqrt{embedded latex}');

mq.latex('\\sqrt{\\embed{thing}[data]}');
Copy link
Member

Choose a reason for hiding this comment

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

we need to maintain backwards compatibility, right?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, we'll need to come up with a strategy here if we actually try to land this.

Maybe we let \embed stay broken forever and come up with a different name for the version that uses correct LaTeX syntax.

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