Skip to content

Fix the unexpected error that occurs when an empty control block is used.#387

Closed
cocolato wants to merge 2 commits into
sqlalchemy:mainfrom
cocolato:fix/control_line
Closed

Fix the unexpected error that occurs when an empty control block is used.#387
cocolato wants to merge 2 commits into
sqlalchemy:mainfrom
cocolato:fix/control_line

Conversation

@cocolato

@cocolato cocolato commented Feb 4, 2024

Copy link
Copy Markdown
Contributor

Fixes: #146

When the first control block is empty or comment, the correct result will now be rendered.

from mako.template import Template
template = r"""
% if False:
% elif False:
% else:
    test
% endif
"""

t = Template(template)
print(t.render())

Result:


    test

@zzzeek

zzzeek commented Feb 4, 2024

Copy link
Copy Markdown
Member

cool.

can we add changelog files to these patches? Dont worry about getting them perfect I can fix them up later

example

1d6c58e#diff-22252f506a8da7ab5343802f53f7c35770390f669fe0f73a9e2345f0914ebb98

@zzzeek zzzeek requested a review from sqla-tester February 4, 2024 14:25

@sqla-tester sqla-tester left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

OK, this is sqla-tester setting up my work on behalf of zzzeek to try to get revision d0a0981 of this pull request into gerrit so we can run tests and reviews and stuff

@sqla-tester

Copy link
Copy Markdown
Collaborator

New Gerrit review created for change d0a0981: https://gerrit.sqlalchemy.org/c/sqlalchemy/mako/+/5148

@cocolato

cocolato commented Feb 4, 2024

Copy link
Copy Markdown
Contributor Author

Ok, it has been updated.

@zzzeek zzzeek requested a review from sqla-tester February 5, 2024 17:12

@sqla-tester sqla-tester left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

OK, this is sqla-tester setting up my work on behalf of zzzeek to try to get revision 571a73a of this pull request into gerrit so we can run tests and reviews and stuff

@sqla-tester

Copy link
Copy Markdown
Collaborator

Patchset 571a73a added to existing Gerrit review https://gerrit.sqlalchemy.org/c/sqlalchemy/mako/+/5148

@zzzeek zzzeek left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

some changes. also inside the commit block, if you can modify this on push (use git commit --amend) , add the fixes line:

Fixes: #146
Closes: #387
Pull-request: https://github.com/sqlalchemy/mako/pull/387
Pull-request-sha: 571a73a1d84ee922b38ce5817a5822fbe0268cd0

Comment thread mako/codegen.py Outdated
return True
return False

if should_add_pass(node, children):

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

my comment didnt get transmitted from gerrit.

can we change this, so that we are using the same conditional from the old version, and just this last part with the "for c in children" is moved into a function def _search_for_control_line(). that way the form of the conditional stays the same and we move just that one check.

like this:

if not children or (
    all(
        isinstance(c, (parsetree.Comment, parsetree.ControlLine))
        for c in children
    )
    and all(
        (node.is_ternary(c.keyword) or c.isend)
        for c in children
        if isinstance(c, parsetree.ControlLine)
    )
) or _search_for_control_line(children):
    self.printer.writeline("pass")

Closes: sqlalchemy#387
Pull-request: sqlalchemy#387
Pull-request-sha: f816f1f
@cocolato

cocolato commented Feb 6, 2024

Copy link
Copy Markdown
Contributor Author

Thanks so much for your help! Everything's been updated now.

@zzzeek zzzeek requested a review from sqla-tester February 6, 2024 13:19

@sqla-tester sqla-tester left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

OK, this is sqla-tester setting up my work on behalf of zzzeek to try to get revision fb257ff of this pull request into gerrit so we can run tests and reviews and stuff

@sqla-tester

Copy link
Copy Markdown
Collaborator

Patchset fb257ff added to existing Gerrit review https://gerrit.sqlalchemy.org/c/sqlalchemy/mako/+/5148

@sqla-tester sqla-tester left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Michael Bayer (zzzeek) wrote:

code review left on gerrit

View this in Gerrit at https://gerrit.sqlalchemy.org/c/sqlalchemy/mako/+/5148

  • /COMMIT_MSG (line 34): Done
  • mako/codegen.py (line 862): Done

@sqla-tester

Copy link
Copy Markdown
Collaborator

Michael Bayer (zzzeek) wrote:

graet, thanks

View this in Gerrit at https://gerrit.sqlalchemy.org/c/sqlalchemy/mako/+/5148

@sqla-tester

Copy link
Copy Markdown
Collaborator

Gerrit review https://gerrit.sqlalchemy.org/c/sqlalchemy/mako/+/5148 has been merged. Congratulations! :)

artdogma added a commit to artdogma/pymako that referenced this pull request Aug 27, 2025
…sed.

Fixes: sqlalchemy/mako#146

When the first control block is empty or comment, the correct result will now be rendered.

```python
from mako.template import Template
template = r"""
% if False:
% elif False:
% else:
    test
% endif
"""

t = Template(template)
print(t.render())
```

Result:
```

    test

```

Closes: #387
Pull-request: sqlalchemy/mako#387
Pull-request-sha: fb257ff866148a79bfe58ccaf7c98f499f8d8424

Change-Id: Ief78cdf4eb39b6e8257332ed034dab052c715420
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants