Add default helper filters #130

bmwant · 2017-06-27T20:29:12Z

Rationale

We want to have optional list of default filters for jinja environment that will be hidden with a feature flag default_filters. Those will provide unified interface of adding some useful jinja2.contextfilterfilters within a package.

TODO

Add unittests
Add showcase example to readme.
Comply codestyle

Notes

@samuelcolvin I've included minor not related changes to setup file, but besides that let's collaborate on this to deliver the feature.

codecov · 2017-06-27T20:29:20Z

Codecov Report

Merging #130 into master will decrease coverage by 100%.
The diff coverage is 0%.

@@           Coverage Diff           @@
##           master   #130     +/-   ##
=======================================
- Coverage     100%     0%   -100%     
=======================================
  Files           1      2      +1     
  Lines          79     96     +17     
  Branches       10     13      +3     
=======================================
- Hits           79      0     -79     
- Misses          0     96     +96

Impacted Files	Coverage Δ
aiohttp_jinja2/helpers.py	`0% <0%> (ø)`
aiohttp_jinja2/__init__.py	`0% <0%> (-100%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ce735f3...e03cb57. Read the comment docs.

bmwant · 2017-06-27T20:30:17Z

aiohttp_jinja2/helpers.py

+
+
+@jinja2.contextfilter
+def reverse_url(context, name_, **parts):


I'd rather leave name_ as __aiohttp_jinja2_route_name to prevent names conflict here.

Use __name -- the chance of conflict is reasonable low with this name

jettify · 2017-06-27T20:41:35Z

setup.py

+        'Programming Language :: Python',
+        'Programming Language :: Python :: 3',
+        'Programming Language :: Python :: 3.4',
+        'Programming Language :: Python :: 3.5',


lets add 3.6 entry as well as 'Framework :: AsyncIO',

jettify · 2017-06-27T20:45:20Z

requirements-dev.txt

@@ -1,12 +0,0 @@
-flake8==3.3.0


We use pyup bot to track dependencies updates like here #129 Could you please check if pyup bot work with setup.py too. Otherwise lets leave as it is.

there is ongoing development for adding support to use setup.py file as source of requirements pyupio/pyup#137. Anyway I get the case and will revert that changes

It doesn't.
Please revert.

asvetlov

I left very many comments but my main question is: why do we need the feature at all?

Prom my perspective {{ url('name', arg=val) }} is prettier and more native than {{ 'name' | url(arg=val) }}.

The last notation is very confusing.

asvetlov · 2017-06-28T12:52:02Z

aiohttp_jinja2/helpers.py

+
+
+@jinja2.contextfilter
+def reverse_url(context, name_, **parts):


Use __name -- the chance of conflict is reasonable low with this name

asvetlov · 2017-06-28T12:52:54Z

aiohttp_jinja2/helpers.py

+        kwargs['query'] = parts.pop('query')
+    if parts:
+        kwargs['parts'] = parts
+    return app.router[name_].url(**kwargs)


.url_for should be used

asvetlov · 2017-06-28T12:53:30Z

aiohttp_jinja2/helpers.py

+    """
+    app = context['app']
+    kwargs = {}
+    if 'query' in parts:


Don't process query separately.

asvetlov · 2017-06-28T12:57:34Z

aiohttp_jinja2/helpers.py

+def reverse_url(context, name_, **parts):
+    """Filter for generating urls.
+
+    See http://aiohttp.readthedocs.io/en/stable/web.html#


We don't use sphinx autodocs for documentation generation.
Please drop the tail of docstring but update docs/index.rst

asvetlov · 2017-06-28T13:19:12Z

aiohttp_jinja2/helpers.py

+
+
+@jinja2.contextfilter
+def reverse_url(context, name_, **parts):


I suggest url_for() name

asvetlov · 2017-06-28T13:24:05Z

requirements-dev.txt

@@ -1,12 +0,0 @@
-flake8==3.3.0


It doesn't.
Please revert.

samuelcolvin · 2017-07-06T12:22:28Z

sorry, I hadn't seen this. I'll update my PR to reflex @asvetlov's requested changes.

I think setup changes should go in a separate PR.

bmwant · 2017-08-14T11:12:52Z

closing this one and will pick small improvements for setup.py file in different PR.

@asvetlov

* adding helper filters url and static * switching to contextfunction * rename test_jinja_globals.py & add tests * adding docs for url and static functions * full coverage to please codecov * @asvetlov's comments on #130 * adapting as per comments

samuelcolvin and others added 6 commits March 27, 2017 14:13

adding helper filters url and static

25ea7c3

Add test require dependencies

3e2052e

Change codestyle

695ba2b

Set test requirements in setup module

c262374

Mangle name not to conflict with one in test

85e0890

Update readme

e03cb57

bmwant added the enhancement label Jun 27, 2017

bmwant commented Jun 27, 2017

View reviewed changes

jettify reviewed Jun 27, 2017

View reviewed changes

asvetlov requested changes Jun 28, 2017

View reviewed changes

samuelcolvin added a commit to samuelcolvin/aiohttp-jinja2 that referenced this pull request Jul 6, 2017

@asvetlov's comments on aio-libs#130

88b6d57

samuelcolvin added a commit to samuelcolvin/aiohttp-jinja2 that referenced this pull request Aug 8, 2017

@asvetlov's comments on aio-libs#130

b429669

bmwant closed this Aug 14, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add default helper filters #130

Add default helper filters #130

bmwant commented Jun 27, 2017

codecov bot commented Jun 27, 2017 •

edited

Loading

bmwant Jun 27, 2017

asvetlov Jun 28, 2017

jettify Jun 27, 2017

jettify Jun 27, 2017

bmwant Jun 28, 2017

asvetlov Jun 28, 2017

asvetlov left a comment

asvetlov Jun 28, 2017

asvetlov Jun 28, 2017

asvetlov Jun 28, 2017

asvetlov Jun 28, 2017

asvetlov Jun 28, 2017

asvetlov Jun 28, 2017

samuelcolvin commented Jul 6, 2017

bmwant commented Aug 14, 2017



		@jinja2.contextfilter
		def reverse_url(context, name_, **parts):

Add default helper filters #130

Add default helper filters #130

Conversation

bmwant commented Jun 27, 2017

Rationale

TODO

Notes

codecov bot commented Jun 27, 2017 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

asvetlov left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

samuelcolvin commented Jul 6, 2017

bmwant commented Aug 14, 2017

codecov bot commented Jun 27, 2017 •

edited

Loading