isomorphic-git / lightning-fs

A lean and fast 'fs' for the browser
MIT License
477 stars 47 forks source link

fix trailing slash #14

Closed jcubic closed 5 years ago

jcubic commented 5 years ago

Simple fix for #13

billiegoose commented 5 years ago

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

billiegoose commented 5 years ago

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

https://github.com/isomorphic-git/lightning-fs/blob/e47c9c17c950169dd1ae78815b3becf61e670529/src/path.js#L18-L31

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 commented 5 years ago

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

billiegoose commented 5 years ago

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

https://github.com/isomorphic-git/lightning-fs/blob/e47c9c17c950169dd1ae78815b3becf61e670529/src/path.js#L85-L91

jcubic commented 5 years ago

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 commented 5 years ago

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

billiegoose commented 5 years ago

image

image

billiegoose commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

Will check broken build tomorrow.

jcubic commented 5 years ago

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

billiegoose commented 5 years ago

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 :)

isomorphic-git-bot commented 5 years ago

:tada: This PR is included in version 3.2.1 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

jcubic commented 5 years ago

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.