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.

Sign up for GitHub

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

📋 New lines in markdown lists are not written to typst #1777

Merged
merged 1 commit into from
Jan 14, 2025
Merged

📋 New lines in markdown lists are not written to typst #1777

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/three-moles-give.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"myst-to-typst": patch
---

New lines in lists are not written to typst
4 changes: 3 additions & 1 deletion packages/myst-to-typst/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
import type { VFile } from 'vfile';
import type { GenericNode } from 'myst-common';
import { fileError, fileWarn, toText, getMetadataTags } from 'myst-common';
import { captionHandler, containerHandler, getDefaultCaptionSupplement } from './container.js';

Check warning on line 6 in packages/myst-to-typst/src/index.ts

View workflow job for this annotation

GitHub Actions / lint 

'getDefaultCaptionSupplement' is defined but never used
import type {
Handler,
ITypstSerializer,
Expand Down Expand Up @@ -106,7 +106,9 @@

const handlers: Record<string, Handler> = {
text(node, state) {
state.text(node.value);
// We do not want markdown formatting to be carried over to typst
// As the meaning in lists, etc. is different
state.text(node.value.replaceAll('\n', ' '));
Copy link

Choose a reason for hiding this comment

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

Things like this would be better handled in myst AST --- the AST should be designed to correctly abstract over new line behavior (new paragraph or item where it should be, etc.) and remove all newlines otherwise (normalize the text), so that backends don't handle this at all.

Copy link
Member Author

Choose a reason for hiding this comment

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

This could certainly be done much further upstream, e.g. at markdown parse. I don't think there are any consequences of that. Other backends (latex/word/jats) are not effected by these new lines. It is possible that a markdown round-trip would be.

We don't currently have a node for a stylistic-only-in-the-source-newline. Are you suggesting we just remove it all together?

Copy link

Choose a reason for hiding this comment

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

It's a design decision to consider: to normalize the text to remove double spaces and new lines, since then in the backend you just pass the text to the renderer (typst in this case) and it will always work. You will not round-trip to the original markdown (I don't know if it is a problem). If you need to round-trip, then I think you need to keep the text exactly as-is (as you do now) and take care of it in the backend (or a separate pass), like you did above.

},
paragraph(node, state) {
const { identifier } = node;
Expand Down
28 changes: 26 additions & 2 deletions packages/myst-to-typst/tests/lists.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
title: myst-to-typst admonitions
title: myst-to-typst lists
cases:
- title: important admonition
- title: important list
mdast:
type: root
children:
Expand All @@ -25,3 +25,27 @@ cases:
- This is a list
+ My other, nested
+ bullet point list!
- title: List with new lines
mdast:
type: root
children:
- type: list
children:
- type: listItem
spread: true
children:
- type: text
value: |-
A list
with a new line
- type: listItem
spread: true
children:
- type: text
value: |
And math
- type: inlineMath
value: Ax=b
typst: |-
- A list with a new line
- And math $A x = b$
Loading