thibaudcolas / curlylint

Experimental HTML templates linting for Jinja, Nunjucks, Django templates, Twig, Liquid
https://www.curlylint.org/
MIT License
239 stars 25 forks source link

Allow indentation inside template tags #6

Open robertpro opened 4 years ago

robertpro commented 4 years ago

Is your proposal related to a problem?

Yes

Describe the solution you’d like

I would like to indent inside template tags

Describe alternatives you’ve considered

setting curlylint --rule 'indent: 4'

Additional context

Given this code:

{% extends 'home/base.html' %}

{% load cache%}

{% block body %}
    {% cache 6000 home.home %}
        {{ block.super }}
        <p>Last request: {{ date }}</p>
    {% endcache %}
{% endblock %}

I get these errors:

curlylint --rule 'indent: 4' apps/home/templates/home/home.html
apps/home/templates/home/home.html
6:8     Bad indentation, expected 4, got 8      indent
7:8     Bad indentation, expected 4, got 8      indent

Oh no! 💥 💔 💥
2 errors reported

Which I can fix like this:

{% extends 'home/base.html' %}

{% load cache%}

{% block body %}
    {% cache 6000 home.home %}
    {{ block.super }}
    <p>Last request: {{ date }}</p>
    {% endcache %}
{% endblock %}

Result:

curlylint --rule 'indent: 4' apps/home/templates/home/home.html
All done! ✨ 🍰 ✨

Can we add this feature?

thibaudcolas commented 4 years ago

Hey @robertpro! Well that seems more like a bug to me, for the rule to not treat opening and closing template tags as needing indentation.

The indent rule is one that comes straight from https://github.com/motet-a/jinjalint, from which this project started. To be honest it’s not a rule I’m intending to use personally, so not the one I’d be the most concerned about making as useful as possible, but if you want to take a shot at fixing this I’d be happy to help.

I suspect the problem would be around here: https://github.com/thibaudcolas/curlylint/blob/77f01d5fc246e6bc62d145d8f3bcece193f00c01/curlylint/rules/indent/indent.py#L198-L211.

The first step in fixing this will be to write unit tests for this rule – currently there are no tests at all so I’d be weary of making any changes that break it further.

robertpro commented 4 years ago

Sounds good @thibaudcolas I will take a look this weekend.

jace commented 4 years ago

I've arrived at curlylint looking at a Jinja2 linter, and this hit me out of the box. Curlylint doesn't seem to like my indentation no matter what I use. I can't turn off the indentation checker either.

thibaudcolas commented 4 years ago

@jace 👋 how have you configured the linter? Due to the experimental nature of it all linting rules are "opt-in", so you’d need to not "turn on" the indentation check.

jace commented 3 years ago

@thibaudcolas I missed your last comment. My pre-commit config (using an outdated curlylint version since the hook was disabled):

repos:
  - repo: https://github.com/thibaudcolas/curlylint
    rev: v0.12.0
    hooks:
      - id: curlylint

And in pyproject.toml:

[tool.curlylint]
include = '\.jinja2$'
exclude = '''
/(
    \.eggs
  | \.git
  | \.hg
  | \.mypy_cache
  | \.tox
  | \.venv
  | _build
  | __pycache__
  | buck-out
  | build
  | dist
  | node_modules
  | funnel/assets
)/
'''

[tool.curlylint.rules]
# Indent 2 spaces
indent = 2
# All role attributes must be valid.
# See https://www.curlylint.org/docs/rules/aria_role.
aria_role = true
# The `alt` attribute must be present.
# See https://www.curlylint.org/docs/rules/image_alt.
image_alt = true
thibaudcolas commented 3 years ago

Alright, in this case you should be able to turn off the indentation check by removing:

indent = 2
Mischback commented 1 year ago

Feeling like a necromancer, but my issue is related.

Indentation seems really broken atm.

Given the following template (included line numbers on purpose!)

  1 <!DOCTYPE html>
  2 <html lang="en">
  3   <head>
  4     {% block html_head %}
  5       {% block html_meta %}
  6       <meta charset="utf-8" />
  7       <meta name="viewport" content="width=device-width, initial-scale=1" />
  8         {% block html_meta_desc required %}
  9         {#
 10         <meta name="description" content="foo" />
 11         <meta name="keywords" content="bar" />
 12         #}
 13         {% endblock html_meta_desc %}
 14       {% endblock html_meta %}
 15     <title>{% block page_title %}fnord{% endblock page_title %}</title>
 16     {% endblock html_head %}
 17   </head>
 18   <body>
 19     {% block html_body %}
 20     <div id="main">
 21       <strong>base.html</strong>: block "html-body"
 22     </div>
 23     {% endblock html_body %}
 24   </body>
 25 </html> 

using this configuration

[tool.curlylint.rules]
# FIXME: ``true`` means: all roles must be valid. However, providing a list of
#        roles allows limitation to the actual roles in use.
aria_role = true
# FIXME: ``true`` means: the ``lang`` attribute must be present. However,
#        providing a list of accepted values allows finer control.
html_has_lang = true
# Require all ``<img>`` tags to have the ``alt`` attribute (even empty).
image_alt = true
# use 2 spaces for indentation.
# NOTE: This must be synchronized with ``.editorconfig``.
indent = 2                                                                                                                                                           
# allow users to zoom
meta_viewport = true
# don't use the ``autofocus`` attribute on any element
no_autofocus = true
# TODO: Needs investigation!
# Don't use positive values for ``tabindex`` attributes
tabindex_no_positive = true

and running through pre-commit returns this:

base.html
2:2 Bad indentation, expected 0, got 2  indent
3:4 Bad indentation, expected 0, got 4  indent
4:6 Bad indentation, expected 2, got 6  indent
5:6 Bad indentation, expected 4, got 6  indent
6:6 Bad indentation, expected 4, got 6  indent
7:8 Bad indentation, expected 4, got 8  indent
8:8 Bad indentation, expected 6, got 8  indent
12:8    Bad indentation, expected 4, got 8  indent
13:6    Bad indentation, expected 2, got 6  indent
14:4    Bad indentation, expected 2, got 4  indent
15:4    Bad indentation, expected 0, got 4  indent
17:2    Bad indentation, expected 0, got 2  indent
18:4    Bad indentation, expected 0, got 4  indent
19:4    Bad indentation, expected 2, got 4  indent
20:6    Bad indentation, expected 4, got 6  indent
21:4    Bad indentation, expected 2, got 4  indent
22:4    Bad indentation, expected 0, got 4  indent

I understand that this is experimental, and I will just turn that option off. But it may provide additional context for anybody who might give this a shot!

Thank you for curlylint! Some other options make it stand out in the field of HTML/templating linters. I use it besides djLint and they work well together.

dagostinelli commented 1 year ago

Ditto. Necromancer.

This fails with 9:0 Should be indented with tabs indent

{% block content %}
<div>
    <div>
        <div>
            <div>
                <div>
                    <form>
                        {% for error in form.errors %}
                            <div class="error">{{ error }}</div>
                        {% endfor %}
                    </form>
                </div>
            </div>
        </div>
    </div>
</div>
{% endblock %}

This works:

{% block content %}
<div>
    <div>
        <div>
            <div>
                <form>
                    {% for error in form.errors %}
                        <div class="error">{{ error }}</div>
                    {% endfor %}
                </form>
            </div>
        </div>
    </div>
</div>
{% endblock %}

The difference is the second sample has one less level of <div> tags.

Very strange error.