-
Notifications
You must be signed in to change notification settings - Fork 70
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
🔄 Support recursive includes in myst and tex #1082
Conversation
@@ -116,13 +113,6 @@ export async function loadFile( | |||
const { sha256, useCache } = checkCache(cache, content, file); | |||
if (useCache) break; | |||
const tex = new TexParser(content, vfile); | |||
await includeDirectiveTransform(tex.ast as GenericParent, vfile, { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Live reload was not working well for tex
include content. The fastProcessFile
function was getting triggered correctly for dependent files, but this transform was never run on the file with \input{...}
(useCache
was still true
).
Now, when this function is called in processMdast
it handles tex
as well, so live reload behaviour matches md processing, regardless of useCache
for file loading. (And calling the function here is unnecessary.)
*/ | ||
export const makeFileLoader = (session: ISession, baseFile: string) => (fullFile: string) => { | ||
session.store.dispatch( | ||
watch.actions.addLocalDependency({ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These include
dependencies do not show up in the page json "dependency" field, but they are respected for live site reloading.
I think that's ok - the json "dependencies" are used for loading data from other pages; in the case of include
, this data is actually moved to the page itself.
🤔 Maybe if we update include
to allow including partial snippets from md files we may need to load the rest of the data from that page...? (Similar to embed
pulling in a single figure.) But for now, it's unnecessary.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree, the included files are a local convinience, not for how the final documents work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code looks good, fun to see the implementation. Nice work @fwkoch.
This adds more robust support for "recursive includes" - these are where one file
include
s another file whichinclude
s a third file, etc.Additionally:
include
dependencies error nicely (including printing the "include
-stack") - no infinite loops:tex
source files with\input
macros resolve and reload recursively as wellResolves: