Implement raw/endraw by morenol · Pull Request #148 · jinja2cpp/Jinja2Cpp (original) (raw)

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 andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation20 Commits2 Checks0 Files changed

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

@morenol

morenol

"noname.j2tpl:1:3: error: Unexpected token: '<>'\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 %}",

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still not sure how to handle this case?

How should I solve this? @flexferrum

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suppose the first endraw should close the first raw block and the second endraw should be unexpected.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think in this case it's necessary to return ExpectedRawEnd error but points to the end of the {% raw %} tag.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. I suppose, you can do it at this line:

FinishCurrentBlock(m_template->size());

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.

flexferrum

flexferrum

static std::regex GetRoughTokenizer()
{
return std::regex(R"((\{\{)|(\}\})
return std::regex(R"((\{\{)|(\}\})

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let me now and I change that.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've just checked. We should do trimming. So the current implementation is correct.

flexferrum

break;
else if (m_currentBlockInfo.type != TextBlockType::RawText && m_currentBlockInfo.type != TextBlockType::Comment)
{
FinishCurrentLine(match.position() + 12);

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No sure we can use here the length as a constant. raw/endraw (according to regex) can contain spaces between the keyword and %}

flexferrum

size_t StripBlockRight(TextBlockInfo& currentBlockInfo, size_t position)
{
bool doTrim = m_settings.trimBlocks && (m_currentBlockInfo.type == TextBlockType::Statement |

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TextBlockType::RawBlock )

Please, use the clang-format with the provided format file.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok. I see. I'll do it by myself.

@codecov

flexferrum

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! Thanks!

@morenol

The only pending thing is the UNIVERSAL_STR for the regex

@flexferrum

The only pending thing is the UNIVERSAL_STR for the regex

Don't mind. :)

@morenol

Ok, then I only will squash the three last commits

@flexferrum

Ok, then I only will squash the three last commits

I would do it during the merge. :)

2 participants

@morenol @flexferrum