Skip to content

Replace custom quote function with shlex.quote#107

Merged
WtfJoke merged 1 commit intortcTo:developfrom
pbhandari:fix/quoting-logic
Aug 10, 2016
Merged

Replace custom quote function with shlex.quote#107
WtfJoke merged 1 commit intortcTo:developfrom
pbhandari:fix/quoting-logic

Conversation

@pbhandari
Copy link
Contributor

The custom written shell.quote function was causing issues with it not
correctly escaping some characters. As Python 3.3+ comes with a quote
function in the shlex module, we use that in place of the custom
quote function.

Fixes issue #97.

The custom written `shell.quote` function was causing issues with it not
correctly escaping some characters. As Python 3.3+ comes with a `quote`
function in the `shlex` module, we use that in place of the custom
`quote` function.

Fixes issue #97.
@pbhandari
Copy link
Contributor Author

@WtfJoke: if you're still skeptical about using shlex.quote wholesale, I could just modify the shell.quote function to escape out the $

@WtfJoke WtfJoke self-assigned this Jul 18, 2016
@WtfJoke
Copy link
Member

WtfJoke commented Jul 18, 2016

Sorry about my late response. I'll have a look at it during the next weeks/days.
Quite busy lately.

But I think we can merge that without any problems.
Thanks for your contribution Prajjwal! :)

@WtfJoke WtfJoke merged commit 1947948 into rtcTo:develop Aug 10, 2016
@WtfJoke
Copy link
Member

WtfJoke commented Aug 10, 2016

Looks good, thank you :)

@dentych
Copy link

dentych commented Sep 23, 2016

I would like to say, that this shlex.quote thing completely messed up the script for me. I had to remove all shlex again.

It was executing commands like: scm login -r -u '"'"''"'"' -P '"'"''"'"'

I manually replaced all shlex, and put in " everywhere. That worked.

@WtfJoke
Copy link
Member

WtfJoke commented Sep 23, 2016

Ok, thank you for the info. I will revert this.

@WtfJoke
Copy link
Member

WtfJoke commented Oct 4, 2016

I would like to say, that this shlex.quote thing completely messed up the script for me. I had to remove all shlex again.

I reset the code to the point without merging this pull request

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants