Skip to content

Conversation

@gvalkov
Copy link

@gvalkov gvalkov commented Mar 23, 2016

Hello,

This PR adds the following API change to the run() function:

# Works with strings.
run('my command')

# Also works with lists.
run(['my', 'command'])

# All arguments are safely quoted.
run(['my', '$(ls /etc)', 'command with spaces'])  
# runs: "my '$(ls /etc)' 'command with spaces'"

This is of course, quite similar to the way subprocess.Popen and os.execv handle arguments by default. I think using a list is more convenient when running more complex commands, as you then don't have to worry about construction a safe string to pass to run.

Another possibility (that might be taking this a bit too far) is to make command a vararg. For example:

# These are equivalent.
run('my', 'command')  # This quotes arguments for us.
run('my command')

The six version bump was necessary for the six.moves.shlex_quote function, which is in different places in Py2 and Py3.

Kind regards,
Georgi

in the parameter list below.
:param str command: The shell command to execute.
:param str|list command:
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

str|list is what seems to fail CI - I have to figure out what syntax sphinx expects here.

@gvalkov gvalkov force-pushed the run-with-list-args branch from ebc35dc to d8db52f Compare March 24, 2016 20:42
@gvalkov
Copy link
Author

gvalkov commented Mar 24, 2016

Unfortunately, sphinx cannot resolve more than one type in a param or type directive. Since inv sites make sphinx-build treat warnings as errors, we end up with a broken build.

If you consider this patch for merging, I'll look into how this issue can be worked around (or fixed).

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.

1 participant