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

Trimming TEXT token values while lexing #12

Closed
PolyPik opened this issue Jul 12, 2019 · 5 comments
Closed

Trimming TEXT token values while lexing #12

PolyPik opened this issue Jul 12, 2019 · 5 comments
Milestone

Comments

@PolyPik
Copy link
Contributor

PolyPik commented Jul 12, 2019

In TwigPHP, the use of whitespace control modifiers on statements can affect the value of the preceding or following TEXT_TYPE token depending on where the modifier was used.

Should this lexer implementation do the same and trim the preceding or following TEXT Token value when it encounters a statement with whitespace control modifiers?

@ericmorand
Copy link
Member

I don't think it should. It would make the lexer lossy.

I'll have something for you (this issue and #10) in the next few days. Certainly in the form of a token stream (see Kocal/prettier-plugin-twig#17 (comment)) that will be easily filterable and come with a toAst function that will return a token list that only contains the token needed for the final output - so without WHITESPACE or COMMENT or trimming tokens and with the TEXT tokens trimmed accordingly.

It is already done and mainly tested (thanks to @Kocal), I just have to move it there.

@PolyPik
Copy link
Contributor Author

PolyPik commented Jul 12, 2019

Nice to hear that. However is this token stream's toAst function going to turn TEST_OPERATOR tokens into OPERATOR tokens as well? TEST_OPERATOR has no equivalent in TwigPHP.

@PolyPik
Copy link
Contributor Author

PolyPik commented Jul 29, 2019

@ericmorand I'm waiting for the changes on your part.

ericmorand added a commit to ericmorand/twig-lexer that referenced this issue Jul 30, 2019
ericmorand added a commit that referenced this issue Jul 30, 2019
@ericmorand ericmorand added this to the 0.5.0 milestone Jul 30, 2019
@ericmorand
Copy link
Member

The token stream will be deployed by the end of the day.

Note that the TEST_OPERATOR token will remain. I really think that the is operator is not a traditional operator. It is actually a language structure that instanciate a test. It can't be customized or replaced without breaking the language, and it's impossible to create a custom operator that would mimic its behavior.

This is backed up by the fact that in both TwigPHP and Twing, there is a test at one point in the parser that goes "if the token is an operator > if its value is is then let's do something special; else let's instanciate an operator". Clearly, would is be an operator, it would fall into the same case as all other operators and would not need a special treatment.

It should be trivial to replace TEST_OPERATOR tokens by OPERATOR tokens in the stream, would you need it.

@PolyPik
Copy link
Contributor Author

PolyPik commented Jul 30, 2019

Got it.

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

No branches or pull requests

2 participants