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

[BUG] [Formatter] Nunjucks block assignment changes in v1.28 incorrect #670

Closed
3 tasks done
aarongoldenthal opened this issue May 26, 2023 · 3 comments
Closed
3 tasks done
Labels

Comments

@aarongoldenthal
Copy link

System Info

  • OS: Windows 11, Alpine 3.17
  • Python Version (python --version): 3.10.11
  • djLint Version (djlint --version): 1.29.0
  • template language: nunjucks

Issue

Nunjucks block assignment changes from #646 in v1.28, with fixes in v1.29, are incorrect.

Give the following code as formatted in v1.27.2:

{% for tag in collections.all | getAllTags | filterTagList | sort %}
    {% set tagUrl %}/tags/{{ tag | slugify }}/{% endset %}
    <a href="{{ tagUrl }}" class="post-tag">{{ tag }} ({{ collections[tag].length }})</a>
{% endfor %}

the formatting with v1.28/v1.29 is

{% for tag in collections.all | getAllTags | filterTagList | sort %}
    {% set tagUrl %}
        /tags/{{ tag | slugify }}/{% endset %}
        <a href="{{ tagUrl }}" class="post-tag">{{ tag }} ({{ collections[tag].length }})</a>
    {% endfor %}

This has two issues

  1. The {% endset %} isn't carried to the next line with the appropriate indent, so the following lines all have the incorrect indent.
  2. Reformatting block sets like this introduces undesirable whitespace, that then has to be trimmed. It be really helpful if there were an option to disable this behavior (without disabling everything). I didn't see one in configuration.

How To Reproduce

For reference my .djlintrc is

{
  "extension": "njk",
  "indent": "4",
  "blank_line_after_tag": "extends, include",
  "blank_line_before_tag": "extends, include",
  "profile": "nunjucks",
  "require_pragma": false,
  "max_line_length": "120",
  "max_attribute_length": "100",
  "use_gitignore": true,
  "format_attribute_template_tags": true,
  "preserve_leading_space": false,
  "preserve_blank_lines": true,
  "format_css": false,
  "format_js": false
}
@aarongoldenthal aarongoldenthal added 🦠 bug Something isn't working 🧽 formatter labels May 26, 2023
@aarongoldenthal
Copy link
Author

aarongoldenthal commented May 27, 2023

After think a little more about it, I still think the the right answer is to not format the contents of block {% set %}. It's assigning the value of a string to a variable and only the author knows if whitespace in that string is significant or not.

There could obviously be cases where it doesn't matter, like the original {% include %} in #646, so maybe the right answer is either:

  • If the value is literal content (i.e. only non-whitespace literal characters and/or variables), then don't format {% set %}
  • Identify a set of tags to format {% set %} (e.g. {% include %}, otherwise don't format. May be with a configuration option (like blank_line_after_tag or blank_line_before_tag)

That doesn't necessarily resolve the {% endset %} issue noted above.

@christopherpickering
Copy link
Contributor

Thanks, I'll get this fixed in the next release. The endset is mssing in the de-dent config.

New change:

{% for tag in collections.all | getAllTags | filterTagList | sort %}
     {% set tagUrl %}
-        /tags/{{ tag | slugify }}/{% endset %}
-        <a href="{{ tagUrl }}" class="post-tag">[{{ tag }} ({{ collections[tag].length }}](https://github.com/Riverside-Healthcare/djLint/issues/%7B%7B%20tagUrl%20%7D%7D)[)](https://github.com/Riverside-Healthcare/djLint/issues/%7B%7B%20tagUrl%20%7D%7D)</a>
-    {% endfor %}
+        /tags/{{ tag | slugify }}/
+    {% endset %}
+    <a href="{{ tagUrl }}" class="post-tag">[{{ tag }} ({{ collections[tag].length }})](https://github.com/Riverside-Healthcare/djLint/issues/%7B%7B%20tagUrl%20%7D%7D)</a>
+{% endfor %}

If at some point you want to prevent set from being indented you could add it to the ignore_blocks config 👍🏽

christopherpickering pushed a commit that referenced this issue May 30, 2023
# [1.30.0](v1.29.0...v1.30.0) (2023-05-30)

### Bug Fixes

* **formatter:** fixed endset indent level ([cdebe42](cdebe42)), closes [#670](#670)
* **formatter:** fixed issue with spaceless macro tags having spacess added before - ([5593937](5593937)), closes [#667](#667)
* **npm:** install exact version of djlint ([0727279](0727279))

### Features

* **npm:** add script to update djlint version in postinstall using semantic-release/exec ([040ffc6](040ffc6))
@christopherpickering
Copy link
Contributor

🎉 This issue has been resolved in version 1.30.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants