Issue 1170 - [adblockplus.org Anwiki to CMS migration] Migrate content

includes/subscriptionList.tmpl

-{%- macro processSubscription(subscription, parent=None) %}

[Sebastian Noack, 2015/02/13 17:51:49]

You seem to mix camcelcase with underscore notation. I'd rather stick to
underscore notation for jinja code.

[kzar, 2015/02/20 14:55:18]

Done.

+{#
+ # This file is part of the Adblock Plus website,
+ # Copyright (C) 2006-2015 Eyeo GmbH
+ #
+ # Adblock Plus is free software: you can redistribute it and/or modify
+ # it under the terms of the GNU General Public License version 3 as
+ # published by the Free Software Foundation.
+ #
+ # Adblock Plus is distributed in the hope that it will be useful,
+ # but WITHOUT ANY WARRANTY; without even the implied warranty of
+ # MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ # GNU General Public License for more details.
+ #
+ # You should have received a copy of the GNU General Public License
+ # along with Adblock Plus.  If not, see <http://www.gnu.org/licenses/>.
+ #}
+
+{%- macro process_subscription(subscription, parent=None) %}
   <tr{% if subscription["deprecated"] %} class="deprecated"{% endif %}>
     {%- if subscription["supplements"] %}
-    <td rowspan="2" class="dummy"></td><td rowspan="2">

[Sebastian Noack, 2015/02/13 17:51:49]

Nit: I think there should be a newline, at least after closing the first cell.

[kzar, 2015/02/20 14:55:18]

Done.

+      <td rowspan="2" class="dummy"></td>
+      <td rowspan="2">
     {%- else %}
-    <td rowspan="2" colspan="2">
+      <td rowspan="2" colspan="2">
     {%- endif %}
-      <strong class="subscriptionTitle">{{subscription["name"]}}</strong><br />
-      {{subscription["specialization"]}}
+      <strong class="subscriptionTitle">{{ subscription["name"] }}</strong><br>
+      {{ subscription["specialization"] }}
       {%- if subscription["supplements"] %}
-      <br />{{ "supplements_prefix" |translate }} {{' / '.join(subscription["supplements"])}} {{ "supplements_suffix" |translate }}

[Sebastian Noack, 2015/02/13 17:51:49]

Assuming this is HTML (not XHTML), there shouldn't be a trailing slash for
self-closing tags, like <br>.

[kzar, 2015/02/20 14:55:18]

Done.

+      <br>{{ "supplements_prefix"|translate }} {{ ' / '.join(subscription["supplements"]) }} {{ "supplements_suffix"|translate }}
       {%- endif %}
     </td>
     <td>
       {%- if subscription["maintainer"] %}
-      {{ "maintainer_prefix" |translate }} {{subscription["maintainer"]}} {{ "maintainer_suffix" |translate }}<br />

[Sebastian Noack, 2015/02/13 17:51:49]

Nit: Any reason you add a space before the pipe indicating a template filter?
This seems to be inconsistent with other jinja2 code we have. I have never seen
this notation before.

[kzar, 2015/02/20 14:55:18]

Done.

+      {{ "maintainer_prefix"|translate }} {{ subscription["maintainer"] }} {{ "maintainer_suffix"|translate }}<br>
       {%- endif %}
-      {%- set isFirst = True %}
+      {%- set is_first = True %}
       {% for key in ('homepage', 'forum', 'contact', 'faq', 'blog', 'changelog', 'policy') -%}
         {%- set url = subscription[key] -%}
         {%- if url -%}
-          {%- if not isFirst %}, {% endif -%}
-          {%- set isFirst = False -%}
-          <a href="{{url}}">{{key |translate}}</a>
+          {%- if not is_first %}, {% endif -%}
+          {%- set is_first = False -%}
+          <a href="{{ url }}">{{ key|translate }}</a>
         {%- endif %}
       {%- endfor %}
     </td>
   </tr>
   <tr{% if subscription["deprecated"] %} class="deprecated"{% endif %}>
     <td>
       {%- if subscription["deprecated"] %}
-      <strong>{{ "deprecation_warning" |translate }}</strong><br />
+      <strong>{{ "deprecation_warning"|translate }}</strong><br>
       {%- endif %}
-      {{ "subscribe" |translate}}{{' '}}
+      {{ "subscribe"|translate }}{{ ' ' }}
       {%- for title, url, complete in subscription["variants"] -%}
-        <a href="abp:subscribe?location={{url|urlencode}}&amp;title={{title|urlencode}}
+        <a href="abp:subscribe?location={{ url|urlencode }}&amp;title={{ title|urlencode }}
           {%- if parent and not complete -%}
-            {%- set mainTitle, mainURL, mainComplete = parent.variants[0] -%}
-            &amp;requiresLocation={{mainURL|urlencode}}&amp;requiresTitle={{mainTitle|urlencode}}
+            {%- set main_title, main_url, main_complete = parent.variants[0] -%}
+            &amp;requiresLocation={{ main_url|urlencode }}&amp;requiresTitle={{ main_title|urlencode }}
           {%- endif -%}
-        ">{{title}}</a>{%- if not loop.last %}, {% endif -%}
+        ">{{ title }}</a>{%- if not loop.last %}, {% endif -%}
       {%- endfor %}
     </td>
   </tr>
   {%- if not parent -%}
-    {%- for supplement in subscription["supplemented"] |subscription_sort -%}
-      {{ processSubscription(supplement, subscription) }}
+    {%- for supplement in subscription["supplemented"]|subscription_sort -%}
+      {{ process_subscription(supplement, subscription) }}
     {%- endfor -%}
   {%- endif -%}
 {%- endmacro %}
 
-{%- set currentType = subscriptions[0]["type"] -%}
-  <h2>{{ ("type_" + currentType) |translate }}</h2>
+{% macro display_subscriptions(subscriptions) %}
+  {%- set current_type = subscriptions[0]["type"] -%}
+  <h2>{{ ("type_" + current_type)|translate }}</h2>
 
-<table class="subscriptions">
-{%- for subscription in subscriptions |subscription_sort -%}
-  {%- if not subscription["supplements"] -%}
-    {%- if currentType != subscription["type"] -%}
-      {%- set currentType = subscription["type"] %}
-</table>
+  <table class="subscriptions">
+  {%- for subscription in subscriptions|subscription_sort -%}
+    {%- if not subscription["supplements"] -%}
+      {%- if current_type != subscription["type"] -%}
+        {%- set current_type = subscription["type"] %}
+  </table>
 
-<h2>{{ ("type_" + currentType) |translate }}</h2>
+  <h2>{{ ("type_" + current_type)|translate }}</h2>
 
-<table class="subscriptions">
+  <table class="subscriptions">
+      {%- endif -%}
+      {{ process_subscription(subscription) }}
     {%- endif -%}
-    {{ processSubscription(subscription) }}
-  {%- endif -%}
-{%- endfor %}
-</table>
+  {%- endfor %}
+  </table>
+{% endmacro %}