Skip to content

Conversation

@jcubic
Copy link
Member

@jcubic jcubic commented Apr 28, 2019

Simple fix for #13

@billiegoose
Copy link
Member

Darnit I wrote a whole fancy post but then I refreshed the page. :(

@billiegoose
Copy link
Member

Long story short, can we move this fix to here:

lightning-fs/src/path.js

Lines 18 to 31 in e47c9c1

function splitPath(path) {
if (path.length === 0) return [];
if (path === "/") return ["/"];
let parts = path.split("/");
if (path[0] === "/") {
// assert(parts[0] === '')
parts[0] = "/";
} else {
if (parts[0] !== ".") {
parts.unshift(".");
}
}
return parts;
}

and make it not silently "fix" double-slashes in the middle of paths like /here/is////a/path?

so probably somethign like:

if (parts[parts.length - 1] === '/') parts.pop()

@jcubic
Copy link
Member Author

jcubic commented May 1, 2019

Not sure how to do this since you can't just replace path.split(filepath) with splitPath(path).

@billiegoose
Copy link
Member

😆 No you can! path.split IS splitPath. See:

lightning-fs/src/path.js

Lines 85 to 91 in e47c9c1

module.exports = {
join: joinPath,
normalize: normalizePath,
split: splitPath,
basename,
dirname,
};

@jcubic
Copy link
Member Author

jcubic commented May 2, 2019

So filepath is just path separator? that name is misleading, I just though that this is some kind of other path like '/foo/bar'.split('/foo/'); I would then rename filepath parameter to separator.

@billiegoose
Copy link
Member

No. :( filepath is not the path separator. path is not a string, it's a module.

@billiegoose
Copy link
Member

image

image

@billiegoose
Copy link
Member

I try to never use path or url as variable names in my projects because they are such frequently used builtin modules, and you end up shadowing the module name.

@jcubic
Copy link
Member Author

jcubic commented May 2, 2019

Ok, now I understand I'll update the code to use the module then.

but after change it don't throw error. fs.stat('//foo//bar') is working the same as /foo/bar it seems that split is called twice, second time it get /foo/bar as argument. is that ok? Or should it throw Error that if found //.

@jcubic
Copy link
Member Author

jcubic commented May 2, 2019

Will check broken build tomorrow.

@jcubic
Copy link
Member Author

jcubic commented May 3, 2019

The build faling is not related to the code, tests are passing. The issue is with SauceLabs that give timeout errors.

@billiegoose
Copy link
Member

but after change it don't throw error.

It didn't throw an error for the //foo//bar case before though either. I guess I was mistaken LOL.

Thanks so much!! I can't wait to see what you're creating with lightning-fs :)

@billiegoose billiegoose merged commit eef9ad4 into master May 5, 2019
@billiegoose billiegoose deleted the trailing-slash branch May 5, 2019 13:32
@isomorphic-git-bot
Copy link
Member

🎉 This PR is included in version 3.2.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@jcubic
Copy link
Member Author

jcubic commented May 5, 2019

I use it together with isomorphic git, right that I've reused my old service worker to for server like access for file in FS. (the same I use it my git terminal) with this small fix the code is almost the same (only library switch) but it load faster.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants