search

readthedocs / ext-theme

Read the Docs drop in replacement site templates
2 stars 2 forks source link

Try djlint instead of djhtml #371

Closed agjohnson closed 2 months ago

agjohnson commented 3 months ago

Djhtml is inconsistent and doesn't provide any linting checks. Running djlint instead, and through pre-commit on a per-file basis, gives a fairly easy way to migrate to stronger formatting/linting on template code.

agjohnson commented 3 months ago

If we want to enable this, it should only run on future edits to templates. The reformatting is more consistent than djhtml, but the linting rules are quite helpful and enforce our development rules. However, this would involve fixing some unrelated template issues fairly frequently. Here's an example of fixes:

readthedocsext/theme/templates/includes/footer.html

T002 75:34 Double quotes should be used in tags. {% trans 'Version' %
H022 77:16 Use HTTPS for external links. <a href="http://docs
T002 86:34 Double quotes should be used in tags. {% trans 'Language'
D018 90:18 (Django) Internal links should use the {% url ... %} pattern. <form action="/i18n/
H006 144:16 Img tag should have height and width attributes. <img class="ui tiny
H006 153:16 Img tag should have height and width attributes. <img class="ui tiny
H006 162:16 Img tag should have height and width attributes. <img class="ui tiny
H006 171:16 Img tag should have height and width attributes. <img class="ui tiny
H006 180:16 Img tag should have height and width attributes. <img class="ui tiny
H006 189:16 Img tag should have height and width attributes. <img class="ui tiny

We could start with reformatting and introduce rules next, tune rules back, or just accept that the edits are all really minor and go 100% on djlint from the start.

ericholscher commented 2 months ago

I'm 👍 on moving forward here with limited linting rules to start. It would be really nice if there was a black-style auto-formatter, so we could format everything once, and then move forward with stricter rules. I really dislike hitting random linter issues on every PR.

agjohnson commented 2 months ago

To clarify, djhtml is already doing autoformatting on templates, it just isn't as strict as djlint. djlint has more opinionated formatting and has the option of strict rule checks in addition, which is the stronger upgrade. The autoformatting will only change slightly -- indentation is more opinionated and attribute wrapping is more frequent.

I might be wrong about how this will work, but if we're okay fixing template issues a file at a time, I think the configuration here works. That is, when a template file is touched next, djlint will update the whole file. We will have to tune ignore rules as we go, which is fine, and perhaps just punt on some rules for now too, which I would say is also fine.

ericholscher commented 2 months ago

Yea, I'm fine being pretty aggressive with ignore rules, and then removing them as a later step.