gh-119670: Add 'always' keyword argument to `shlex.quote` by jb2170 · Pull Request #119674 · python/cpython

jb2170 · 2024-05-28T17:19:49Z

Addresses #119670

Issue: shlex.quote: Add 'always' keyword argument #119670

📚 Documentation preview 📚: https://cpython-previews--119674.org.readthedocs.build/

Removes early return for the special case of an empty string

vstinner

Thanks for your interesting change.

Please add tests to Lib/test/test_shlex.py. Also, you can copy the NEWS entry in Doc/whatsnew/3.14.rst, in a new "shlex" sub-section of Improved Modules.

The first assertTrue test checks all strings now start with a single quote, ie they have actually been escaped The second test of assertFalse checks that not all the strings have been escaped when always=False (the default value), since the first, third, and fifth string do not need escaping unless requested.

jb2170 · 2024-06-03T22:56:44Z

Tests added in b8ef5ce, and whatsnew entry added in bf7dce0 😄

vstinner

LGTM

picnixz

Personally, I'd prefer "force" rather than "always" but that's bike-shedding (using "force" is more common when you want to force a behaviour I think).

picnixz · 2024-09-08T12:54:35Z

+:func:`shlex.quote` now has an *always* keyword argument for forcing
+the escaping of the string passed to it


Suggested change

:func:`shlex.quote` now has an *always* keyword argument for forcing

the escaping of the string passed to it

Allow :func:`shlex.quote` to unconditionally escape its input

via the keyword-only argument *always*.

picnixz · 2024-09-08T12:56:32Z

+* Add keyword-only argument of *always* to :func:`shlex.quote` for forcing
+  the escaping of the string passed to it.


* Allow :func:`shlex.quote` to unconditionally escape its input via ``shlex.quote(string, always=True)``. (Contributed by YOUR_NAME_OR_GITHUB_NICKNAME in :gh:`119670`.)

picnixz · 2024-09-08T12:59:20Z

+   If the *always* keyword argument is set to ``True`` then the string *s*
+   will always be escaped, even in the absence of special characters. This is
+   nice for uniformity, for example when escaping a list of strings.


Suggested change

If the *always* keyword argument is set to ``True`` then the string *s*

will always be escaped, even in the absence of special characters. This is

nice for uniformity, for example when escaping a list of strings.

If *always* is ``True``, then the string *s* is

unconditionally escaped, even in the absence of

special characters. This is typically useful for

uniformly escaping a list of strings:

vstinner · 2024-09-09T09:10:29Z

Personally, I'd prefer "force" rather than "always"

Oh, I also like force=True. What do you think @jb2170?

jb2170 · 2024-09-10T21:32:49Z

@picnixz

Personally, I'd prefer "force" rather than "always" but that's bike-shedding (using "force" is more common when you want to force a behaviour I think).

I could be swayed; force seems like a conventional word within the programming world, eg 'rm -rf'. But that is then superseded by the following:

@vstinner

Oh, I also like force=True. What do you think @jb2170?

Changing the behaviour to always quote its input would be ideal in my eyes! It requires tweaking a couple of the tests in Lib/test/test_shlex.py but I will commit those.

At this point however I question "who would ever want quote(s, force=False)?". We could get rid of the force kwarg, returning to the function's existing mono-parameter signature, and reduce the function body down to just

def quote(s):
    return "'" + s.replace("'", "'\"'\"'") + "'"

This simplifies quote to the one-liner that it should be, and it does-what-it-says-on-the-tin. Oh and the re import can be removed, since that's the only place it's used! 😄

Perhaps my main issue is that `quote` doesn't always quote, its behaviour quite unpredictable from the function name alone, relying on a footgun regex.

If we were to go ahead with this simplification the only question is "is there anyone out there relying on basic strings not being quoted by quote?". I would guess no-one would care, paraphrasing the discussion on PEP 475's backwards compatibility "The authors of this (PEP) don’t think that such applications exist"

I'll push a commit, see what you think... 👀 😃

vstinner · 2024-09-11T09:02:08Z

I only asked about the parameter name force instead of always, not to enable it by default, sorry.

picnixz · 2025-08-22T08:31:56Z

Could you:

resolve the conflicts?
revert the unconditional quoting but use force instead of always?

vstinner · 2025-09-01T16:36:21Z

It seems like @jb2170 is inactive for one year. Sadly, I suggest closing the PR and the related issue.

jb2170 · 2025-09-01T18:09:59Z

I'm still here, I'll give it a second look 😅

vstinner · 2025-09-08T14:20:48Z

Ping @jb2170.

github-actions · 2026-04-16T12:22:00Z

This PR is stale because it has been open for 30 days with no activity.

jb2170 · 2026-04-20T23:09:04Z

I'll do a fresh rebase / branch / PR since this PR is very much behind main. Sorry for abandoning it a while!

It's coming along on my branch shlex-quote-force. I suggest force to be an opt-in keyword-only argument for shlex.quote. 🙂

I want to talk about this behavior of shlex.quote too as it caught my attention when I was editing the function. I think the returning of "''" for falsey non-str data isn't right. I've put some commits on another branch shlex-quote-typeerror. I'll have a sleep on it. Discussion thread open here

I'll check my work and open a PR for shlex-quote-force later today.

jb2170 · 2026-04-21T18:25:43Z

Superseded by #148846

jb2170 added 3 commits May 28, 2024 17:38

Simplify existing control flow logic of shlex.quote

5be56a0

Removes early return for the special case of an empty string

Add 'always' kwarg to shlex.quote

1305d90

Update shlex docs for shlex.quote, adding info about the always kwarg

9748cc1

bedevere-app Bot added the awaiting review label May 28, 2024

bedevere-app Bot mentioned this pull request May 28, 2024

shlex.quote: Add 'always' keyword argument #119670

Open

📜🤖 Added by blurb_it.

0048c87

vstinner reviewed Jun 3, 2024

View reviewed changes

Comment thread Doc/library/shlex.rst Outdated

Comment thread Lib/shlex.py Outdated

Comment thread Doc/library/shlex.rst Outdated

vstinner reviewed Jun 3, 2024

View reviewed changes

Comment thread Doc/library/shlex.rst Outdated

jb2170 added 5 commits June 3, 2024 23:15

Keep the empty string clearly a special case for shlex.quote

36be111

Make always a keyword-only argument for shlex.quote

9e9eff0

Update shlex docs explaining reasoning for quote(..., always=True)

5f74da5

Add shlex subsection to whatsnew for shlex.quote additions

bf7dce0

vstinner reviewed Jun 4, 2024

View reviewed changes

Comment thread Lib/test/test_shlex.py Outdated

Explicitly write the expected results of test_shlex.testQuoteAlways

ffdc665

vstinner approved these changes Jun 4, 2024

View reviewed changes

bedevere-app Bot added awaiting merge and removed awaiting review labels Jun 4, 2024

picnixz reviewed Sep 8, 2024

View reviewed changes

Simplify shlex.quote's behaviour to do what it says on the tin

1a7a5cf

github-actions Bot added the stale Stale PR or inactive for long period of time. label Apr 16, 2026

jb2170 closed this Apr 20, 2026

jb2170 deleted the fix-issue-119670 branch April 20, 2026 22:45

jb2170 restored the fix-issue-119670 branch April 20, 2026 22:45

jb2170 deleted the fix-issue-119670 branch April 20, 2026 22:49

jb2170 mentioned this pull request Apr 21, 2026

gh-119670: Add force keyword only argument to shlex.quote #148846

Open

		:func:`shlex.quote` now has an always keyword argument for forcing
		the escaping of the string passed to it

		* Add keyword-only argument of always to :func:`shlex.quote` for forcing
		the escaping of the string passed to it.

-   If the *always* keyword argument is set to ``True`` then the string *s*
-   will always be escaped, even in the absence of special characters. This is
-   nice for uniformity, for example when escaping a list of strings.
+   If *always* is ``True``, then the string *s* is
+   unconditionally escaped, even in the absence of
+   special characters. This is typically useful for
+   uniformly escaping a list of strings:

Uh oh!

Conversation

jb2170 commented May 28, 2024 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vstinner left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

jb2170 commented Jun 3, 2024

Uh oh!

Uh oh!

vstinner left a comment

Choose a reason for hiding this comment

Uh oh!

picnixz left a comment

Choose a reason for hiding this comment

Uh oh!

picnixz Sep 8, 2024

Choose a reason for hiding this comment

Uh oh!

picnixz Sep 8, 2024

Choose a reason for hiding this comment

Uh oh!

picnixz Sep 8, 2024

Choose a reason for hiding this comment

Uh oh!

vstinner commented Sep 9, 2024

Uh oh!

jb2170 commented Sep 10, 2024

Perhaps my main issue is that quote doesn't always quote, its behaviour quite unpredictable from the function name alone, relying on a footgun regex.

Uh oh!

vstinner commented Sep 11, 2024

Uh oh!

picnixz commented Aug 22, 2025

Uh oh!

vstinner commented Sep 1, 2025

Uh oh!

jb2170 commented Sep 1, 2025

Uh oh!

vstinner commented Sep 8, 2025

Uh oh!

github-actions Bot commented Apr 16, 2026

Uh oh!

jb2170 commented Apr 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jb2170 commented Apr 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jb2170 commented May 28, 2024 •

edited by github-actions Bot

Loading

Perhaps my main issue is that `quote` doesn't always quote, its behaviour quite unpredictable from the function name alone, relying on a footgun regex.

jb2170 commented Apr 20, 2026 •

edited

Loading