Skip to content
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

Allow run_bash parameters to be specified separately #335

Open
mothslaw opened this issue Feb 2, 2021 · 0 comments
Open

Allow run_bash parameters to be specified separately #335

mothslaw opened this issue Feb 2, 2021 · 0 comments

Comments

@mothslaw
Copy link
Contributor

mothslaw commented Feb 2, 2021

Is your feature request related to a problem? Please describe.
A common problem/confusion in plugin code is remembering to (and remembering how to!) properly quote/escape command arguments when calling run_bash. This is because we're forcing the plugin to represent the entire command line as one single string.

For example, let's say I want to rename a remote file. I might be tempted to write this:

command = "mv {} {}".format(oldname, newname)
libs.run_bash(command, ...)

But that will fail if either filename has a space in it.

So, maybe this:

command = "mv \"{}\" \"{}\"".format(oldname, newname)
libs.run_bash(command, ...)

but that fails if the filename has interpolatable characters in it (such as dollar signs).

Similarly, this will fail if either filename has an apostrophe in it:

command = "mv '{}' '{}'".format(oldname, newname)
libs.run_bash(command, ...)

To get around this, some plugins have written their own function quote_for_bash. This function will take in a Python string representing the exact thing they want to pass as an argument, and will return a string that is quoted/escaped so that it is safe to pass as a single argument on a Bash command line:

command = "mv {} {}".format(quote_for_bash(oldname), quote_for_bash(newname))
libs.run_bash(command, ...)

Describe the solution you'd like
I'd suggest we do the same thing that Python's subprocess.run does. Namely, it allows for the user to provide a list of strings. Each string becomes its own separate argument:

command = ["mv", oldname, newname]
libs.run_bash(command, ...)

As with subprocess.run, we could of course continue to support use of one single string for the whole command line, as well as this list approach.

Describe alternatives you've considered
The quote_for_bash idea the plugins are using looks like this:

def quote_for_bash(raw_string):
  # Escape any single-quote characters in the raw string
  string_to_quote = raw_string.replace("'", "'\"'\"'")
  
  # Surround the string with single quotes
  return "'{}'".format(string_to_quote)

This is easy enough to write. What's hard is remembering to use it, and remembering when you have to use it and when you don't.

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

No branches or pull requests

1 participant