For escapeShellArg, only wrap with double quotes.

[isaackehle]

It should be sufficient to wrap characters that would be escaped with double quotes.

ie: from command line, this works:

rsync -avzs --rsh="ssh -p 22 -i ~/.ssh/key.pem -o StrictHostKeyChecking=no" "user@host:/tmp/File Info for 'My User'.xlsx" "/tmp/tmpfile.xlsx"

and this change works for all of my cases.

[drgroot]

wouldn't this fail if the argument has a double quote?

[mattijs]

Do you have examples of cases that do not work for you? There has been a lot of going back and forth on the topic of shell escaping (#31, #23, #16, #13). It is important to support all allowed characters in filenames as well as shell expansion.

sidenote: Tests are failing for this PR.

[isaackehle]

Sorry it's been so long, I didn't see the comments and failed tests until now.

The problem I was running into with the escapeShellArg() function as it was written was the arguments were being double-escaped. So the rsync command would fail, since you would end up pulling something like this:

rsync -avzs --rsh="ssh -p 22 -i ~/.ssh/key.pem -o StrictHostKeyChecking=no" "user@host:/tmp/File Info for ''My User''.xlsx" "/tmp/tmpfile.xlsx"

Which is why I was saying it wasn't necessary to escape in this function. Perhaps there is a better solution. On the surface, it doesn't seem like there was any other movement on this?

[mattijs]

The output you got from the library doesn't look like one that would work.

I created the start of a test suite for shell output but didn't put any more effort into it. I'll try to pick it up and add your test case. Noted in #37