Skip to content

bpo-25567: Add the support of bytes in quotes. #10871

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

Conversation

hobbestigrou
Copy link

@hobbestigrou hobbestigrou commented Dec 3, 2018

Add the support of bytes in quotes method.

Co-authored-by: Nan Wu nanbytesflow@gmail.com

https://bugs.python.org/issue25567

@the-knights-who-say-ni
Copy link

Hello, and thanks for your contribution!

I'm a bot set up to make sure that the project can legally accept your contribution by verifying you have signed the PSF contributor agreement (CLA).

Unfortunately we couldn't find an account corresponding to your GitHub username on bugs.python.org (b.p.o) to verify you have signed the CLA (this might be simply due to a missing "GitHub Name" entry in your b.p.o account settings). This is necessary for legal reasons before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue.

You can check yourself to see if the CLA has been received.

Thanks again for your contribution, we look forward to reviewing it!

cases where you cannot use a list.
Return a shell-escaped version of the string *s* or the bytes object. The
returned value is a string literal or bytes that can safely be used as one
token in a shell command line, for cases where you cannot use a list.

This idiom would be unsafe:
Copy link
Member

Choose a reason for hiding this comment

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

You should add ".. versionchanged:: 3.8" markup and describe the new feature.

Lib/shlex.py Outdated
@@ -306,17 +306,24 @@ def split(s, comments=False, posix=True):


_find_unsafe = re.compile(r'[^\w@%+=:,./-]', re.ASCII).search
_find_unsafe_bytes = re.compile(b'[^\w@%+=:,./-]', re.ASCII).search
Copy link
Member

Choose a reason for hiding this comment

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

re.ASCII is useless here, no?

Lib/shlex.py Outdated

# use single quotes, and put single quotes into double quotes
# the string $'b is then quoted as '$'"'"'b'
return "'" + s.replace("'", "'\"'\"'") + "'"
return b"'" + s.replace(b"'", b"'\"'\"'") + b"'" \
Copy link
Member

Choose a reason for hiding this comment

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

Move the code inside previous if/else.

@@ -0,0 +1 @@
Added the support of bytes in quote method from shlex module.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
Added the support of bytes in quote method from shlex module.
Add the bytes support to the :func:`shlex.quote` function.

Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

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

You might amend the commit message (and use git push --force) to use "Co-Authored-By:" instead of "Co-authored-by:". I don't know if tools checking for "Co-Authored-By:" are case sensitive?

Add the support of bytes in quotes method in shlex module.

Co-authored-by: Nan Wu <nanbytesflow@gmail.com>
@hobbestigrou hobbestigrou force-pushed the 25567-bpo_quotes_bytes_support branch from e82a4c0 to 448bde6 Compare December 4, 2018 15:40
@csabella
Copy link
Contributor

Closing and reopening to trigger CI builds.

@csabella csabella closed this May 22, 2019
@csabella csabella reopened this May 22, 2019
@@ -306,17 +306,23 @@ def split(s, comments=False, posix=True):


_find_unsafe = re.compile(r'[^\w@%+=:,./-]', re.ASCII).search
_find_unsafe_bytes = re.compile(b'[^\w@%+=:,./-]').search
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
_find_unsafe_bytes = re.compile(b'[^\w@%+=:,./-]').search
_find_unsafe_bytes = re.compile(br'[^\w@%+=:,./-]').search

This line generates a SyntaxWarning that causes test failure in CI where assert_python_ok asserts for no output in stderr that has this SyntaxWarning. Perhaps use a raw string ?

/Users/karthikeyansingaravelan/stuff/python/cpython/Lib/shlex.py:309: SyntaxWarning: invalid escape sequence \w
  _find_unsafe_bytes = re.compile(b'[^\w@%+=:,./-]').search

@csabella
Copy link
Contributor

@hobbestigrou, please address the code review comments. Thank you!

@csabella
Copy link
Contributor

Closing due to inactivity. This issue is now available for someone else to work on it. If this PR is used in a new PR, please remember to credit to original author. Thanks!

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

Successfully merging this pull request may close these issues.

6 participants