Skip to content
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.

Sign up for GitHub

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

fix percent escape not working when not at the beginning of the line #383

Closed
wants to merge 4 commits into from
Closed

fix percent escape not working when not at the beginning of the line #383

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
773b6bd
fix percent escape && add test
cocolato Jan 17, 2024
5d3e742
fix regex && add test
cocolato Jan 19, 2024
ab8e747
format code style
cocolato Jan 19, 2024
ff8ce2e
don't replace percent when it is not the first non-whitespace character
cocolato Jan 26, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion mako/lexer.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -357,10 +357,12 @@ def match_text(self):
r"""
(.*?) # anything, followed by:
(
(?<=\n)(?=[ \t]*(?=%|\#\#)) # an eval or line-based
((?<=\n)|^)(?=[ \t]*(?=%|\#\#)) # an eval or line-based
Copy link
Member

Choose a reason for hiding this comment

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

i dont think the |^ is needed

Copy link
Member

Choose a reason for hiding this comment

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

also we can remove the "%" from this line

Copy link
Contributor Author

@cocolato cocolato Jan 18, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Thanks for the review ! I think the '%' here cannot be removed. Doing so would cause the regex in match_text to fail to consume the newline character '\n' in cases such as \n %for. Removal would result in the match_control_line's regular expression failing to recognize these control keywords.

mako/mako/lexer.py

Lines 424 to 429 in dc66614

def match_control_line(self):
match = self.match(
r"(?<=^)[\t ]*(%(?!%)|##)[\t ]*((?:(?:\\\r?\n)|[^\r\n])*)"
r"(?:\r?\n|\Z)",
re.M,
)

# comment preceded by a
# consumed newline and whitespace
|
(?<!%)(?=%%+)
Copy link
Member

Choose a reason for hiding this comment

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

this can read:

                 (?<!%)(?=%%+)  # consume the first percent sign out of a group of percent signs

so the overall block looks like:

        match = self.match(
            r"""
                (.*?)         # anything, followed by:
                (
                 (?<=\n)(?=[ \t]*(?=\#\#)) # an eval or line-based
                                             # comment, preceded by a
                                             # consumed newline and whitespace
                 |
                 (?<!%)(?=%%+)  # consume the first percent sign out of a group of percent signs
                 |
                 (?=\${)      # an expression
                 |
                 (?=</?[%&])  # a substitution or block or call start or end
                              # - don't consume
                 |
                 (\\\r?\n)    # an escaped newline  - throw away
                 |
                 \Z           # end of string
                )""",
            re.X | re.S,
        )

|
(?=\${) # an expression
|
(?=</?[%&]) # a substitution or block or call start or end
Expand Down
23 changes: 22 additions & 1 deletion test/test_lexer.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -202,14 +202,35 @@ def test_percent_escape(self):
[
Text("""\n\n""", (1, 1)),
Text("""% some whatever.\n\n""", (3, 2)),
Text(" %% more some whatever\n", (5, 2)),
Text(" ", (5, 2)),
Text("% more some whatever\n", (5, 6)),
ControlLine("if", "if foo:", False, (6, 1)),
ControlLine("if", "endif", True, (7, 1)),
Text(" ", (8, 1)),
],
),
)

def test_percent_escape2(self):
template = """%% do something
%%% do something
if <some condition>:
%%%% do something
"""
node = Lexer(template).parse()
self._compare(
node,
TemplateNode(
{},
[
Text("% do something\n", (1, 2)),
Text("%% do something\nif <some condition>:\n", (2, 2)),
Text(" ", (4, 2)),
Text("%%% do something\n ", (4, 6)),
],
),
)

def test_old_multiline_comment(self):
template = """#*"""
node = Lexer(template).parse()
Expand Down
17 changes: 17 additions & 0 deletions test/test_template.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1667,3 +1667,20 @@ class FuturesTest(TemplateTest):
def test_future_import(self):
t = Template("${ x / y }", future_imports=["division"])
assert result_lines(t.render(x=12, y=5)) == ["2.4"]


class EscapeTest(TemplateTest):
def test_percent_escape(self):
t = Template(
"""%% do something
%%% do something
if <some condition>:
%%%% do something
"""
)
assert result_lines(t.render()) == [
"% do something",
"%% do something",
"if <some condition>:",
"%%% do something",
]