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

Using shlex.join() when logging a command #490

Merged
merged 3 commits into from
Oct 2, 2021
Merged

Conversation

dhermes
Copy link
Collaborator

@dhermes dhermes commented Oct 2, 2021

No description provided.

nox/command.py Outdated
@@ -84,7 +85,7 @@ def run(
success_codes = [0]

cmd, args = args[0], args[1:]
full_cmd = f"{cmd} {' '.join(args)}"
full_cmd = f"{cmd} {shlex.join(args)}"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Aw man; added in 3.8

Copy link
Collaborator Author

@dhermes dhermes Oct 2, 2021

Choose a reason for hiding this comment

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

Luckily the implementation is dead simple:

def join(split_command):
    """Return a shell-escaped string from *split_command*."""
    return ' '.join(quote(arg) for arg in split_command)

and

$ python3.6 -c 'import shlex; print(shlex.quote)'
<function quote at 0x7f9c9e123d08>
$ python3.7 -c 'import shlex; print(shlex.quote)'
<function quote at 0x1023fd710>

nox/command.py Outdated
@@ -67,6 +68,11 @@ def _clean_env(env: Optional[dict]) -> Optional[dict]:
return clean_env


def _shlex_join(args: Sequence[str]) -> str:
# shlex.join() was added in Python 3.8
return " ".join(shlex.quote(arg) for arg in split_command)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Whoops, copy-pasta life

@@ -84,7 +90,7 @@ def run(
success_codes = [0]

cmd, args = args[0], args[1:]
full_cmd = f"{cmd} {' '.join(args)}"
full_cmd = f"{cmd} {_shlex_join(args)}"
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It occurs to me I should probably do a docs search for python -m pip and see what should be updated.

(As reviewers may already see from the branch name, I made this PR fully from the browser.)

@theacodes theacodes merged commit d95c57c into main Oct 2, 2021
@theacodes theacodes deleted the dhermes-patch-1 branch October 2, 2021 18:12
@theacodes
Copy link
Collaborator

Thanks @dhermes!

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

Successfully merging this pull request may close these issues.

3 participants