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

style: reformat templates with djlint #568

Closed
wants to merge 2 commits into from
Closed

Conversation

tymees
Copy link
Member

@tymees tymees commented Nov 1, 2023

So, basically a competitor for #567 using DJLint instead. Run without any special config, except excluding docs. (This means also untracked files btw...)

To run:

$ pip install djlint
$ djlint . --reformat

I haven't looked at it before making the PR, making a very pure format. As with #567, non-trimmed blocktrans are broken

@tymees tymees marked this pull request as draft November 1, 2023 16:32
{% if item.url %}
</a>
{% endif %}
<div class="navigation-content {% if not item.url %}inactive{% endif %} {% if main_counter == active %}active{% endif %} ">
Copy link
Contributor

Choose a reason for hiding this comment

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

The original may have been weird but this definitely isn't better

{% trans "FETC-GW" %}
Copy link
Contributor

Choose a reason for hiding this comment

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

Yay! Newlines at end of file

Copy link
Member Author

Choose a reason for hiding this comment

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

Poe's law is at full effect here....

@@ -1,8 +1,6 @@
{% extends "base/base.html" %}

Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should set max-blank-lines=3 or something reasonable. I still want to be able to add empty lines for readability

https://djlint.com/docs/configuration/#max-blank-lines

Copy link
Member Author

Choose a reason for hiding this comment

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

Done; I've also added some config to require a blank line after the extend and load 'blocks'

Comment on lines +20 to +59
$(function() {
{
# We use a counter so that in
case we need more than 2 extra forms, this piece of code just works without changes #
} {
% counter extra_form_counter create 1 %
}

{
%
for form in formset %
}
add_title('{{ form.prefix }}-informed_consent', 'Informed consent participant');
add_title('{{ form.prefix }}-director_consent_declaration', '{% trans '
Toestemmingsverklaring van de schoolleider / hoofd van het departement ' %}');

{
%
if not form.instance.study %
}
if ($("#extra-{% counter extra_form_counter value %}").attr('data-used') == 0)
$("#extra-{% counter extra_form_counter value %}").hide() {
% counter extra_form_counter increment 1 %
} {
%
else %
}

{
%
if not form.instance.study.needs_additional_external_forms %
}
hide_school_forms("{{form.prefix}}");
{
% endif %
} {
% endif %
} {
% endfor %
}
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we all saw this coming to a certain degree. Doesn't make the end result any more bearable.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ehhhhh yeah that's not great...

Simple fix would be to not format JS... But honestly, maaaybe we should just not do this horrible JS generation. It's kinda an anti-pattern better solved with some data attribute parsing

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's keep looking for other places that are borked. Once the new attachments system comes in this code will be gone anyway.

@tymees
Copy link
Member Author

tymees commented Jan 16, 2024

Superseded by the actual attempt in #600

@tymees tymees closed this Jan 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants