Skip to content
This repository has been archived by the owner on Mar 13, 2018. It is now read-only.

replace backslashes to support windows filesystem #217

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

mcarey1590
Copy link

No description provided.

@mcarey1590 mcarey1590 changed the title replace backslashes to support windows filesystem #215 replace backslashes to support windows filesystem Nov 17, 2015
@mbleigh
Copy link
Contributor

mbleigh commented Nov 17, 2015

I think the solution here should be to split on path.sep instead of replacing \\

@mcarey1590
Copy link
Author

we still have to replace the \ because we are replacing the base path with an empty string before splitting

@blasten
Copy link
Contributor

blasten commented Dec 1, 2015

👍 It should use path.sep https://github.com/jinder/path/blob/master/path.js#L414

@@ -103,7 +103,7 @@ function getGuideName (filepath) {
function formatParsedGuideFilepath (srcPath, destDir) {

var relativeSrcPath = srcPath
.replace(process.cwd() + path.sep, '')
.replace((process.cwd() + path.sep).replace(/\\/g, '/'), '')
.split('/')
Copy link
Contributor

Choose a reason for hiding this comment

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

split(path.sep)

@addyosmani
Copy link
Contributor

Friendly ping @mcarey1590 on switching to split(path.sep).

@mcarey1590
Copy link
Author

the issue I see with switching to split(path.sep) is that replacing cwd has to happen before the split, and the srcPath with always use forward slashes. On a windows machine the srcPath always had forward slashes where the cwd always used backslashes. Splitting srcPath on path.sep would not work on a windows environment.

@arthurevans
Copy link
Contributor

How about:

var relativeSrcPath = path.relative(process.cwd(), srcPath).split(path.sep);

I think path.relative will correctly normalize windows path names (i.e., will correctly recognize foo\\bar/blech as relative to foo/bar).

@mcarey1590
Copy link
Author

I tried Arthur's suggestion and window's path names were normalized correctly. This change has been added to pull request

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants