Skip to content

Support n filter in the page tag#299

Closed
gagern wants to merge 1 commit intosqlalchemy:masterfrom
gagern:n_page_filters
Closed

Support n filter in the page tag#299
gagern wants to merge 1 commit intosqlalchemy:masterfrom
gagern:n_page_filters

Conversation

@gagern
Copy link
Copy Markdown
Contributor

@gagern gagern commented Jul 4, 2019

In some situations, it is inconvenient to pass default_filters in the
Template constructor depending on the template in question. It might be
easier in such situations to express page filters in the template itself.
However, dropping the existing default_filters (either explicitly set or the
default of ["str"] resp. ["unicode"]) might break existing templates.

The code change here comes to the rescue in such situations. Existing
templates keep working as they are, but editors of templates get a tool to
replace the default filters for specific templates. They do take on the
responsibility of turning all encountered inputs into strings, lest they
fail along the lines of #272.

This change should be sufficiently backwards compatible to not cause any
concerns. Sure, technically a "n" filter at page tag level was treated as a
no-op so far. So theoretically existing templates could break. But there
was no incentive to have such an "n" filter at the page tag level, and the
expressed semantics of the "n" filter is to suppress default filters, so
semantically anyone relying on it being a no-op in that situation was using
unsupported hacks anyway.

In some situations, it is inconvenient to pass default_filters in the
Template constructor depending on the template in question.  It might be
easier in such situations to express page filters in the template itself.
However, dropping the existing default_filters (either explicitly set or the
default of ["str"] resp. ["unicode"]) might break existing templates.

The code change here comes to the rescue in such situations.  Existing
templates keep working as they are, but editors of templates get a tool to
replace the default filters for specific templates.  They do take on the
responsibility of turning all encountered inputs into strings, lest they
fail along the lines of sqlalchemy#272.

This change should be sufficiently backwards compatible to not cause any
concerns.  Sure, technically a "n" filter at page tag level was treated as a
no-op so far.  So theoretically existing templates could break.  But there
was no incentive to have such an "n" filter at the page tag level, and the
expressed semantics of the "n" filter is to suppress default filters, so
semantically anyone relying on it being a no-op in that situation was using
unsupported hacks anyway.
sqlalchemy-bot pushed a commit that referenced this pull request Jul 4, 2019
I found it difficult to locate what the <%page> tag is capable
of, iterate everything it is known to be useful for including
links in the intro paragraph for <%page>.

Change-Id: Ice4c92a10943829f33c423f015eb49baaacc5fdc
References: #299
@zzzeek
Copy link
Copy Markdown
Member

zzzeek commented Jul 4, 2019

this looks great and I am glad someone has come along (e.g. you) to put some life into the project. I just pushed a doc update to support this better.

@zzzeek zzzeek requested a review from sqla-tester July 4, 2019 13:37
Copy link
Copy Markdown
Collaborator

@sqla-tester sqla-tester left a comment

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 to try to get revision f8d5a22 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 f8d5a22: https://gerrit.sqlalchemy.org/#/c/sqlalchemy/mako/+/1342

@sqla-tester
Copy link
Copy Markdown
Collaborator

mike bayer (zzzeek) wrote:

Code-Review+2 Workflow+1

@sqla-tester
Copy link
Copy Markdown
Collaborator

Gerrit review https://gerrit.sqlalchemy.org/1342 has been merged. Congratulations! :)

@gagern
Copy link
Copy Markdown
Contributor Author

gagern commented Jul 20, 2019

Will you roll a new release including this feature?

I currently have no concrete plans for any other work on Mako in the near future, so from my perspective there is no incentive to wait for more additions before the next release.

@zzzeek
Copy link
Copy Markdown
Member

zzzeek commented Jul 20, 2019

OK I'll try to release this when I next am doing releases, it's the weekend which isa little tight for me

@zzzeek
Copy link
Copy Markdown
Member

zzzeek commented Jul 20, 2019

the release is done. thank you for your contributions!

artdogma added a commit to artdogma/pymako that referenced this pull request Aug 27, 2025
In some situations, it is inconvenient to pass default_filters in the
Template constructor depending on the template in question.  It might be
easier in such situations to express page filters in the template itself.
However, dropping the existing default_filters (either explicitly set or the
default of ["str"] resp. ["unicode"]) might break existing templates.

The code change here comes to the rescue in such situations.  Existing
templates keep working as they are, but editors of templates get a tool to
replace the default filters for specific templates.  They do take on the
responsibility of turning all encountered inputs into strings, lest they
fail along the lines of sqlalchemy/mako#272.

This change should be sufficiently backwards compatible to not cause any
concerns.  Sure, technically a "n" filter at page tag level was treated as a
no-op so far.  So theoretically existing templates could break.  But there
was no incentive to have such an "n" filter at the page tag level, and the
expressed semantics of the "n" filter is to suppress default filters, so
semantically anyone relying on it being a no-op in that situation was using
unsupported hacks anyway.

Closes: #299
Pull-request: sqlalchemy/mako#299
Pull-request-sha: f8d5a22db3230634d2b42c59909985f31875a9f5

Change-Id: Ide030975229c1df7c0cef534976f740a03c17ca6
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.

4 participants