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

Support optionality in new {% types %} tag #322

Closed
ruudk opened this issue Nov 5, 2024 · 13 comments · Fixed by #323
Closed

Support optionality in new {% types %} tag #322

ruudk opened this issue Nov 5, 2024 · 13 comments · Fixed by #323

Comments

@ruudk
Copy link
Contributor

ruudk commented Nov 5, 2024

The new types tag supports optional variables like this:

{% types {
    name?: 'string',
} %}

But this is currently detected as a problem:

 KO ./tests/EndToEnd/Types/optionality.twig
 ------- -----------------------------------------------------
  ERROR   6    | {% types {
          7    |     name?: 'string',
          >>   | Expecting 1 whitespace before "?:"; found 0.
          8    | } %}
 ------- -----------------------------------------------------

And fixed like this:

{% types {
    name ?: 'string|null',
} %}

which is not valid.

@VincentLanglet
Copy link
Owner

VincentLanglet commented Nov 5, 2024

Can you link the PR introducing the syntax in twig ? I'd like to know how they parse it.

Currently '?:' is parsed as an operator. We will need to parse it as a punctuation (like :) in some case.

All the work to do is my Tokenizer.

@ruudk
Copy link
Contributor Author

ruudk commented Nov 5, 2024

Sure, it's done here:

It's shown as closed but it's actually merged.

@VincentLanglet
Copy link
Owner

VincentLanglet commented Nov 5, 2024

I assume

{% types {'foo': 'int', 'bar'?: 'string'} %}

is not valid too or is it valid @ruudk ?

So I also have to avoid the HashQuoteRule to add extra quote...

@ruudk
Copy link
Contributor Author

ruudk commented Nov 5, 2024

No indeed. The types tag has a special syntax.

The keys are unquoted. They can have a ? at the end.

The values as always quoted.

@VincentLanglet
Copy link
Owner

Current WIP #323

@VincentLanglet
Copy link
Owner

I'm opened to suggestion @ruudk

Currently

{% types {foo: 'int', bar?: 'string'} %}

is tokenized

[
                0 => Token::BLOCK_START_TYPE,
                1 => Token::WHITESPACE_TYPE,
                2 => Token::BLOCK_NAME_TYPE, // types
                3 => Token::WHITESPACE_TYPE,
                4 => Token::PUNCTUATION_TYPE, // {
                5 => Token::NAME_TYPE, // foo
                6 => Token::PUNCTUATION_TYPE, // :
                7 => Token::WHITESPACE_TYPE,
                8 => Token::STRING_TYPE, // 'int'
                9 => Token::PUNCTUATION_TYPE, // ,
                10 => Token::WHITESPACE_TYPE,
                11 => Token::NAME_TYPE, // bar
                12 => Token::PUNCTUATION_TYPE, // ?:
                13 => Token::WHITESPACE_TYPE,
                14 => Token::STRING_TYPE, // 'string'
                15 => Token::PUNCTUATION_TYPE, // }
                16 => Token::WHITESPACE_TYPE,
                17 => Token::BLOCK_END_TYPE,
                18 => Token::EOL_TYPE,
                19 => Token::EOF_TYPE,
            ]

In order to not add quote with the HashQuote rule, on way is to use different token type for this.
So should we introduce some TYPE_NAME_TYPE or VAR_NAME_TYPE ? (For index 5 and 11)
Should I instead use something different than PUNCTUATION_TYPE for index 6 and 12 ?
Also I could use something different for the string type too...

I dunno.

@ruudk
Copy link
Contributor Author

ruudk commented Nov 5, 2024

What if we split ?: into 2 tokens: OPTIONALYTY_TYPE and PUNCTUATION_TYPE.

Is it possible for HashQuoteRule to ignore NAME_TYPE when Token::BLOCK_NAME_TYPE == types?

@ruudk
Copy link
Contributor Author

ruudk commented Nov 5, 2024

What if we split ?: into 2 tokens: OPTIONALYTY_TYPE and PUNCTUATION_TYPE.

Or, we keep it as a single token and call it OPTIONALITY_PUNCTUATION_TYPE

@VincentLanglet
Copy link
Owner

What if we split ?: into 2 tokens: OPTIONALYTY_TYPE and PUNCTUATION_TYPE.

I can indeed, actually I just called ?: a punctuation type and it seems enough.

Is it possible for HashQuoteRule to ignore NAME_TYPE when Token::BLOCK_NAME_TYPE == types?

That's a good solution.

@VincentLanglet
Copy link
Owner

Does #323 works fine for you @ruudk ?

@ruudk
Copy link
Contributor Author

ruudk commented Nov 5, 2024

I'll test it out now!

@ruudk
Copy link
Contributor Author

ruudk commented Nov 5, 2024

Tested, it works!

@VincentLanglet
Copy link
Owner

Release in 3.2.0

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants