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

📒 Track notebook source for embedded content #334

Merged
merged 4 commits into from
Mar 29, 2023
Merged

Conversation

fwkoch
Copy link
Collaborator

@fwkoch fwkoch commented Mar 29, 2023

figure/embed directives may embed code/output cells from other notebooks. To recompute these, we need to know where they come from.

This PR adds url source and sourceKey to all embed nodes to locate the source notebook cell. This also adds dependencies at the top level of the page config with all url sources required to compute embedded content in the page.

@@ -179,7 +179,7 @@ export async function writeFile(
const toc = tic();
const selectedFile = selectFile(session, file);
if (!selectedFile) return;
const { frontmatter, ...mdastPost } = selectedFile;
const { frontmatter, mdast, kind, sha256, slug, references, dependencies } = selectedFile;
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 is just removing file from the written page config. I think this is ok? Previously this was writing local file paths... not useful at all for the site. (The original file is still available as a download in the export lists.)

Copy link
Collaborator

Choose a reason for hiding this comment

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

My thought on file is that it will be helpful when we sync up with an 'Edit on GitHub' which will require a local file path.

Copy link
Contributor

Choose a reason for hiding this comment

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

ok so sounds like this is one mini change to revert!

Copy link
Collaborator

Choose a reason for hiding this comment

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

In talking through with @fwkoch I think we should actually remove file, it is non-relative at the moment. We can bring this back soon with a feature of edit on github!

@rowanc1
Copy link
Collaborator

rowanc1 commented Mar 29, 2023

Code changes look good. I think I would prefer to keep file in the exported config, but we can also put this when we need it if you feel strongly.

Another note:
We also need to traverse the URLs and remap them in the theme CDN code. I think that code might actually be better in common.

@rowanc1 rowanc1 merged commit 8a1f168 into main Mar 29, 2023
@rowanc1 rowanc1 deleted the feat/embed-source branch March 29, 2023 21:33
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.

3 participants