shlex punctuation_chars inconsistency

[tphh]

BPO	35168
Nosy	@rhettinger, @vsajip, @tirkarthi
PRs	bpo-35168: Make shlex.punctuation_chars read-only #11631 [3.7] bpo-35168: Make shlex.punctuation_chars read-only (GH-11631) #15926 [3.8] bpo-35168: Make shlex.punctuation_chars read-only (GH-11631) #15927

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2019-09-11.12:47:41.844>
created_at = <Date 2018-11-05.17:32:14.036>
labels = ['easy', '3.8', 'type-bug', '3.7', 'docs']
title = 'shlex punctuation_chars inconsistency'
updated_at = <Date 2019-09-11.12:47:41.843>
user = 'https://bugs.python.org/tphh'

bugs.python.org fields:

activity = <Date 2019-09-11.12:47:41.843>
actor = 'vinay.sajip'
assignee = 'docs@python'
closed = True
closed_date = <Date 2019-09-11.12:47:41.844>
closer = 'vinay.sajip'
components = ['Documentation']
creation = <Date 2018-11-05.17:32:14.036>
creator = 'tphh'
dependencies = []
files = []
hgrepos = []
issue_num = 35168
keywords = ['patch', 'patch', 'patch', 'patch', 'easy']
message_count = 9.0
messages = ['329310', '329312', '329315', '329324', '329361', '329423', '351820', '351841', '351842']
nosy_count = 5.0
nosy_names = ['rhettinger', 'vinay.sajip', 'docs@python', 'xtreak', 'tphh']
pr_nums = ['11631', '15926', '15927']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue35168'
versions = ['Python 3.7', 'Python 3.8']

[tphh]

The newly added shlex.punctuation_chars is special compared to the other public instance variables: It can ONLY be used when constructing a shlex instance, unlike other public instance variables, such as commenters, which can ONLY be set later.

>>> s = shlex.shlex('abc // def')
>>> s.commenters = '/'
>>> list(s)
['abc', '', '']

>>> s = shlex.shlex('abc // def', punctuation_chars = '/')
>>> list(s)
['abc', '//', 'def']

However, setting punctuation_chars later shows this rather useless error message:

>>> s = shlex.shlex('abc // def')
>>> s.punctuation_chars = '/'
>>> list(s)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/opt/python/3.7.1/lib/python3.7/shlex.py", line 295, in __next__
    token = self.get_token()
  File "/opt/python/3.7.1/lib/python3.7/shlex.py", line 105, in get_token
    raw = self.read_token()
  File "/opt/python/3.7.1/lib/python3.7/shlex.py", line 133, in read_token
    if self.punctuation_chars and self._pushback_chars:
AttributeError: 'shlex' object has no attribute '_pushback_chars'

[tirkarthi]

Thanks for the report. The code was added with c1f974c and self._pushback_chars is declared only when punctuation_chars is passed to shlex.shlex in the constructor as you have mentioned in https://github.com/python/cpython/blob/master/Lib/shlex.py#L59 . Adding Vinay for thoughts on the usage.

[vsajip]

I agree that it's inconsistent, but quite a bit of setting up is done when punctuation_chars is provided, as per the link in msg329312. One could convert the attribute to a property and have a setter that does the equivalent set up, but some of the setup is one-time (removing things from wordchars, etc.), and would require additional work to handle the case where the property is reassigned multiple times. I have no problem updating the documentation to indicate in a note that it must be provided in the constructor and not later, but apart from the fact that it's inconsistent, is there a use case for supporting setting it later? That would mean setting it up so that you could set it several times, unset it, etc.

[rhettinger]

It makes sense to me that information used in an expensive one-time setup should be specified in advance where other parameters that are more easily changed are specified downstream. The API reflects the a sensible way to use the tool. Making it to easy to change later increases the risk of misuse.

[tphh]

So a documentation update and a better run time error message which clarifies that shlex.punctuation_chars is read-only?

[vsajip]

a better run time error message which clarifies that shlex.punctuation_chars is read-only

That it can be set only via the __init__(), yes.

[vsajip]

New changeset 972cf5c by Vinay Sajip (Alex) in branch 'master':
bpo-35168: Make shlex.punctuation_chars read-only (bpo-11631)
972cf5c

[vsajip]

New changeset aca878e by Vinay Sajip in branch '3.7':
[3.7] bpo-35168: Make shlex.punctuation_chars read-only (GH-11631) (GH-15926)
aca878e

[vsajip]

New changeset 3b92ddb by Vinay Sajip in branch '3.8':
[3.8] bpo-35168: Make shlex.punctuation_chars read-only (GH-11631) (GH-15927)
3b92ddb