search

djlint / djLint

✨ HTML Template Linter and Formatter. Django - Jinja - Nunjucks - Handlebars - GoLang
https://djLint.com
GNU General Public License v3.0
686 stars 84 forks source link

Jinja formatting issues #715

Closed jessielw closed 1 month ago

jessielw commented 1 year ago

Pull Request Check List

Resolves: #issue-number-here

netlify[bot] commented 1 year ago

Deploy Preview for djlint ready!

Name Link
Latest commit 71698c098271b48d2218b841ca0107a5a9348db4
Latest deploy log https://app.netlify.com/sites/djlint/deploys/64f1d2647d2f640008a3b370
Deploy Preview https://deploy-preview-715--djlint.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

jessielw commented 1 year ago

Note: This forces double quotes on the outside and single quotes inside.

jessielw commented 1 year ago

Update: New code is stable, passes all tests, allows the linter to detect the outer most quotes and apply changes to the inner most to prevent vscode and syntax errors as well as handles spacing issues that can be caused by using the built in formatter in vscode and then using djlint

codecov[bot] commented 1 year ago

Codecov Report

Patch coverage: 29.62% and project coverage change: -1.69% :warning:

Comparison is base (07dfbad) 95.35% compared to head (e9dc2aa) 93.67%. Report is 18 commits behind head on master.

:exclamation: Current head e9dc2aa differs from pull request most recent head 71698c0. Consider uploading reports for the commit 71698c0 to get more accurate results

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #715 +/- ## ========================================== - Coverage 95.35% 93.67% -1.69% ========================================== Files 16 16 Lines 1033 1059 +26 Branches 278 285 +7 ========================================== + Hits 985 992 +7 - Misses 34 50 +16 - Partials 14 17 +3 ``` | [Files Changed](https://app.codecov.io/gh/Riverside-Healthcare/djLint/pull/715?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Riverside-Healthcare) | Coverage Ξ” | | |---|---|---| | [src/djlint/formatter/indent.py](https://app.codecov.io/gh/Riverside-Healthcare/djLint/pull/715?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Riverside-Healthcare#diff-c3JjL2RqbGludC9mb3JtYXR0ZXIvaW5kZW50LnB5) | `87.19% <29.62%> (-11.36%)` | :arrow_down: |

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

christopherpickering commented 1 year ago

Sorry for the delay here. I updated the test case a bit, and added a more complex test. Can every see if they agree w/ this code:

image

netlify[bot] commented 8 months ago

Deploy Preview for djlint canceled.

Name Link
Latest commit 14dc2716832c21711a40c4aa40b792dfc8eabba6
Latest deploy log https://app.netlify.com/sites/djlint/deploys/65c9841a578bd400097352e3
jessielw commented 8 months ago

Sorry for the delay here. I updated the test case a bit, and added a more complex test. Can every see if they agree w/ this code:

  • if outer quote is a double, the first set of inner should be a singe. Lines 1-4 didn't make this change
  • notice line 3 and 4 lost a backslash in a string... it should not have been dropped
  • this should probably be a config action, I see issues coming up w/ folks who disagree (at the same time remove the jinja restriction)

image

Not really sure how we could remedy this. VSCode is still going to complain about the expected test output.

Not really sure what the best approach would be to remedy this since VSCode doesn't like the expected results either. image

dakixr commented 1 month ago

Hey, just checking what is missing here before merging?

I am using the ident.py changes locally and seems to be working just fine

jessielw commented 1 month ago

The post I made above yours is the last I messed with it. The repository owner isn't maintaining it right now to my understanding. There is @monosans I believe but haven't heard anything on this subject from him. Perhaps he could shed some light on a solution to this.

christopherpickering commented 1 month ago

:tada: This PR is included in version 1.34.2 :tada:

The release is available on:

Your semantic-release bot :package::rocket:

dakixr commented 1 month ago

Hi @christopherpickering, I just updated the package to the latest release, tks for your incredible work!

However this formatting issue is still present.

Just as an example:

code:

<link rel="stylesheet" href="{{ static('css/output.css') }}?{{ now() .strftime('%d-%m-%Y') }}" />

gets formatted into:

<link rel="stylesheet" href="{{ static("css/output.css") }}?{{ now().strftime('%d-%m-%Y') }}" />

As a workaround I am manually changing ident.py with https://github.com/jessielw/djLint/blob/7b7e504de721e51cf4c5dcc8115540145a8eac23/src/djlint/formatter/indent.py contents.

Is there any reason why this changes were not included in the new releases?

monosans commented 1 month ago

Hi @christopherpickering, I just updated the package to the latest release, tks for your incredible work!

However this formatting issue is still present.

Just as an example:

code:

<link rel="stylesheet" href="{{ static('css/output.css') }}?{{ now() .strftime('%d-%m-%Y') }}" />

gets formatted into:

<link rel="stylesheet" href="{{ static("css/output.css") }}?{{ now().strftime('%d-%m-%Y') }}" />

As a workaround I am manually changing ident.py with jessielw/djLint@7b7e504/src/djlint/formatter/indent.py contents.

Is there any reason why this changes were not included in the new releases?

Seems like I've merged branches incorrectly. Will take a look right now. FYI Christopher is not maintaining djLint anymore.

dakixr commented 1 month ago

Hi @christopherpickering, I just updated the package to the latest release, tks for your incredible work! However this formatting issue is still present. Just as an example: code:

<link rel="stylesheet" href="{{ static('css/output.css') }}?{{ now() .strftime('%d-%m-%Y') }}" />

gets formatted into:

<link rel="stylesheet" href="{{ static("css/output.css") }}?{{ now().strftime('%d-%m-%Y') }}" />

As a workaround I am manually changing ident.py with jessielw/djLint@7b7e504/src/djlint/formatter/indent.py contents. Is there any reason why this changes were not included in the new releases?

Seems like I've merged branches incorrectly. Will take a look right now. FYI Christopher is not maintaining djLint anymore.

Oh so I will rephrase then, tks you @monosans for your work!

Got it!

monosans commented 1 month ago

I'm pretty sure I've merged everything correctly. Need to find the reason why it doesn't work.

dakixr commented 1 month ago

The issue is with the commit: "- Add the ability to detect what ever the outer most quotes was for e…"

That one re-introduced the bug.

I am using this script to fix it locally for the new releases:

#!/bin/bash

# URL to fetch the new contents from
# Commit: WORKING - "- Added some regex to ensure we have double quoted attributes for the…"
URL="https://raw.githubusercontent.com/jessielw/djLint/7b7e504de721e51cf4c5dcc8115540145a8eac23/src/djlint/formatter/indent.py"
# Commit: BROKEN - "- Add the ability to detect what ever the outer most quotes was for e…"
# URL="https://raw.githubusercontent.com/jessielw/djLint/e9dc2aaf7c977def61af6c75fd0b7fc87634d198/src/djlint/formatter/indent.py"

# Find all indent.py files under **/djlint/formatter/ directories
files=$(find . -type f -path "*/djlint/formatter/indent.py")

# Loop over each found file and replace its contents
for file in $files; 
do
    echo "Replacing contents of $file"
    curl -s $URL -o "$file"
done

echo "Replacement complete."