Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Mimic the behavior of the 'in' operator when applied to strings #214

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

Merged

Conversation

@league55
Copy link
Contributor

@league55 league55 commented Aug 23, 2021

In the original Jinja, when the 'in' operator is applied to strings it searches for a substring.
It happens naturally, as a string is a list of chars in Python. This logic wasn't translated to Jinja2cpp right away.
Examples:
AS IS:

'b' in 'abc' -> false
'abc' in 'b' -> false

EXPECTED:

'b' in 'abc' -> true
'abc' in 'b' -> false

This PR proposes a fix: expect two possible types of inputs - strings and lists.
Lists are being treated the same way as before, for strings sequence::find(substring) is applied.

 

...ead of lists) from original Jinja - search for a substring.
Comment on lines +274 to +279
std::decay_t<decltype(srcStr)> emptyStrView;
using CharT = typename decltype(emptyStrView)::value_type;
std::basic_string<CharT> emptyStr;

auto substring = sv_to_string(srcStr);
auto seq = GetAsSameString(srcStr, this->GetArgumentValue("seq", context)).value_or(emptyStr);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This feels too verbose but I haven't found a simpler way to parse arguments.
I used this place as a reference:

std::decay_t<decltype(srcStr)> emptyStrView;
using CharT = typename decltype(emptyStrView)::value_type;
std::basic_string<CharT> emptyStr;
auto oldStr = GetAsSameString(srcStr, this->GetArgumentValue("old", context)).value_or(emptyStr);
auto newStr = GetAsSameString(srcStr, this->GetArgumentValue("new", context)).value_or(emptyStr);
auto count = ConvertToInt(this->GetArgumentValue("count", context));
auto str = sv_to_string(srcStr);

@league55 league55 marked this pull request as ready for review August 23, 2021 13:12
Copy link
Contributor Author

Hi @rmorozov , does this change make sense to you?

Copy link

@league55 I believe you're missing changes in the docs too...

Copy link
Contributor Author

@luismartingil This repository has no documentation.
I have checked https://github.com/jinja2cpp/jinja2cpp.github.io , but there is nothing to be updated really, as current documentation just claims compatibility with a Jinja in operator, it doesn't describe it https://github.com/jinja2cpp/jinja2cpp.github.io/blob/master/docs/j2_compatibility.md#current-jinja2-support , So it is still relevant IMO
luismartingil reacted with thumbs up emoji

Copy link

@rmorozov Can you please check this PR when you have some time? Thanks!

Copy link

@rmorozov All CI checks passed, right? Do you think we can merge it?

@rmorozov rmorozov merged commit af50a9c into jinja2cpp:master Aug 24, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /