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

Update H033 regex to ignore data-action #743

Closed Mouarius closed 1 year ago

Mouarius commented 1 year ago

Pull Request Check List

Resolves: #issue-number-here

Description of the context

I came across a linting issue when using djlint along with Catalyst custom elements : I was using the data-action attribute on a form element with multiline content, as it is defined in the docs for multiple actions.

Code example :

<form action="#"
    method="get"
    data-action="
        submit:my-custom-element#handleFormSubmit
        click:my-custom-element#handleClick
        some-event:my-custom-element#handleSomething" >
    <!-- some content -->
</form>

Error

I got a linting H033 : Extra whitespace found in form action. error, not for the action but for the data-action. I want it to apply only to action attributes.

Reasons to fix it

I want to be able to have my actions string spanning over multiple lines without triggering an error with djlint, which maybe should not apply to a data-action attribute. This is maybe a minor change, and I'm not an expert of regex so maybe someone should review my regex and the legitimity of my proposition.

What do you guys think ?

netlify[bot] commented 1 year ago

Deploy Preview for djlint canceled.

Name Link
Latest commit aa78ca7c73e15998484b73ae77b181a4093e40e1
Latest deploy log https://app.netlify.com/sites/djlint/deploys/64e9cd9cef5667000767c37d
silverwind commented 1 year ago

Does not seem right. It should exactly match the action attribute and nothing else, so foo-actionshould also not trigger it.

Mouarius commented 1 year ago

Apparently it doesn't, it triggered this rule with my data-action attribute on my form. As an example, I've tested this regex and apparently it does match everything containing "action" in it : https://regex101.com/r/8YN0EZ/1 Maybe I could change it to be sure that it only matches "action" exactly.

silverwind commented 1 year ago

Your latest version seems good, but I think it can be simpler and just look for a whitespace before action, so it does not need a lookbehind:

https://regex101.com/r/3ZAco0/2

Mouarius commented 1 year ago

You're right ! I'm changing it right now ! Thank you !

Your latest version seems good, but I think it can be simpler and just look for a whitespace before action, so it does not need a lookbehind:

https://regex101.com/r/3ZAco0/2

You're right ! I'm changing it right now ! Thank you !

silverwind commented 1 year ago

Updated one more time, removed the unnecessary character class:

https://regex101.com/r/3ZAco0/3

Mouarius commented 1 year ago

I did it already ;)

codecov[bot] commented 1 year ago

Codecov Report

Patch and project coverage have no change.

Comparison is base (c339061) 95.35% compared to head (7c96cc6) 95.35%. Report is 7 commits behind head on dev.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## dev #743 +/- ## ======================================= Coverage 95.35% 95.35% ======================================= Files 16 16 Lines 1033 1033 Branches 278 278 ======================================= Hits 985 985 Misses 34 34 Partials 14 14 ```

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

christopherpickering commented 1 year ago

Thanks for this, sorry I missed it! I will update the pr with a test and release.

christopherpickering commented 1 year ago

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

The release is available on:

Your semantic-release bot :package::rocket: