Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Django/Jinja2 improvements #1800

Merged
merged 2 commits into from
Mar 10, 2019
Merged

Django/Jinja2 improvements #1800

merged 2 commits into from
Mar 10, 2019

Conversation

RunDevelopment
Copy link
Member

I completely remade the Django/Jinja2 language definition using markup templating.

Because Django templates are (almost) a subset of Jinja2 templates both remain one language definition.

The new definition only half the previous size because it uses a positional approach for tags, keywords and filters instead of keyword lists.

For future reviewers:

This is the code I used to test the language:

{% extends layout_template if layout_template is defined else 'master.html' %}

{% from 'forms.html' import input as input_field, textarea %}

{% include "sidebar.html" ignore missing %}
{% include "sidebar.html" ignore missing with context %}
{% include "sidebar.html" ignore missing without context %}

{% get_language_info_list for available_languages as langs %}
{% for lang in langs %} ... {% endfor %}

{% for athlete in athlete_list %}
    <li>{{ athlete.name }}</li>
{% empty %}
    <li>Sorry, no athletes in this list.</li>
{% endfor %}

{% trans "starting point" as start %}
{% trans "end point" as end %}
{% trans "La Grande Boucle" as race %}
{% blocktrans with amount=article.price %}
That will cost $ {{ amount }}.
{% endblocktrans %}

{% blocktrans with myvar=value|filter %}
This will have {{ myvar }} inside.
{% endblocktrans %}

{% load l10n %}

{% localize on %}
    {{ value }}
{% endlocalize %}

{% localize off %}
    {{ value }}
{% endlocalize %}

{% timezone None %}
    Server time: {{ value }}
{% endtimezone %}

{% load tz %}
{% get_current_timezone as TIME_ZONE %}
<form action="{% url 'set_timezone' %}" method="POST">
    {% csrf_token %}
    <label for="timezone">Time zone:</label>
    <select name="timezone">
        {% for tz in timezones %}
        <option value="{{ tz }}"{% if tz == TIME_ZONE %} selected{% endif %}>{{ tz }}</option>
        {% endfor %}
    </select>
    <input type="submit" value="Set">
</form>

{{ my_date|date:"Y-m-d" }}
{{ value|truncatechars:9 }}
{{ name|striptags|title }}
{{ listx|join(', ') }}
{{ "%s - %s"|format("Hello?", "Foo!") }}
{{ users|join(', ', attribute='username') }}
{{ mytext|urlize(40, true) }}
{{ [1, 2, -3]|max }}
{{ foo['bar'] }}

<ul{{ {'class': 'my_list', 'missing': none,
        'id': 'list-%d'|format(variable)}|xmlattr }}>

<img src="{{ static('path/to/company-logo.png') }}" alt="Company Logo">
<a href="{{ url('admin:index') }}">Administration</a>

{%+ if something %}yay{% endif %}
{%- endfor %}

{% if messages|length >= 100 %}
   You have lots of messages today!
{% endif %}

{% if name is defined %}
{% if loop.index is divisibleby 3  %}
{% if loop.index is divisibleby(3) %}
{% if foo.attribute is sameas false %}
{% if somevar is True %}
{% if somevar is not True %}

{% for value in values %}
    {% if loop.previtem is defined and value > loop.previtem %}
        The value just increased!
    {% endif %}
    {{ value }}
    {% if loop.nextitem is defined and loop.nextitem > value %}
        The value will increase even more!
    {% endif %}
{% endfor %}
{%- for item in sitemap recursive %}
    <li><a href="{{ item.href|e }}">{{ item.title }}</a>
    {%- if item.children -%}
        <ul class="submenu">{{ loop(item.children) }}</ul>
    {%- endif %}</li>
{%- endfor %}
{% for user in users if not user.hidden %}
    <li>{{ user.username|e }}</li>
{% endfor %}

{% regroup cities|dictsort:"country" by country as country_list %}

{% load i18n %}
<html>
  <head>
    <title>{% trans 'Homepage - Hall of Fame' %}</title>
  </head>
  <body>
    {# Translated in the view: #}
    <h1>{{ message }}</h1>
    <p>
      {% blocktrans count member_count=bands.count %}
      Here is the only band in the hall of fame:
      {% plural %}
      Here are all the {{ member_count }} bands in the hall of fame:
      {% endblocktrans %}
    </p>
    <ul>
    {% for band in bands %}
      <li>
        <h2><a href="{{ band.get_absolute_url }}">{{ band.name }}</a></h2>
        {% if band.can_rock %}<p>{% trans 'This band can rock!' %}</p>{% endif %}
      </li>
    {% endfor %}
    </ul>
  </body>
</html>

<style>
a {
	color: {{ bar }};
}
</style>

Before

image

After

image

This resolves #1643.
This fixes #1797.

mAAdhaTTah
mAAdhaTTah requested changes Mar 10, 2019
View reviewed changes
Copy link
Member

@mAAdhaTTah mAAdhaTTah left a comment

Choose a reason for hiding this comment

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

Maybe it's not a big deal, but curious why use {{ instead of {% for some of these tests?

components/prism-django.js Outdated Show resolved Hide resolved
tests/languages/django!+css/inclusion.test Show resolved Hide resolved
@RunDevelopment
Copy link
Member Author

The difference between {{ }} and {% %} is that the first can only contain simple expressions (e.g. {{ foo.bar|baz:"arg" }}) while the latter can hold more complex statements (e.g. {% if foo.bar|baz:"arg" is defined or x > 5 %}).
Therefore it's usually sufficient to just test the more complex case (also most of the features implemented are for statements). The cases where I do use {{}} are usually code I copied from the official specs.

@RunDevelopment RunDevelopment merged commit f246748 into PrismJS:master Mar 10, 2019
@RunDevelopment RunDevelopment deleted the django-improvements branch March 10, 2019 17:53
This was referenced Oct 26, 2019
Open
Open
@snyk-bot snyk-bot mentioned this pull request Mar 8, 2020
Open
@snyk-bot snyk-bot mentioned this pull request Mar 21, 2020
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mAAdhaTTah mAAdhaTTah mAAdhaTTah approved these changes

Assignees
No one assigned
Labels
enhancement language-definitions needs review
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

prism cant work Expand tests for django
2 participants
@RunDevelopment @mAAdhaTTah