Note greediness of PEP 723 reference parser

[SnoopJ]

Issue Description

While preparing a PR for PEP 723 support in pip, I noticed that the reference parser defined by the PEP and listed in the PyPA docs will collate multiple adjacent /// TYPE blocks as a single match, even when separated by a comment line (the spec refers to it as a "content line"). This greedy collation is surprising and makes distinguishing error cases a little complicated, so I think it merits a warning in the docs if it is not possible to update the specification itself.

I believe this quirk is caused by the last + in the reference regex being greedy and matching all the way to the trailing /// instead of to the first available one. In my limited experimentation, replacing this quantifier with +? resolves the issue, producing the expected number of matches.

This shouldn't slip through anybody's code unnoticed, as the collation will produce invalid TOML (the interior /// is invalid syntax), but it is a surprising enough edge case that I thought to report it here.

click for code

import re

script_A = """
# /// script
# data (1)
# ///
#
# /// script
# data (2)
# ///
"""

script_B = """
# /// script
# data (1)
# ///

# /// script
# data (2)
# ///
"""

# These lines adapted from PEP 723's reference parser:
# https://peps.python.org/pep-0723/#reference-implementation

REGEX = r"(?m)^# /// (?P<type>[a-zA-Z0-9-]+)$\s(?P<content>(^#(| .*)$\s)+)^# ///$"
name = "script"
matches_A = list(
    filter(lambda m: m.group("type") == name, re.finditer(REGEX, script_A))
)
matches_B = list(
    filter(lambda m: m.group("type") == name, re.finditer(REGEX, script_B))
)

# output:
# 1
# 2
print(len(matches_A))
print(len(matches_B))

Code of Conduct

• I am aware that participants in this repository must follow the PSF Code of Conduct.

[SnoopJ]

@ofek your input here as the author of PEP 723 will be particularly valuable. Maybe this is fully-intended behavior, but I found it surprising.

[DavidCEllis]

My memory from the discussions on the PEP is that this is intended as described in the documentation to support having /// as a line in a multi-line string in the general case for these blocks.

In order to allow for such a thing in the future, the parser is greedy until the comment block ends. The first example in the spec illustrates this. Blank but commented lines have to be considered as part of the content in order to allow blank lines in formatting, as such the parser is greedy until the comment block finishes.

So for instance, this should all be interpreted as one some-toml block:

# /// some-toml
# 
# embedded-csharp = """
# /// <summary>
# /// text
# ///
# /// </summary>
# public class MyClass { }
# """
#
# other-value = 42
# ///

---

This greedy collation is surprising and makes distinguishing error cases a little complicated, so I think it merits a warning in the docs if it is not possible to update the specification itself.

I'll note that the regex not giving any indication that this is what may be causing the error is one reason I don't use the regex to parse these myself and have my own non-regex parser.

This parser stores a list of potential issues for missing blocks, incomplete blocks and new blocks opening without the previous one being fully closed. If a later stage of parsing fails (such as conversion to TOML) these are then available as potential explanations for the failure.

Your script_A for example would have this associated message:
MetadataWarning(line_number=6, message="New 'script' block encountered before block 'script' closed.")

[abravalheri]

If I am not mistaken, the "greediness" was an intentional requirement brought up by the community of implementers during the discussion of the PEP. It is intentionally designed to simplify parser implementations.