Update convert.py

[bpatterson971]

fix quoting in windows

[sampsyo]

Thanks for the contribution! This is clearly a bug.

What about filenames that contain single quotes? (This is why we're bothering with a full escape of the string.)

Maybe we'd be better off not using shell quoting or string templating at all—just using those $source and $dest tokens as replaceable components after a shlex.split and then invoking command_output with shell=False.

[bpatterson971]

Thanks for taking a look!

In Windows, filenames with single quotes would work as long as the double quotes are surrounding the entire thing. I was able to convert files containing a single quote using this code.

[redbmk]

Double quotes should work in *nix as well, right? What if you just used a regex to replace double quotes with \" and then wrap the whole thing in double quotes? I'm thinking that same code would work for both windows and *nix systems.

[redbmk]

Scratch that...using double quotes might cause variables to be interpreted in *nix systems without further escaping

[sampsyo]

Ah, perhaps I should have asked: what if the filename has a double quote in it? 😃

This is why shell escaping is a debacle best avoided if possible...

[mluds]

I got this to mostly work using subprocess.list2cmdline which escapes args on Windows. See here. I'm still having trouble with certain characters like - and ' though, but only in some cases, which is weird.

[sampsyo]

Weird! Thanks for looking into this.

I think the most straightforward (and most reliable) solution will be to pass the command to subprocess as a list, removing the need for any escaping/quoting on our part. This shouldn't be too hard:

1. Use shlex.split to break the template into components.
2. Loop over the components and format them to fill in the filenames.
3. Pass the resulting list as arguments.

Simple! Interested in tackling this, @mluds?

[mluds]

The only problem is with the --pretend flag where we need to display the command being executed as a string. I made a minimal fix in #1158 and I'd be happy to investigate further if there's any issues.

[sampsyo]

I'm not too worried about the --pretend output being perfectly copy-and-paste-able (this is already not true of the mv commands we emit). If it's OK with you, I'm going to take your fix in #1157.

[sampsyo]

Merged! Can you check whether I messed anything up, @mluds, or if this now works on Windows?

[mluds]

Oops, forgot to mention it didn't pass the Travis CI build! I'll work on that now.

[mluds]

Never mind, it looks like you got to it and it's working. Thanks!