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

jinja[spacing] doesn't seem to handle brackets #2330

Closed
ianw opened this issue Aug 24, 2022 · 1 comment
Closed

jinja[spacing] doesn't seem to handle brackets #2330

ianw opened this issue Aug 24, 2022 · 1 comment
Labels

Comments

@ianw
Copy link

ianw commented Aug 24, 2022

Summary

Running 6.5.0 on zuul-jobs produces wrong output with a range of things using []

roles/encrypt-file/tasks/main.yaml:1: jinja: Jinja2 spacing could be improved: {{ [ encrypt_file ] if encrypt_file is string else encrypt_file }} -> {{  if encrypt_file is string else encrypt_file }} (jinja[spacing])
roles/encrypt-file/tasks/main.yaml:45: jinja: Jinja2 spacing could be improved: {{ [ encrypt_file ] if encrypt_file is string else encrypt_file }} -> {{  if encrypt_file is string else encrypt_file }} (jinja[spacing])
roles/set-zuul-log-path-fact/tasks/main.yaml:1: jinja: Jinja2 spacing could be improved: {{ zuul.change[-2:] }}/{{ zuul.change }}/{{ zuul.patchset }}/{{ zuul.pipeline }}/{{ zuul.job }}/{{ zuul.build[:7] }} -> {{ zuul.change[- 2:] }}/{{ zuul.change }}/{{ zuul.patchset }}/{{ zuul.pipeline }}/{{ zuul.job }}/{{ zuul.build[:7] }} (jinja[spacing])
roles/set-zuul-log-path-fact/tasks/main.yaml:4: jinja: Jinja2 spacing could be improved: {{ zuul.change[-2:] }}/{{ zuul.change }}/{{ zuul.patchset }}/{{ zuul.pipeline }}/{{ zuul.job }}/{{ zuul.build[:7] }} -> {{ zuul.change[- 2:] }}/{{ zuul.change }}/{{ zuul.patchset }}/{{ zuul.pipeline }}/{{ zuul.job }}/{{ zuul.build[:7] }} (jinja[spacing])
roles/validate-zone-db/tasks/find.yaml:9: jinja: Jinja2 spacing could be improved: {{ zone_db_files + [ [zj_zone_db_found_file.path.split("/")[-2], zj_zone_db_found_file.path] ] }} -> {{ zone_db_files + [[zj_zone_db_found_file.path.split("/")[- 2], zj_zone_db_found_file.path]] }} (jinja[spacing])

Most of the others seem fairly reasonable; but perhaps related is the spacing suggestions

roles/multi-node-bridge/vars/CentOS.yaml:2: jinja: Jinja2 spacing could be improved: {% if ansible_distribution_major_version|int >= 8 -%} rhosp-openvswitch {%- else -%} openvswitch {%- endif %} -> {% if ansible_distribution_major_version | int >= 8 -%} rhosp-openvswitch {%- else -%} openvswitch {%- endif %} (jinja[spacing])

I would argue that variable|int > 123 is actually clearer than variable | int > 123 because in the first case, it is clear that the int is being use as essentially a cast applied to the variable.

Issue Type
  • Bug Report
Ansible and Ansible Lint details
linters installed: ansible==2.10.7,ansible-base==2.10.17,ansible-compat==2.2.0,ansible-core==2.13.3,ansible-lint==6.5.0,
@ianw ianw added bug new Triage required labels Aug 24, 2022
@ssbarnea
Copy link
Member

By the time you files this, this was already fixed on main branch by #2328 - I plan to release a fix today.

Please note that we will still add spacing around pipe (filters), the target is to match black behavior. Any divergence from https://black.vercel.app/ behavior, would count as a valid bug.

It may take few more patch releases to get all cases covered but please bare with us and report any bugs. I personally tested with zuul-jobs over the weekend but due to the number of warnings, some were missed. Still, while fixing these I always add test cases, so once fixed we should never see a regression on it.

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