bpo-38208: Simplify string.Template by using __init_subclass__(). #16256
Conversation
Lib/dataclasses.py
Outdated
| @@ -212,6 +208,9 @@ def __init__(self, type): | |||
| def __repr__(self): | |||
| return f'dataclasses.InitVar[{self.type.__name__}]' | |||
|
|
|||
| def __class_getitem__(cls, params): | |||
There was a problem hiding this comment.
These changes aren't related to the title of this PR. Please either split it out into a separate PR, or retitle this PR.
There was a problem hiding this comment.
Oh, yes. They are from other issue.
| @@ -89,6 +66,25 @@ class Template(metaclass=_TemplateMetaclass): | |||
| braceidpattern = None | |||
| flags = _re.IGNORECASE | |||
|
|
|||
| def __init_subclass__(cls): | |||
There was a problem hiding this comment.
This seems like a better approach, but if you're modernizing this then I have a few other suggestions.
Lib/string.py
Outdated
| if 'pattern' in cls.__dict__: | ||
| pattern = cls.pattern | ||
| else: | ||
| pattern = r""" |
There was a problem hiding this comment.
Perhaps this pattern ought to be put at module scope, just above the class definition? The verbose representation might be more readable without all that leading whitespace.
Lib/string.py
Outdated
| {(?P<braced>%(bid)s)} | # delimiter and a braced identifier | ||
| (?P<invalid>) # Other ill-formed delimiter exprs | ||
| ) | ||
| """ % { |
There was a problem hiding this comment.
Too bad we can't convert this to a .format() string without breaking backward compatibility.
There was a problem hiding this comment.
We can. We can even convert it to f-string. I tried, but returned to the old format.
There was a problem hiding this comment.
The only problem with .format() and f-string is that we nee to duplicate literal braces. And the template/f-string will look strange, because braces have special meaning in regexpes and in new format.
There was a problem hiding this comment.
With f-string it would look:
def __init_subclass__(cls):
super().__init_subclass__()
if 'pattern' in cls.__dict__:
pattern = cls.pattern
else:
delim = _re.escape(cls.delimiter)
id = cls.idpattern
bid = cls.braceidpattern or cls.idpattern
pattern = fr"""
{delim}(?:
(?P<escaped>{delim}) | # Escape sequence of two delimiters
(?P<named>{id}) | # delimiter and a Python identifier
{{(?P<braced>{bid})}} | # delimiter and a braced identifier
(?P<invalid>) # Other ill-formed delimiter exprs
)
"""
cls.pattern = _re.compile(pattern, cls.flags | _re.VERBOSE)
Lib/string.py
Outdated
| @@ -146,6 +142,7 @@ def convert(mo): | |||
| self.pattern) | |||
| return self.pattern.sub(convert, self.template) | |||
|
|
|||
| Template.__init_subclass__() # setup pattern | |||
There was a problem hiding this comment.
This is a bit of a head scratcher, but I think the comment could be improved to make it more clear. What's really going on is that this is required to initialize pattern when no subclass is created. Can you improve the clarity of the comment (and maybe move it above this line, and make it a full sentence)? If you need a suggestion, let me know.
There was a problem hiding this comment.
I would appreciate your help with a comment.
|
When you're done making the requested changes, leave the comment: |
|
@warsaw, what is your thought about using an f-string? |
https://bugs.python.org/issue38208