-
Notifications
You must be signed in to change notification settings - Fork 129
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
relative fetch and FileAttachments #193
Conversation
output.insertLeft(node.arguments[0].start + 3, "_file/"); | ||
const arg = node.arguments[0]; | ||
const value = relativeUrl(sourcePath, "/_file/" + join(dirname(sourcePath), getStringLiteralValue(arg))); | ||
output.replaceLeft(arg.start, arg.end, JSON.stringify(value)); |
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.
Note that this also fixes a bug where fetch rewrites didn't work for files in subdirectories:
If you had this in javascript/files.md:
fetch("../file.csv")
it would have been compiled to fetch(".._file//file.csv")
👎
(with this PR it becomes fetch("../_file/file.csv")
👍); tested in input/build/files/
@@ -330,7 +331,7 @@ function normalizePieceHtml(html: string, root: string, sourcePath: string, cont | |||
const path = getLocalPath(sourcePath, href); | |||
if (path) { | |||
context.files.push({name: href, mimeType: mime.getType(href)}); | |||
element.setAttribute("href", `/_file/${path}`); | |||
element.setAttribute("href", relativeUrl(sourcePath, `/_file/${path}`)); |
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.
rewriting <link href>
was not tested; now tested in input/build/files/
990fbe1
to
aeab77d
Compare
aeab77d
to
0e3c901
Compare
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.
Yay, thanks for finding bugs and adding tests!
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.
Excellent! Two minor style nits.
Addresses the _file part of #42.
I wonder if there is a cleaner way to pass the information about the relative path to
_file/subsection/
; maybe by adding it as an argument to define(…). Easy to change.(This would be much simpler if we just removed the specific /_file/ path and let files be in the normal directory where they have been created. But that would be for a different PR.)