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

Fix Documentation build on Windows #51

Merged
merged 3 commits into from Sep 22, 2017
Merged

Conversation

hogpilot
Copy link
Contributor

Fixes #50 by modifying docs/build-typedoc.js to run on Windows.

For whatever reason, node child processes that use spawn must include .cmd in the command name, for example:

    const typedoc = spawn(
      "typedoc.cmd",
      ...
    )

I'm not sure what effect this will have on Unix.

Also, I found that using path.sep to parse the package name was failing because path.sep is '\\' on Windows, but the string it is parsing uses / instead.

@@ -13,7 +13,7 @@ const md = new MarkdownIt();
(function generateTypeDoc() {
return new Promise((resolve, reject) => {
const typedoc = spawn(
"typedoc",
"typedoc.cmd",
Copy link
Contributor

Choose a reason for hiding this comment

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

This will cause a failure on Mac OS and Linux since the command doesn't have .cmd in it. Looks like nodejs/node-v0.x-archive#2318 might be to blame. You could try something like cross-spawn and see if that works. It should be a drop in replacement.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

cross-spawn does work, I can change it to that. Another option would be to switch from spawn to exec. I'm pretty sure it will work on Windows, and can go that route if you prefer

@patrickarlt
Copy link
Contributor

Tested on my Windows and Mac, works! Thanks @hogpilot.

@patrickarlt patrickarlt merged commit 526bc38 into Esri:master Sep 22, 2017
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.

None yet

2 participants