Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

download-selected-local not working on Windows #1069

Closed
1 task done
MikeWillis opened this issue Nov 29, 2017 · 5 comments
Closed
1 task done

download-selected-local not working on Windows #1069

MikeWillis opened this issue Nov 29, 2017 · 5 comments
Assignees
Labels

Comments

@MikeWillis
Copy link
Contributor

Prerequisites

Description

I believe there is a problem with /lib/menus/commands.js on line 394 (the download-selected-local function). I added:

console.log("remotePath = '" + remotePath + "'");

on my Windows machine on line 395 to debug, and the output is like so:

remotePath = '/example.com/\htdocs\path\to\file.txt'

My remote server is a linux box, and the download is failing. To make matters worse, no error is generated, in fact the system thinks that the download was successful, and it proceeds to bring up the atom notification claiming that all transfers succeeded.

Steps to Reproduce

  1. On a windows machine, connect to a site hosted on linux
  2. trigger the download-selected-local function

Expected behavior: [What you expect to happen]
the file is downloaded

Actual behavior: [What actually happens]
the file is not downloaded

Reproduces how often: [What percentage of the time does it reproduce?]
100%

Versions

Windows 10
Atom 1.22.1
Electron 1.6.15
Chrome 56.0.2924.87
Node 7.4.0
Remote FTP 2.0.0

Additional Information

I contributed the download-selected-local function a long time ago, but I had it set to modify the path like so:

// if this is windows, the path may have \ instead of / as directory separators
const remotePath = atom.project.remoteftp.root.remote + localPath.replace(/\\/g, '/');

I'm really not familiar with the Path.posix.normalize() function so I'm reading up on it now, and if I can come up with a fix I'll make a pull request. But hopefully someone else beats me to it, this can be a pretty serious issue if multiple people are sharing files and they think they downloaded a fresh copy of something when in fact they didn't

@MikeWillis
Copy link
Contributor Author

After reading up on Path.posix.normalize() it seems to me that this function will normalize the path according to the local operating system, which is why my Windows system ends up with \ as directory separator, which doesn't agree with my remote server's / directory separator

@icetee
Copy link
Owner

icetee commented Nov 29, 2017

I find it so good:

const remotePath = Path.posix.normalize(Path.join(atom.project.remoteftp.root.remote + localPath));

@MikeWillis
Copy link
Contributor Author

MikeWillis commented Nov 29, 2017

hmm on my machine that results in an error:

remotePath = '\example.com\htdocs\path\to\file.txt'
Uncaught Error: EEXIST: file already exists, mkdir 'C:\MyFolderStructure\example.com\pData\Local\atom\app-1.22.1'
    at fs.mkdirSync (fs.js:855)
    at Object.fs.mkdirSync (ELECTRON_ASAR.js:670)
    at sync (C:\Users\me\.atom\packages\Remote-FTP\node_modules\mkdirp\index.js:71)
    at sync (C:\Users\me\.atom\packages\Remote-FTP\node_modules\mkdirp\index.js:77)
    at sync (C:\Users\me\.atom\packages\Remote-FTP\node_modules\mkdirp\index.js:78)

edit: I left out a few lines from the error message, added them here

@icetee icetee self-assigned this Nov 29, 2017
@icetee icetee added the bug label Nov 29, 2017
@icetee
Copy link
Owner

icetee commented Nov 29, 2017

I also get such an error message. The folder or file does not exist on the server.

Maybe you can try to improve it in pull request?

@MikeWillis
Copy link
Contributor Author

Sure I'd be glad to, I switched my machine to just do a replace of all \ with /

I'll do some digging today to see if there's a better option and will put a pull request in with whatever I come up with

MikeWillis added a commit to MikeWillis/remote-ftp that referenced this issue Nov 30, 2017
icetee added a commit that referenced this issue Dec 2, 2017
Fix bug #1069 and fix download onceConnected call
@icetee icetee closed this as completed Dec 2, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants