Fix tokenize bug of #8590 #8591
Conversation
lib/eex/lib/eex/tokenizer.ex
Outdated
| # token and, if so, it is not followed by an "end" | ||
| # token. If this is the case, we are on a start expr. | ||
| case :elixir_tokenizer.tokenize(rest, 1, file: "eex", check_terminators: false) do | ||
| {:ok, tokens} -> |
There was a problem hiding this comment.
Hi @LostKobrakai!
I have cleaned up your commit a bit and then I noticed that your implementation actually has a bug. It will match if the code starts with a variable named endgame, for example.
I think that instead of looking at the buffer, you should look if the first token is the end token. It will be the same result, except you will rely on the Elixir tokenizer to handle it properly.
There was a problem hiding this comment.
Great, I'll take a look at it. I wasn't really sure how the elixir_tokenizer behaves, so I tried to stay clear of it :D
There was a problem hiding this comment.
I've gone and replace the check with looking at the results of the tokenizer and added the same improvements to tokens ending in do %>
… followed by do block as test
| @@ -154,6 +158,13 @@ defmodule EEx.Tokenizer do | |||
| :expr | |||
| end | |||
|
|
|||
| # Tokenize the remaining passing check_terminators as false, | |||
| # which relax the tokenizer to not error on unmatched pairs. | |||
| # If the tokens start with an "end" we have a middle expr. | |||
There was a problem hiding this comment.
I would remove the last comment line (If the tokens start with an "end" we have a middle expr.) or move it before the pattern matching (although it's a bit redundant).
| # If the tokens start with an "end" we have a middle expr. |
There was a problem hiding this comment.
I would say it is still relevant info for the callers of this function. :)
|
❤️ 💚 💙 💛 💜 |
No description provided.