Fixed exception in archive consumer.

[konstantin-popov]

The package throws exception on Windows when archive-view uses it in archive with nested files/directories.

Not sure if importing path is ok here or it should go somewhere in Atom-FS package for abstraction.

[Alhadis]

Hey there,

I'm afraid this isn't really fixing the underlying issue. You see, the way the filesystem API works is that it normalises path separators the first time a resource is referenced, freeing all other functions to assume forward slashes in paths are being used.

So the real problem is somewhere along the lines, separators aren't being normalised like they should be. Could I trouble you to email me the ZIP file (or something with an analogous structure)?

[konstantin-popov]

I found the problem and tested the fix using one of the test files in archive-viewer package, this tar file.

Is this what you need?

[konstantin-popov]

So the real problem is somewhere along the lines, separators aren't being normalised like they should be.

The source of backslashes is here (package node-ls-archive used by archive-viewer to list archives' contents). Surprisingly for me, it seems to replace all forward slashes with backslashes not only on Windows. Any idea why is it so?

[Alhadis]

Nope, no idea. But I'm looking into this to see what the real issue is.

I doubt the solution is as simple as using path.sep, however.

[Alhadis]

@konstantin-popov Never mind. I forgot what I wrote stitchPath for: and it's not even related to the filesystem API whatsoever.

[Alhadis]

Could you please use this instead?

const {sep} = require("path");

[Alhadis]

Thanks!

[konstantin-popov]

@Alhadis, after what I have seen in node-ls-archive I thought that this patch should be tested on Mac/Linux (system with a different path separator). Do you happen to use any of these?

[Alhadis]

Yes, don't worry. My normal system is running macOS; I tested your earlier fix with a VirtualBox install of Windows 7. Both platforms are functioning fine, it would seem. =)

Note that the stitchPaths method was added to smooth over an inconsistency from atom/archive-view#45. Basically, their DirectoryView objects lack the same path-properties which FileView instances are given. We needed both DirectoryView and FileView classes to both have an archive-relative path... :\

[Alhadis]

BTW, all of this ugly code will be washed away once/if the Atom team merges atom/archive-view#47. Because what we're currently doing now, in order to get icons attached to DOM elements, is apply one sneaky monkey-patch after another... :( You can read more on that topic in the PR.

[konstantin-popov]

Thanks for your explanation! I thought something like this when looking at punchClass method. 😃

[Alhadis]

Yeah, haha. =) In fact, the entire lib/consumers folder is full of such hacky, vile code: hopefully we're able to purge it all sooner rather than later. Everything's all up to the Atom team, though, and whether they want to accept my PRs (if they don't, uh, I'm not sure how much longer the consumers hacks will be hanging around for... 😢 )