Implement raw/endraw #148
Implement raw/endraw #148
Conversation
test/errors_test.cpp
Outdated
| "noname.j2tpl:1:3: error: Unexpected token: '<<End of block>>'\n{{}}\n--^-------"}, | ||
| InputOutputPair{"{% raw %}{% raw %}{{ x }{% endraw %}{% endraw %}", | ||
| "noname.j2tpl:1:37: error: Unexpected raw block end\n{% raw %}{% raw %}{{ x }{% endraw %}{% endraw %}\n ---^-------"}, | ||
| // FIXME: InputOutputPair{"{% raw %}", |
There was a problem hiding this comment.
Still not sure how to handle this case?
How should I solve this? @flexferrum
There was a problem hiding this comment.
I suppose the first endraw should close the first raw block and the second endraw should be unexpected.
There was a problem hiding this comment.
Yes, it is ok. I saw that in this page https://jbmoelker.github.io/jinja-compat-tests/tags/raw/
That case I think is working properly.
The case that I am not sure how to handle is this one:
{% raw %} I mean, the error when the regex_iterator arrives to the EOF and the RawBlock has not been closed.
There was a problem hiding this comment.
I think in this case it's necessary to return ExpectedRawEnd error but points to the end of the {% raw %} tag.
There was a problem hiding this comment.
Yes, I mean, I have to modify the test InputOutputPair values, but the main error here is that this error is not being thrown because I am not sure where I should return ExpectedRawEnd.
Maybe you could suggest me where I should look at.
There was a problem hiding this comment.
I see. I suppose, you can do it at this line:
Jinja2Cpp/src/template_parser.h
Line 355 in bc8c2c2
At this point you can check that the current block is finished or it is raw text. Otherwise you can return an error depends on the unclosed block type.
| @@ -56,7 +56,7 @@ struct ParserTraits<char> : public ParserTraitsBase<> | |||
| { | |||
| static std::regex GetRoughTokenizer() | |||
| { | |||
| return std::regex(R"((\{\{)|(\}\})|(\{%)|(%\})|(\{#)|(#\})|(\n))"); | |||
| return std::regex(R"((\{\{)|(\}\})|(\{%[\+\-]?\s+raw\s+[\+\-]?%\})|(\{%[\+\-]?\s+endraw\s+[\+\-]?%\})|(\{%)|(%\})|(\{#)|(#\})|(\n))"); | |||
There was a problem hiding this comment.
I'm not sure we should do right trimming for raw and left trimming for endraw. Need to check original jinja implementation.
Also I think we can use UNIVERSAL_STR here and get rid of copy/paste of the regexp string.
There was a problem hiding this comment.
Let me now and I change that.
There was a problem hiding this comment.
I've just checked. We should do trimming. So the current implementation is correct.
src/template_parser.h
Outdated
| break; | ||
| else if (m_currentBlockInfo.type != TextBlockType::RawText && m_currentBlockInfo.type != TextBlockType::Comment) | ||
| { | ||
| FinishCurrentLine(match.position() + 12); |
There was a problem hiding this comment.
No sure we can use here the length as a constant. raw/endraw (according to regex) can contain spaces between the keyword and %}
src/template_parser.h
Outdated
|
|
||
| size_t StripBlockRight(TextBlockInfo& currentBlockInfo, size_t position) | ||
| { | ||
| bool doTrim = m_settings.trimBlocks && (m_currentBlockInfo.type == TextBlockType::Statement || m_currentBlockInfo.type == TextBlockType::RawBlock ); |
There was a problem hiding this comment.
TextBlockType::RawBlock )
Please, use the clang-format with the provided format file.
There was a problem hiding this comment.
When I use that format, the file has a lot of changes, is that ok?
There are changes in the include order, the position of the curly brackets, ++ var changes to ++var, and the indentation inside the switches.
There was a problem hiding this comment.
Ok. I see. I'll do it by myself.
Codecov Report
@@ Coverage Diff @@
## master #148 +/- ##
==========================================
+ Coverage 88% 88.14% +0.14%
==========================================
Files 65 65
Lines 8163 8269 +106
==========================================
+ Hits 7184 7289 +105
- Misses 979 980 +1
Continue to review full report at Codecov.
|
|
The only pending thing is the UNIVERSAL_STR for the regex |
Don't mind. :) |
|
Ok, then I only will squash the three last commits |
I would do it during the merge. :) |
Solves #146