search

amoussard / sftp-deployment

Spend less time managing file transfers and more time coding. SFTP support for Atom.io to send and receive files directly in your server.
MIT License
76 stars 44 forks source link

Fix paths on Windows #18

Closed DSpeichert closed 9 years ago

DSpeichert commented 10 years ago

Fixes #17

coveralls commented 10 years ago

Coverage Status

Coverage increased (+5.3%) when pulling 59ad679072af75c0278d88340b2a303461c1d644 on DSpeichert:master into c62107a63fea7cbd7e63aef7466f2acfb4841f08 on amoussard:master.

DevScarabyte commented 10 years ago

looking at the commit it looks like you deleted libs/SFTP.js also thanks :)

DSpeichert commented 10 years ago

@heihachi Yes, I figured it was not used at all.

DevScarabyte commented 10 years ago

I see what you mean lol

buschtoens commented 10 years ago

Tested on Windows 7. Works.

johngeffe commented 10 years ago

These changes need to be made to the FtpConnection.js file also.

buschtoens commented 10 years ago

This will also fix #8, if the changes are also applied to the FTP js.

buschtoens commented 10 years ago

This will also fix #8, if the changes are also applied to the FTP js.

@amoussard Would you care to merge this? :)

johngeffe commented 10 years ago

This could be simplified by just adding the .replace to the end of the single line of code:

var destinationFile = path.join(self.config.getRemotePath(), file.getPath()).replace(/[\\*]/g,"/");

amoussard commented 10 years ago

Hi all, Sorry for the delay, I was in vacations... I will take a look and merge that ;)

amoussard commented 10 years ago

I have take a look at your PR. I have only one question, why this transformation is not done in FileManager, when the path is "registered" to be treated ?

DSpeichert commented 10 years ago

That is a good question. It was a hotfix, so it's probably not the most right solution. I will look into FileManager later today.

amoussard commented 10 years ago

Okay nice. It's not a problem, just tell me if you find (I don't know if I will have the time today). However, I have made a branch, you can check if the implementation seems correct to you : https://github.com/amoussard/sftp-deployment/tree/evol/fix-windows-path

DSpeichert commented 10 years ago

Looks good but doesn't destinationDirectory also require the transformation?

Spillmaker commented 10 years ago

Just adding a note to you guys that this fixed my problems when connecting to SFTP. But on FTP-connections i get an error:

Prohibited file name: \www\beta

And in my config it is: "remote_path": "/www/beta/"

buschtoens commented 10 years ago

@Probeus Yes, we already know. The fix was only applied to the SFTP connection, but to FTP. @DSpeichert will try to move it upstream into the FileManager.

johngeffe commented 10 years ago

the implementation at https://github.com/amoussard/sftp-deployment/blob/evol/fix-windows-path/lib/filesystem/FileManager.js has a code issue. remove the () before the = on line 104 ;-)