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

%p does not escape single quotes properly #32

Closed
Smile4ever opened this Issue Oct 23, 2016 · 4 comments

Comments

Projects
None yet
2 participants
@Smile4ever
Contributor

Smile4ever commented Oct 23, 2016

I have a file name:
Swedish House Mafia - Don't You Worry Child ft. John Martin.mp4

I have configured a custom cleanup action, named "Open file in default application". The command for this is:
xdg-open %p

When I click this cleanup action from the context menu on the file, I get a new window with the cleanup output:

cd /data/Google Drive/Media/Divers
xdg-open '/data/Google Drive/Media/Divers/Swedish House Mafia - Don't You Worry Child ft. John Martin.mp4'
/bin/bash: -c: line 0: unexpected EOF while looking for matching `''
/bin/bash: -c: line 1: syntax error: unexpected end of file
Process finished.

This window does not show if the cleanup action runs succesfully. The cleanup action works with files that do not contain single quotes in their filename.

I'm using MATE and this is detected by QDirStat, which falls back to the default terminal emulator xterm.

@shundhammer

This comment has been minimized.

Show comment
Hide comment
@shundhammer

shundhammer Oct 23, 2016

Owner

QDirStat had always escaped such single quotes, but not the way common shells (Bash, Zsh) expect it: They don't want a backslash in front of that embedded single quote. Rather, you need to terminate the string with a single quote, start a new one with double quotes only containing the embedded single quote, and then re-open the old string.

Thus, 'Don't do this' becomes 'Don'"'"'t do this'.

Yikes. This is so utterly broken by design I can't find proper words for it. I just wonder how much other software is out there that does it the way most people would expect it: Trying to use a backslash to escape that embedded single quote ('Don\'t do this').

Of course, such file names should be avoided entirely, but you can't help some slightly broken MP3 ripper program doing it, so it needs to be handled correctly.

Fixed with c8da4e8.

Owner

shundhammer commented Oct 23, 2016

QDirStat had always escaped such single quotes, but not the way common shells (Bash, Zsh) expect it: They don't want a backslash in front of that embedded single quote. Rather, you need to terminate the string with a single quote, start a new one with double quotes only containing the embedded single quote, and then re-open the old string.

Thus, 'Don't do this' becomes 'Don'"'"'t do this'.

Yikes. This is so utterly broken by design I can't find proper words for it. I just wonder how much other software is out there that does it the way most people would expect it: Trying to use a backslash to escape that embedded single quote ('Don\'t do this').

Of course, such file names should be avoided entirely, but you can't help some slightly broken MP3 ripper program doing it, so it needs to be handled correctly.

Fixed with c8da4e8.

@shundhammer

This comment has been minimized.

Show comment
Hide comment
@shundhammer
Owner

shundhammer commented Oct 23, 2016

@shundhammer

This comment has been minimized.

Show comment
Hide comment
@shundhammer

shundhammer Oct 23, 2016

Owner

In that Stack Overflow thread there was another, better readable solution (in particular better readable in the cleanup output window and in the log):

'Don't do this' -> 'Don'\''t do this'

Implemented with b08df9a.

Owner

shundhammer commented Oct 23, 2016

In that Stack Overflow thread there was another, better readable solution (in particular better readable in the cleanup output window and in the log):

'Don't do this' -> 'Don'\''t do this'

Implemented with b08df9a.

@Smile4ever

This comment has been minimized.

Show comment
Hide comment
@Smile4ever

Smile4ever Oct 23, 2016

Contributor

I can confirm the fix works, thank you.

Contributor

Smile4ever commented Oct 23, 2016

I can confirm the fix works, thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment