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

🪄 Updates for multi-article typst export with interactive figures #1701

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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/fifty-cougars-invent.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'myst-to-typst': patch
---

Support additional greek characters in typst
7 changes: 7 additions & 0 deletions .changeset/kind-lies-kick.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'myst-directives': patch
'myst-transforms': patch
'myst-cli': patch
---

Add static figure placeholder for images that should be used only for PDF exports
5 changes: 5 additions & 0 deletions .changeset/large-countries-cheer.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'myst-cli': patch
---

Prevent html outputs that translate to empty images
5 changes: 5 additions & 0 deletions .changeset/nasty-pets-smell.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'myst-to-typst': patch
---

Support restarting counter for each typst article in multi-article export
5 changes: 5 additions & 0 deletions .changeset/new-birds-bake.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'myst-to-typst': patch
---

Fix typst crossreferences to other pages
5 changes: 5 additions & 0 deletions .changeset/rare-months-rest.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'myst-cli': patch
---

Prioritize project-level parts for project-level typst export
28 changes: 26 additions & 2 deletions packages/myst-cli/src/build/typst.ts
Original file line number Diff line number Diff line change
Expand Up @@ -267,6 +267,31 @@ export async function localArticleToTypstTemplated(
macros: [],
commands: {},
};
const state = session.store.getState();
const projectFrontmatter = selectors.selectLocalProjectConfig(state, projectPath ?? '.') ?? {};
if (file === selectors.selectCurrentProjectFile(state)) {
// If export is defined at the project level, prioritize project parts over page parts
partDefinitions.forEach((def) => {
const part = extractTypstPart(
session,
{ type: 'root', children: [] },
{},
def,
projectFrontmatter,
templateYml,
);
if (Array.isArray(part)) {
// This is the case if def.as_list is true
part.forEach((item) => {
collected = mergeTypstTemplateImports(collected, item);
});
parts[def.id] = part.map(({ value }) => value);
} else if (part != null) {
collected = mergeTypstTemplateImports(collected, part);
parts[def.id] = part?.value ?? '';
}
});
}
const hasGlossaries = false;
const results = await Promise.all(
content.map(async ({ mdast, frontmatter, references }, ind) => {
Expand Down Expand Up @@ -311,8 +336,7 @@ export async function localArticleToTypstTemplated(
frontmatter = content[0].frontmatter;
typstContent = results[0].value;
} else {
const state = session.store.getState();
frontmatter = selectors.selectLocalProjectConfig(state, projectPath ?? '.') ?? {};
frontmatter = projectFrontmatter;
const { dir, name, ext } = path.parse(output);
typstContent = '';
let fileInd = 0;
Expand Down
5 changes: 5 additions & 0 deletions packages/myst-cli/src/process/mdast.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,8 @@ import {
transformFilterOutputStreams,
transformLiftCodeBlocksInJupytext,
transformMystXRefs,
removeStaticImages,
removeNonStaticImages,
} from '../transforms/index.js';
import type { ImageExtensions } from '../utils/resolveExtension.js';
import { logMessagesFromVFile } from '../utils/logging.js';
Expand Down Expand Up @@ -379,10 +381,13 @@ export async function finalizeMdast(
const vfile = new VFile(); // Collect errors on this file
vfile.path = file;
if (simplifyFigures) {
removeNonStaticImages(mdast);
// Transform output nodes to images / text
reduceOutputs(session, mdast, file, imageWriteFolder, {
altOutputFolder: simplifyFigures ? undefined : imageAltOutputFolder,
});
} else {
removeStaticImages(mdast);
}
transformOutputsToFile(session, mdast, imageWriteFolder, {
altOutputFolder: simplifyFigures ? undefined : imageAltOutputFolder,
Expand Down
39 changes: 39 additions & 0 deletions packages/myst-cli/src/transforms/images.ts
Original file line number Diff line number Diff line change
Expand Up @@ -746,6 +746,45 @@ export function transformPlaceholderImages(
remove(mdast, '__remove__');
}

/**
* Remove all static images
*
* This should only be run for web builds
*/
export function removeStaticImages(mdast: GenericParent) {
selectAll('image', mdast)
.filter((image: GenericNode) => image.static)
.forEach((image: GenericNode) => {
image.type = '__remove__';
});
remove(mdast, '__remove__');
}

/**
* Remove all figure content except static/placeholder, where present
*
* This should only be run for static builds
*/
export function removeNonStaticImages(mdast: GenericParent) {
const containers = selectAll('container', mdast);
containers.forEach((container: GenericNode) => {
const hasStatic = !!container.children?.find((child) => child.static);
const hasPlaceholder = !!container.children?.find((child) => child.placeholder);
if (!hasStatic && !hasPlaceholder) return;
container.children = container.children?.filter((child) => {
if (['caption', 'legend'].includes(child.type)) {
// Always keep caption/legend
return true;
}
if (hasStatic) {
// Prioritize static image over placeholder
return !!child.static;
}
return !!child.placeholder;
});
});
}

/**
* Trim base64 values for urlSource when they have been replaced by image urls
*/
Expand Down
3 changes: 3 additions & 0 deletions packages/myst-cli/src/transforms/outputs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -279,6 +279,9 @@ export function reduceOutputs(
],
};
htmlTransform(htmlTree);
if ((selectAll('image', htmlTree) as GenericNode[]).find((htmlImage) => !htmlImage.url)) {
return undefined;
}
return htmlTree.children;
} else if (content_type.startsWith('image/')) {
const path = writeCachedOutputToFile(session, hash, cache.$outputs[hash], writeFolder, {
Expand Down
15 changes: 15 additions & 0 deletions packages/myst-directives/src/figure.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,10 @@ export const figureDirective: DirectiveSpec = {
type: String,
doc: 'A placeholder image when using a notebook cell as the figure contents. This will be shown in place of the Jupyter output until an execution environment is attached. It will also be used in static outputs, such as a PDF output.',
},
static: {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I suspect we do not want to call this static. @stevejpurves suggested hard-copy in #1157 . We could also do something like placeholder:pdf, but really, this applies to all "paper" exports: docx, tex, typst, and pdf. placeholder:paper? Not sure how confusing placeholder vs placeholder:pdf might be.

Copy link
Contributor

Choose a reason for hiding this comment

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

Can we block this PR until we resolve #1157 (comment)?

Copy link
Member

Choose a reason for hiding this comment

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

Lets break this into ~6 prs and get the ones in that are simple and make this one smaller.

type: String,
doc: 'An image only used in static outputs. This image will always take priority for static outputs, such as PDFs, and will never be used in dynamic outputs, such as web builds.',
},
'no-subfigures': {
type: Boolean,
doc: 'Disallow implicit subfigure creation from child nodes',
Expand Down Expand Up @@ -96,6 +100,17 @@ export const figureDirective: DirectiveSpec = {
align: data.options?.align as Image['align'],
});
}
if (data.options?.static) {
children.push({
type: 'image',
static: true,
url: data.options.static as string,
alt: data.options?.alt as string,
width: data.options?.width as string,
height: data.options?.height as string,
align: data.options?.align as Image['align'],
});
}
if (data.body) {
children.push(...(data.body as GenericNode[]));
}
Expand Down
5 changes: 5 additions & 0 deletions packages/myst-to-typst/src/container.ts
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,11 @@ export const containerHandler: Handler = (node, state) => {
return;
}

if (node.enumerator?.endsWith('.1')) {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This could be improved. It places strict assumptions on the enumerator in numbering. e.g. It works great for:

numbering:
  enumerator: 3.%s

But doesn't work at all for:

numbering:
  enumerator: (3-%s)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Possibly we need better translation between myst and typst enumerators.

state.write(`#set figure(numbering: "${node.enumerator}")\n`);
state.write(`#counter(figure.where(kind: "${kind}")).update(0)\n\n`);
}

if (nonCaptions && nonCaptions.length > 1) {
const allSubFigs =
nonCaptions.filter((item: GenericNode) => item.type === 'container').length ===
Expand Down
4 changes: 2 additions & 2 deletions packages/myst-to-typst/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -363,10 +363,10 @@ const handlers: Record<string, Handler> = {
legend: captionHandler,
captionNumber: () => undefined,
crossReference(node: CrossReference, state, parent) {
if (node.remote) {
if (node.remoteBaseUrl) {
// We don't want to handle remote references, treat them as links
const url =
(node.remoteBaseUrl ?? '') +
node.remoteBaseUrl +
(node.url === '/' ? '' : node.url ?? '') +
(node.html_id ? `#${node.html_id}` : '');
linkHandler({ ...node, url: url }, state);
Expand Down
4 changes: 4 additions & 0 deletions packages/myst-to-typst/src/math.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,10 @@ const math: Handler = (node, state) => {
const { identifier: label } = normalizeLabel(node.label) ?? {};
addMacrosToState(value, state);
state.ensureNewLine();
if (node.enumerator?.endsWith('.1')) {
state.write(`#set math.equation(numbering: "(${node.enumerator})")\n`);
state.write(`#counter(math.equation).update(0)\n\n`);
}
// Note: must have spaces $ math $ for the block!
state.write(`$ ${value} $${label ? ` <${label}>` : ''}\n\n`);
state.ensureNewLine(true);
Expand Down
7 changes: 5 additions & 2 deletions packages/myst-to-typst/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,8 @@ const textOnlyReplacements: Record<string, string> = {
'©': '#emoji.copyright ',
'®': '#emoji.reg ',
'™': '#emoji.tm ',
'<': '\\< ',
'>': '\\> ',
'<': '\\<',
Copy link
Collaborator Author

@fwkoch fwkoch Dec 18, 2024

Choose a reason for hiding this comment

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

Making this change did not feel good - I'm not sure why we were previously adding a space after these characters. However, for the examples I was looking at, the addition of this space looked very wrong, and simply removing it here fixed the output.

@rowanc1 perhaps you have some recollection of this...?

'>': '\\>',
' ': '~',
' ': '~',
// eslint-disable-next-line no-irregular-whitespace
Expand Down Expand Up @@ -105,16 +105,19 @@ const mathReplacements: Record<string, string> = {
'×': 'times',
Α: 'A',
α: 'alpha',
𝜶: 'alpha',
Β: 'B',
β: 'beta',
ß: 'beta',
𝜷: 'beta',
Γ: 'Gamma',
γ: 'gamma',
Δ: 'Delta',
'∆': 'Delta',
δ: 'delta',
Ε: 'E',
ε: 'epsilon',
𝝴: 'epsilon',
Ζ: 'Z',
ζ: 'zeta',
Η: 'H',
Expand Down
16 changes: 16 additions & 0 deletions packages/myst-transforms/src/containers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,10 @@ function isPlaceholder(node: GenericNode) {
return node.type === 'image' && node.placeholder;
}

function isStatic(node: GenericNode) {
return node.type === 'image' && node.static;
}

/** Nest node inside container */
function createSubfigure(node: GenericNode, parent: GenericParent): GenericParent {
const children = node.type === 'container' && node.children ? node.children : [node];
Expand Down Expand Up @@ -130,6 +134,7 @@ export function containerChildrenTransform(tree: GenericParent, vfile: VFile) {
hoistContentOutOfParagraphs(container);
let subfigures: GenericNode[] = [];
let placeholderImage: GenericNode | undefined;
let staticImage: GenericNode | undefined;
let caption: GenericNode | undefined;
let legend: GenericNode | undefined;
const otherNodes: GenericNode[] = [];
Expand Down Expand Up @@ -161,6 +166,15 @@ export function containerChildrenTransform(tree: GenericParent, vfile: VFile) {
} else {
placeholderImage = child;
}
} else if (isStatic(child)) {
if (staticImage) {
fileError(vfile, 'container has multiple static images', {
node: container,
ruleId: RuleId.containerChildrenValid,
});
} else {
staticImage = child;
}
} else if (SUBFIGURE_TYPES.includes(child.type)) {
subfigures.push(child);
} else {
Expand All @@ -186,6 +200,7 @@ export function containerChildrenTransform(tree: GenericParent, vfile: VFile) {
caption ? 'caption' : undefined,
legend ? 'legend' : undefined,
placeholderImage ? 'placeholder image' : undefined,
staticImage ? 'static image' : undefined,
]
.filter(Boolean)
.join(', ');
Expand All @@ -206,6 +221,7 @@ export function containerChildrenTransform(tree: GenericParent, vfile: VFile) {
}
const children: GenericNode[] = [...subfigures];
if (placeholderImage) children.push(placeholderImage);
if (staticImage) children.push(staticImage);
// Caption is above tables and below all other figures
if (container.kind === 'table') {
if (caption) children.unshift(caption);
Expand Down
Loading