Add serve --one-page option to live preview a single page #475
Conversation
index.js
Outdated
| if (options.onePage) { | ||
| // replace slashes for paths on Windows | ||
| // eslint-disable-next-line no-param-reassign | ||
| options.onePage = options.onePage.replace('\\', '/'); |
There was a problem hiding this comment.
On Windows nodejs supports both / and \. Is there any particular reason why this was done?
There was a problem hiding this comment.
On PowerShell and cmd.exe, paths are normally written with \. However, we internally normalise all paths to use / as the separator (including when storing the value of pages.src in siteData.json). Since the value passed to --one-path is directly used to look up which page to serve, I believe it would be better to allow users to enter \ as the path separator when specifying the file path to --one-page, which would be less confusing than if an error was emitted when \ is used.
There was a problem hiding this comment.
Ok, please use path.normalize(p) during file comparison then. We don't use the regex technique in our codebase, instead we use path.normalize().
There was a problem hiding this comment.
Replacing options.onePage.replace('\\', '/') with path.normalize() actually breaks the lookup. This is because the default site.json uses paths with /. While we use path. functions for filesystem operations, page.src is still written with / into siteData.json.
path.normalize() on a Windows system converts all path seperators to \, which breaks the lookup since pages are located by page.src, which needs / to match.
There was a problem hiding this comment.
How about path.posix.normalize?
There was a problem hiding this comment.
@acjh thank you for the suggestion! Unfortunately, path.posix.normalize on Windows-style paths does nothing:
> path.posix.normalize('userGuide\\userQuickStart.md')
'userGuide\\userQuickStart.md'This is because, according to the Node.js docs for path.normalize, only / is considered a path separator under POSIX, and thus \ will not be replaced.
(The phrasing is clearer in the original PR for this section nodejs/node#12700, but was subsequently changed.)
There was a problem hiding this comment.
Understood. Let's add a function in utils similar to ensure-posix-path's ensurePosix.
There was a problem hiding this comment.
@acjh Sure, that will work!
|
Good job tackling this feature request @Xenonym |
|
No problem @damithc! Initial rendering times of se-edu/se-book averaged over 5 runs: On average, there should be ~4x speedup during the initial render. |
Nice. Looking forward to using this feature. 👍 |
|
This feature is saving me a lot of time when updating a massive site such as the CS2103 website :-) 💯 |
|
Glad to hear it @damithc! :D |
What is the purpose of this pull request? (put "X" next to an item, remove the rest)
• [X] Enhancement to an existing feature
Resolves #441.
What is the rationale for this request?
Authors may want to preview only a single page in their website, that also includes changes from other parts of the website (eg. changes from an included page). We can allow them the option of just rendering that one page itself without having to render the entire website on each change, speeding up preview times.
What changes did you make? (Give an overview)
I added a
--one-page <file>option to the CLI that allows an author to specify a single page to preview.Site()andSite.js#generatePages()was then modified to only render that page when a page has been specified via--one-page. Finally,live-serverwas configured to open the specified page directly.Testing instructions:
--one-pageeg.markbind serve --one-page userGuide/components.md._site/userGuide/components.htmland its required files are rendered.components.md.