Skip to content

Commit

Permalink
🧮 Fix math-spacing in paragraphs (#1032)
Browse files Browse the repository at this point in the history
  • Loading branch information
rowanc1 authored Mar 26, 2024
1 parent 117d600 commit 2010854
Show file tree
Hide file tree
Showing 30 changed files with 480 additions and 70 deletions.
5 changes: 5 additions & 0 deletions .changeset/chilled-tigers-fail.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"myst-to-tex": patch
---

Respect math node tightness
5 changes: 5 additions & 0 deletions .changeset/hip-zoos-thank.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"myst-spec-ext": patch
---

Add tightness to math node extension
5 changes: 5 additions & 0 deletions .changeset/honest-penguins-shave.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"markdown-it-myst": patch
---

Add block tightness to the directives
5 changes: 5 additions & 0 deletions .changeset/modern-frogs-join.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"myst-directives": patch
---

Add math tightness to the directive
5 changes: 5 additions & 0 deletions .changeset/nasty-forks-look.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"myst-parser": patch
---

Pass src to the state handlers
5 changes: 5 additions & 0 deletions .changeset/popular-chairs-swim.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"mystmd": patch
---

Handle math tightness for latex
5 changes: 5 additions & 0 deletions .changeset/rich-waves-taste.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"myst-parser": patch
---

compute AMS math tightness based on source
5 changes: 5 additions & 0 deletions .changeset/stale-wolves-lie.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"myst-transforms": patch
---

Keep math tightness when lifting from paragraphs
5 changes: 5 additions & 0 deletions .changeset/thirty-months-pretend.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"myst-common": patch
---

Add block tightness to the directive data
16 changes: 16 additions & 0 deletions packages/markdown-it-myst/src/directives.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,20 @@ import { stateError, stateWarn } from './utils.js';

const COLON_OPTION_REGEX = /^:(?<option>[^:\s]+?):(\s*(?<value>.*)){0,1}\s*$/;

function computeBlockTightness(
src: string,
map: [number, number] | null | undefined,
): boolean | 'before' | 'after' {
const lines = src.split('\n');
const tightBefore =
typeof map?.[0] === 'number' && map[0] > 0 ? lines[map[0] - 1].trim() !== '' : false;
const tightAfter =
typeof map?.[1] === 'number' && map[1] < lines.length ? lines[map[1]].trim() !== '' : false;
const tight =
tightBefore && tightAfter ? true : tightBefore ? 'before' : tightAfter ? 'after' : false;
return tight;
}

/** Convert fences identified as directives to `directive` tokens */
function replaceFences(state: StateCore): boolean {
for (const token of state.tokens) {
Expand Down Expand Up @@ -51,6 +65,8 @@ function runDirectives(state: StateCore): boolean {
directiveOpen.meta = {
arg,
options: simplifyDirectiveOptions(options),
// Tightness is computed for all directives (are they separated by a newline before/after)
tight: computeBlockTightness(state.src, token.map),
};
const startLineNumber = map ? map[0] : 0;
const argTokens = directiveArgToTokens(arg, startLineNumber, state);
Expand Down
15 changes: 15 additions & 0 deletions packages/markdown-it-myst/tests/directives.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -176,4 +176,19 @@ describe('parses directives', () => {
expect(tokens.map((t) => t.type)).toEqual(['fence']);
expect(tokens[0].info).toEqual(' { ab c }');
});
it.each([
[false, 'Paragraph\n\n```{math}\nAx=b\n```\n\nAfter paragraph'],
[false, '```{math}\nAx=b\n```'],
[false, '```{math}\nAx=b\n```\n\nAfter paragraph'],
[true, 'Paragraph\n```{math}\nAx=b\n```\nAfter paragraph'],
['before', 'Paragraph\n```{math}\nAx=b\n```\n\nAfter paragraph'],
['before', 'Paragraph\n```{math}\nAx=b\n```'],
['after', 'Paragraph\n\n```{math}\nAx=b\n```\nAfter paragraph'],
['after', '```{math}\nAx=b\n```\nAfter paragraph'],
])('directives have tightness information: "%s"', (tight, src) => {
const mdit = MarkdownIt().use(plugin);
const tokens = mdit.parse(src, {});
const open = tokens.find((t) => t.type === 'parsed_directive_open');
expect(open?.meta.tight).toBe(tight);
});
});
2 changes: 1 addition & 1 deletion packages/myst-common/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ export type OptionDefinition = ArgDefinition & {

export type DirectiveData = {
name: string;
node: Directive;
node: Directive & { tight?: boolean | 'before' | 'after' };
arg?: ParseTypes;
options?: Record<string, ParseTypes>;
body?: ParseTypes;
Expand Down
19 changes: 11 additions & 8 deletions packages/myst-directives/src/math.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,13 +15,16 @@ export const mathDirective: DirectiveSpec = {
},
run(data: DirectiveData): GenericNode[] {
const { label, identifier } = normalizeLabel(data.options?.label as string | undefined) || {};
return [
{
type: 'math',
identifier,
label,
value: data.body as string,
},
];
const math = {
type: 'math',
identifier,
label,
value: data.body as string,
} as GenericNode;
if (data.node.tight) {
// The default `false` is not written to the AST
math.tight = data.node.tight;
}
return [math];
},
};
23 changes: 16 additions & 7 deletions packages/myst-parser/src/fromMarkdown.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,9 +28,11 @@ export function withoutTrailingNewline(str: string) {
* Loosely based on prosemirror-markdown
*/
export class MarkdownParseState {
src: string;
stack: GenericNode[];
handlers: Record<string, TokenHandler>;
constructor(handlers: Record<string, TokenHandlerSpec>) {
constructor(src: string, handlers: Record<string, TokenHandlerSpec>) {
this.src = src;
this.stack = [u('root', [] as GenericParent[])];
this.handlers = getTokenHandlers(handlers);
}
Expand Down Expand Up @@ -77,8 +79,9 @@ export class MarkdownParseState {
tokens?.forEach((token, index) => {
if (token.hidden && !UNHIDDEN_TOKENS.has(token.type)) return;
const handler = this.handlers[token.type];
if (!handler)
if (!handler) {
throw new Error(`Token type ${token.type} not supported by tokensToMyst parser`);
}
handler(this, token, tokens, index);
});
}
Expand Down Expand Up @@ -111,8 +114,14 @@ type TokenHandler = (
index: number,
) => void;

function getAttrs(spec: TokenHandlerSpec, token: Token, tokens: Token[], index: number) {
const attrs = spec.getAttrs?.(token, tokens, index) || spec.attrs || {};
function getAttrs(
state: MarkdownParseState,
spec: TokenHandlerSpec,
token: Token,
tokens: Token[],
index: number,
) {
const attrs = spec.getAttrs?.(token, tokens, index, state) || spec.attrs || {};
if ('type' in attrs) throw new Error('You can not have "type" as attrs.');
if ('children' in attrs) throw new Error('You can not have "children" as attrs.');
return attrs;
Expand All @@ -134,17 +143,17 @@ function getTokenHandlers(specHandlers: Record<string, TokenHandlerSpec>) {
withoutTrailingNewline(tok.content),
tok,
spec.type,
getAttrs(spec, tok, tokens, i),
getAttrs(state, spec, tok, tokens, i),
);
return;
}
state.openNode(nodeType, tok, getAttrs(spec, tok, tokens, i), spec.isLeaf);
state.openNode(nodeType, tok, getAttrs(state, spec, tok, tokens, i), spec.isLeaf);
state.addText(withoutTrailingNewline(tok.content), tok);
state.closeNode();
};
} else {
handlers[type + '_open'] = (state, tok, tokens, i) =>
state.openNode(nodeType, tok, getAttrs(spec, tok, tokens, i));
state.openNode(nodeType, tok, getAttrs(state, spec, tok, tokens, i));
handlers[type + '_close'] = (state) => state.closeNode();
}
});
Expand Down
2 changes: 1 addition & 1 deletion packages/myst-parser/src/myst.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,7 @@ export function mystParse(content: string, opts?: Options) {
const { vfile } = opts || {};
const parsedOpts = parseOptions(opts);
const tokenizer = createTokenizer(parsedOpts);
const tree = tokensToMyst(tokenizer.parse(content, { vfile }), parsedOpts.mdast);
const tree = tokensToMyst(content, tokenizer.parse(content, { vfile }), parsedOpts.mdast);
applyDirectives(tree, parsedOpts.directives, parsedOpts.vfile);
applyRoles(tree, parsedOpts.roles, parsedOpts.vfile);
return tree;
Expand Down
34 changes: 29 additions & 5 deletions packages/myst-parser/src/tokensToMyst.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,22 @@ import { u } from 'unist-builder';
import { MarkdownParseState, withoutTrailingNewline } from './fromMarkdown.js';
import type { MdastOptions, TokenHandlerSpec } from './types.js';

export function computeAmsmathTightness(
src: string,
map: [number, number] | null | undefined,
): boolean | 'before' | 'after' {
const lines = src.split('\n');
const tightBefore =
typeof map?.[0] === 'number' && map[0] > 0 ? lines[map[0] - 1].trim() !== '' : false;
// Note: The end line might be different/wrong for AMS math. If that is updated, remove the `+1` that shifts the index
const last = typeof map?.[1] === 'number' ? map?.[1] + 1 : undefined;
const tightAfter =
typeof last === 'number' && last < lines.length ? lines[last]?.trim() !== '' : false;
const tight =
tightBefore && tightAfter ? true : tightBefore ? 'before' : tightAfter ? 'after' : false;
return tight;
}

const NUMBERED_CLASS = /^numbered$/;
const ALIGN_CLASS = /(?:(?:align-)|^)(left|right|center)/;

Expand Down Expand Up @@ -298,10 +314,13 @@ const defaultMdast: Record<string, TokenHandlerSpec> = {
type: 'math',
noCloseToken: true,
isText: true,
getAttrs(t) {
return {
getAttrs(t, tokens, index, state) {
const tight = computeAmsmathTightness(state.src, t.map);
const attrs = {
enumerated: t.meta?.enumerated,
};
} as Record<string, any>;
if (tight) attrs.tight = tight;
return attrs;
},
},
footnote_ref: {
Expand Down Expand Up @@ -360,6 +379,7 @@ const defaultMdast: Record<string, TokenHandlerSpec> = {
args: t.meta?.arg,
options: t.meta?.options,
value: t.content || undefined,
tight: t.meta?.tight || undefined,
};
},
},
Expand Down Expand Up @@ -489,13 +509,17 @@ const defaultOptions: MdastOptions = {
nestBlocks: true,
};

export function tokensToMyst(tokens: Token[], options = defaultOptions): GenericParent {
export function tokensToMyst(
src: string,
tokens: Token[],
options = defaultOptions,
): GenericParent {
const opts = {
...defaultOptions,
...options,
handlers: { ...defaultOptions.handlers, ...options?.handlers },
};
const state = new MarkdownParseState(opts.handlers);
const state = new MarkdownParseState(src, opts.handlers);
state.parseTokens(tokens);
let tree: GenericParent;
do {
Expand Down
8 changes: 7 additions & 1 deletion packages/myst-parser/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import type Token from 'markdown-it/lib/token.js';
import type { DirectiveSpec, RoleSpec } from 'myst-common';
import type { VFile } from 'vfile';
import type { MathExtensionOptions } from './plugins.js';
import type { MarkdownParseState } from './fromMarkdown.js';

export type MdastOptions = {
handlers?: Record<string, TokenHandlerSpec>;
Expand All @@ -12,7 +13,12 @@ export type MdastOptions = {

export type TokenHandlerSpec = {
type: string;
getAttrs?: (token: Token, tokens: Token[], index: number) => Record<string, any>;
getAttrs?: (
token: Token,
tokens: Token[],
index: number,
state: MarkdownParseState,
) => Record<string, any>;
attrs?: Record<string, any>;
noCloseToken?: boolean;
isText?: boolean;
Expand Down
12 changes: 11 additions & 1 deletion packages/myst-parser/tests/directives/directives.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { VFile } from 'vfile';

type TestCase = {
title: string;
skip?: boolean;
markdown: string;
mdast: Record<string, any>;
warnings?: number;
Expand All @@ -26,9 +27,18 @@ const casesList: TestCases[] = fs
return yaml.load(content) as TestCases;
});

const only = ''; // Can set this to a test title

casesList.forEach(({ title, cases }) => {
const casesToUse = cases.filter((c) => (!only && !c.skip) || (only && c.title === only));
const skippedCases = cases.filter((c) => c.skip || (only && c.title !== only));
if (casesToUse.length === 0) return;
describe(title, () => {
test.each(cases.map((c): [string, TestCase] => [c.title, c]))(
if (skippedCases.length > 0) {
// Log to test output for visibility
test.skip.each(skippedCases.map((c): [string, TestCase] => [c.title, c]))('%s', () => {});
}
test.each(casesToUse.map((c): [string, TestCase] => [c.title, c]))(
'%s',
(_, { markdown, mdast, warnings = 0 }) => {
const vfile = new VFile();
Expand Down
31 changes: 31 additions & 0 deletions packages/myst-parser/tests/directives/math.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,41 @@ cases:
name: math
options:
label: addition
value: 1+2
children:
- type: math
identifier: addition
label: addition
value: 1+2
- title: tight math
markdown: |-
Tight above
```{math}
:label: addition
1+2
```
But not below
mdast:
type: root
children:
- type: paragraph
children:
- type: text
value: Tight above
- type: mystDirective
name: math
tight: before
options:
label: addition
value: 1+2
children:
- type: math
identifier: addition
label: addition
tight: before
value: 1+2
- type: paragraph
children:
- type: text
value: But not below
Loading

0 comments on commit 2010854

Please sign in to comment.