Giter Site home page Giter Site logo

[RFC] Rule whish list about twig-cs-fixer HOT 22 CLOSED

lyrixx avatar lyrixx commented on July 17, 2024 3
[RFC] Rule whish list

from twig-cs-fixer.

Comments (22)

VincentLanglet avatar VincentLanglet commented on July 17, 2024 3

Released in 2.9.0

from twig-cs-fixer.

smnandre avatar smnandre commented on July 17, 2024 2

How would you handle keys that do need quotes ? Keep the quotes only for them ? (seems legit)

{{ include('foo.twig', { '123': 123 }) }}

{{ include('foo.twig', { 'data-foo': 123 }) }}

from twig-cs-fixer.

VincentLanglet avatar VincentLanglet commented on July 17, 2024

Hi, thanks for the feedback.

A way to know how hard the rule would be to implement is to look at the way the Tokenizer tokenize your strings.

{{ include("hello.html.twig") }}

is tokenized

  • VAR_START_TYPE {{
  • WHITESPACE_TYPE
  • NAME_TYPE include
  • PUNCTUATION_TYPE (
  • STRING_TYPE "hello.html.twig"
  • PUNCTUATION_TYPE )
  • WHITESPACE_TYPE
  • VAR_END_TYPE }}

It should be possible to introduce a rule which say that every STRING_TYPE starting/ending with " should be reformatted.

Something like

{{ include("foo #{p.first} bar") }}

is tokenized

  • VAR_START_TYPE {{
  • WHITESPACE_TYPE
  • NAME_TYPE include
  • PUNCTUATION_TYPE (
  • DQ_STRING_START_TYPE "
  • STRING_TYPE foo
  • INTERPOLATION_START_TYPE #{
  • ...
  • INTERPOLATION_END_TYPE }
  • STRING_TYPE bar
  • DQ_STRING_END_TYPE "
  • PUNCTUATION_TYPE )
  • WHITESPACE_TYPE
  • VAR_END_TYPE }}

So we shouldn't have issue with interpolation.

from twig-cs-fixer.

VincentLanglet avatar VincentLanglet commented on July 17, 2024

For

{{ include('hello.html.twig', { 'publications': alreadyPublished }) }}

it's (without the whitespace type)

  • VAR_START_TYPE {{
  • NAME_TYPE include
  • PUNCTUATION_TYPE (
  • STRING_TYPE 'hello.html.twig'
  • PUNCTUATION_TYPE ,
  • STRING_TYPE 'publications'
  • PUNCTUATION :
  • NAME_TYPE
  • PUNCTUATION_TYPE )
  • VAR_END_TYPE }}

We could look at STRING_TYPE which is just before a PUNCTUATION_TYPE :.
This shouldn't conflict with

foo ? 'bar' :  'baz'

because in such syntax I tokenize : as an OPERATOR_TYPE (and not a punctuation anymore).

NB

{{ include('hello.html.twig', { publications: alreadyPublished }) }}

would be tokenized publications as a NAME_TYPE (not a string anymore).

from twig-cs-fixer.

VincentLanglet avatar VincentLanglet commented on July 17, 2024

So I assume that both of these rule are possible and not so hard.
Would you be interested working on @lyrixx ?

from twig-cs-fixer.

lyrixx avatar lyrixx commented on July 17, 2024

I would like to contribute to this project, But I fear I'll lake time! We'll see :)

from twig-cs-fixer.

smnandre avatar smnandre commented on July 17, 2024
- {{ include("hello.html.twig") }}
+ {{ include('hello.html.twig') }}

Would love to have this rule! 👍

from twig-cs-fixer.

VincentLanglet avatar VincentLanglet commented on July 17, 2024

How would you handle keys that do need quotes ? Keep the quotes only for them ? (seems legit)

Yeah it seems legit to keep the quote.

{{ include('foo.twig', { 'data-foo': 123 }) }}

I think we should rely on Tokenizer::REGEX_NAME = '/[a-zA-Z_\x7f-\xff][a-zA-Z0-9_\x7f-\xff]*/A';

{{ include('foo.twig', { '123': 123 }) }}

Why the quote is needed here ?
I assume twig works like php, therefore ['123' => 123]
is simplified to [123 => 123] anymore

On the contrary

{{ include('foo.twig', { 12e+2: 123 }) }}`

is a valid key but is different from

{{ include('foo.twig', { '12e+2': 123 }) }}`

So i think only number with the format [0-9]+ can be simplified, no ?

from twig-cs-fixer.

smnandre avatar smnandre commented on July 17, 2024

Well obviously if i forget to type the most important char.....

{{ include('foo.twig', { '0123': 123 }) }}

So i think you're right: using the Tokenizer::REGEX_NAME should be a perfect check

from twig-cs-fixer.

VincentLanglet avatar VincentLanglet commented on July 17, 2024

Well obviously if i forget to type the most important char.....

{{ include('foo.twig', { '0123': 123 }) }}

So i think you're right: using the Tokenizer::REGEX_NAME should be a perfect check

Oh, I see.

So we could do Tokenizer::REGEX_NAME and [1-9][0-9]*

from twig-cs-fixer.

VincentLanglet avatar VincentLanglet commented on July 17, 2024

Do you want to beta test #212 @lyrixx @smnandre ?

from twig-cs-fixer.

smnandre avatar smnandre commented on July 17, 2024

On it !

from twig-cs-fixer.

VincentLanglet avatar VincentLanglet commented on July 17, 2024

On it !

Any feedback @smnandre ? I was thinking about merging/releasing the feature in the next days.

from twig-cs-fixer.

smnandre avatar smnandre commented on July 17, 2024

Oh sorry!

It's perfect and really usefull to normalize code styles!

Tested on a personal project + ux website: all works as expected.

👏

from twig-cs-fixer.

VincentLanglet avatar VincentLanglet commented on July 17, 2024

Thanks for the feedback, I released.

I will try to work on

{{ include('hello.html.twig', { 'publications': alreadyPublished }) }}

unless you already started @lyrixx ?

from twig-cs-fixer.

lyrixx avatar lyrixx commented on July 17, 2024

No, Sorry, I didn't find time to work on it

from twig-cs-fixer.

VincentLanglet avatar VincentLanglet commented on July 17, 2024

The HashQuoteRule is ready for beta-test
#217

By default, it require to use quote for hash (opinionated choice, it gives consistency and avoid mistake).
A key like 0123 is reported as an error non fixable because we're not sure if '0123' or '123' is expected.

An option exist to remove quote when possible, it will remove quote

  • For name which doesn't require ' ('foo' is valid 'data-foo' is not).
  • For integer which does not start by 0. (I use the check `$value === (string) (int) $value))

WDYT ? Any edge case forgotten @lyrixx @smnandre ?

from twig-cs-fixer.

lyrixx avatar lyrixx commented on July 17, 2024

LGTM 👍🏼

from twig-cs-fixer.

VincentLanglet avatar VincentLanglet commented on July 17, 2024

LGTM 👍🏼

Thanks.

@smnandre did you find any false positive ?

from twig-cs-fixer.

smnandre avatar smnandre commented on July 17, 2024

I only tested quickly on a personal project to be honest 🤷 ...

from twig-cs-fixer.

VincentLanglet avatar VincentLanglet commented on July 17, 2024

No problem. I merged the PR to test more easily.
I'll try it on some projects.

I'll also maybe change the default value useQuote from true to false before releasing the feature...
I started a poll to know which syntax is preferred https://twitter.com/MisterDeviling/status/1786535766790640069

from twig-cs-fixer.

smnandre avatar smnandre commented on July 17, 2024

It's not impossible there is no common ground here 🤷

from twig-cs-fixer.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.