VincentLanglet / Twig-CS-Fixer

A tool to automatically fix Twig Coding Standards issues
MIT License
234 stars 25 forks source link

partial template #198

Closed seb-jean closed 7 months ago

seb-jean commented 8 months ago

Rule(s) related to or rule(s) missing

FileNameRule

Expected behavior

Sometimes we use partial templates. For example for the Symfony demo, there is the file https://github.com/symfony/demo/blob/main/templates/blog/_post.html.twig

I then tested and I got the error below:

Actual behavior

 KO .\templates\blog\_post.html.twig
 ------- ----------------------------------------------------------
  ERROR   >>   | The file name must use snake_case; expected post.

Do you think this behavior should be taken into account?

VincentLanglet commented 8 months ago

Do you think this behavior should be taken into account?

Yes, it should be indeed.

VincentLanglet commented 8 months ago

Does https://github.com/VincentLanglet/Twig-CS-Fixer/pull/199 would solve your issue @seb-jean ?

seb-jean commented 8 months ago

It's better :).

However, when I have a file like this: tailwind_2_layout.html.twig, I get the error below:

  KO .\templates\form\tailwind_2_layout.html.twig
  ------- -------------------------------------------- ---------------------------
   ERROR >> | The file name must use snake_case; expected tailwind2_layout.
VincentLanglet commented 8 months ago

However, when I have a file like this: tailwind_2_layout.html.twig, I get the error below:

  KO .\templates\form\tailwind_2_layout.html.twig
  ------- -------------------------------------------- ---------------------------
   ERROR >> | The file name must use snake_case; expected tailwind2_layout.

I rely on Symfony/string for the case validator. I check if you string is the same than the one produced by this method https://github.com/symfony/string/blob/7.0/AbstractUnicodeString.php#L347-L353

And looking at the tests it seems to be an expected behavior https://github.com/symfony/string/blob/7.0/Tests/AbstractAsciiTestCase.php#L1063-L1077

When looking on the net, there is some discussion about should it be name1 or name_1 with maybe a majority of name1. Anyway, I think that if you consider name_1 should be allowed, the discussion would be on Symfony side I think. And looking at https://github.com/symfony/symfony/issues/34777 and https://github.com/symfony/symfony/pull/34781 it seems like the discussion was already done.

So if you want to really follow the symfony standard, seems like tailwind2_layout is the valid way.

seb-jean commented 8 months ago

However this file exists in Symfony: https://github.com/symfony/symfony/blob/7.1/src/Symfony/Bridge/Twig/Resources/views/Form/tailwind_2_layout.html.twig

VincentLanglet commented 8 months ago

However this file exists in Symfony: symfony/symfony@7.1/src/Symfony/Bridge/Twig/Resources/views/Form/tailwind_2_layout.html.twig

Indeed. I opened an issue on Symfony side https://github.com/symfony/symfony/issues/54051 to know if

VincentLanglet commented 7 months ago

I'll close this @seb-jean, I consider: