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.

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

Allow indentation inside template tags #6

Open
robertpro opened this issue Jul 5, 2020 · 8 comments
Open

Allow indentation inside template tags #6

robertpro opened this issue Jul 5, 2020 · 8 comments
Labels
bug Something isn't working help wanted Extra attention is needed parser This issue relates to Curlylint’s templates parser
Milestone

Comments

@robertpro
Copy link

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?

@robertpro robertpro added the enhancement New feature or request label Jul 5, 2020
@thibaudcolas
Copy link
Owner

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:

def check_jinja_element(
expected_level, element, inline=False, allow_same_line=False
):
if element.begin.line == element.end.line:
inline = True
for part in element.parts:
check_node(
expected_level,
part,
inline=inline,
allow_same_line=allow_same_line,
)
if element.closing_tag is not None:
check_indent(expected_level, element.closing_tag, inline=inline)
.

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.

@thibaudcolas thibaudcolas added bug Something isn't working help wanted Extra attention is needed and removed enhancement New feature or request labels Jul 7, 2020
@robertpro
Copy link
Author

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

@jace
Copy link

jace commented Sep 3, 2020

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
Copy link
Owner

@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.

@thibaudcolas thibaudcolas added the parser This issue relates to Curlylint’s templates parser label Sep 8, 2020
@thibaudcolas thibaudcolas added this to the Nice to have milestone Mar 13, 2021
@jace
Copy link

jace commented Jun 23, 2021

@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
Copy link
Owner

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

indent = 2

@Mischback
Copy link

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
  • line numbers not matching the input file (off by one)
  • generally deviating from (what I would consider) best practices for HTML, e.g. recommending to not indent <head> or <body within <html>

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
Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working help wanted Extra attention is needed parser This issue relates to Curlylint’s templates parser
Projects
None yet
Development

No branches or pull requests

5 participants