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(docz-core): change filepath entry for windows #31

Merged
merged 2 commits into from Jun 16, 2018

Conversation

marceloavf
Copy link
Contributor

@marceloavf marceloavf commented Jun 13, 2018

Description

Change Windows filepath imports

Review

  • Check if continues working on mac/linux
  • Check if implementation is OK

Closes #12, Closes #30

@@ -114,6 +114,9 @@ export class Entries {
const entries = await Promise.all(files.filter(isEntry).map(createEntry))

for (const entry of entries) {
if (process.platform === "win32") {
entry.filepath = entry.filepath.split('\\').join('/')
Copy link

@albinekb albinekb Jun 13, 2018

Choose a reason for hiding this comment

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

I think you can use path.normalize https://nodejs.org/api/path.html#path_path_normalize_path

❯ node -p "require('path').win32.normalize('/Users/test')"
\Users\test

I use .win32. here to simulate windows

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I tried with path.normalize and even path.win32.normalize but it doesn't work, strange 😞

image

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Seems that the tpl.js compiler just somehow mess up with the final import file with the double slash "\" on windows

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, it's all fine even on the last phase of writing the import, but after writing the import file the slash disappear on windows and the Error appear.

@pedronauck
Copy link
Member

Thanks @marceloavf! You right in your pr, but I think that this need to be in the Entry class when define filepath of entry:

https://github.com/pedronauck/docz/blob/master/packages/docz-core/src/Entry.ts#L105-L108

@marceloavf
Copy link
Contributor Author

Changed it @pedronauck, thanks!

@pedronauck pedronauck merged commit 14bf0e2 into doczjs:master Jun 16, 2018
@marceloavf marceloavf deleted the bugfix/windows-path branch June 20, 2018 23:41
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

3 participants