mozilla / django-browserid

Django application for adding BrowserID support.
Mozilla Public License 2.0
180 stars 80 forks source link

The fancy_tag library isn't actually necessary #280

Closed abompard closed 9 years ago

abompard commented 9 years ago

The fancy_tag library is not compatible with Django 1.8 as is. I opened a ticket with a patch on their tracker, but the last commit being two and a half years ago, I fear it isn't maintained. The good news is: django-browserid doesn't actually need it! To my understanding, fancy_tag was only used to get named keywords in template tags, but this feature is supported by Django since version 1.4, which is also django-browserid's current requirement: https://docs.djangoproject.com/en/1.4/releases/1.4/#args-and-kwargs-support-for-template-tag-helper-functions

Here's the patch to remove fancy_tag. I only have python 2.7 and 3.4 on my system so I could only test the tox environment with those versions of Python, but all tests passed with any version of Django.

diff --git a/django_browserid/templatetags/browserid.py b/django_browserid/templatetags/browserid.py
index e3c6852..e8603dd 100644
--- a/django_browserid/templatetags/browserid.py
+++ b/django_browserid/templatetags/browserid.py
@@ -1,32 +1,30 @@
 from django import template

-from fancy_tag import fancy_tag
-
 from django_browserid import helpers

 register = template.Library()

-@fancy_tag(register, takes_context=True)
-def browserid_info(context, **kwargs):
+@register.simple_tag
+def browserid_info(**kwargs):
     return helpers.browserid_info(**kwargs)

-@fancy_tag(register, takes_context=True)
-def browserid_login(context, **kwargs):
+@register.simple_tag
+def browserid_login(**kwargs):
     return helpers.browserid_login(**kwargs)

-@fancy_tag(register, takes_context=True)
-def browserid_logout(context, **kwargs):
+@register.simple_tag
+def browserid_logout(**kwargs):
     return helpers.browserid_logout(**kwargs)

-@fancy_tag(register, takes_context=True)
-def browserid_js(context, **kwargs):
+@register.simple_tag
+def browserid_js(**kwargs):
     return helpers.browserid_js(**kwargs)

-@fancy_tag(register, takes_context=True)
-def browserid_css(context, **kwargs):
+@register.simple_tag
+def browserid_css(**kwargs):
     return helpers.browserid_css(**kwargs)
diff --git a/setup.py b/setup.py
index 62ff4c2..2d7073f 100755
--- a/setup.py
+++ b/setup.py
@@ -37,7 +37,7 @@ setup(
     author_email='mkelly@mozilla.com',
     url='https://github.com/mozilla/django-browserid',
     license='MPL v2.0',
-    install_requires=['requests>=1.0.0', 'fancy_tag==0.2.0'],
+    install_requires=['requests>=1.0.0'],
     test_suite="runtests.runtests",
     include_package_data=True,
     zip_safe=False,  # because we rely on finding templates on the filesystem

If you prefer a pull request, I can do that.

abompard commented 9 years ago

(OK I just went and made a pull request, it's not like is hard to do anyway ;-) )

abompard commented 9 years ago

Oh, I just realized there are no unit tests covering the template tags, but I tried my changes in my application and the login & logout worked fine.

peterbe commented 9 years ago

So the only people who will suffer are people on Django<=1.3?

Osmose commented 9 years ago

So the only people who will suffer are people on Django<=1.3?

Sounds like it. We don't support those people anyway. How dare they!