Skip to content

add CapitalMode to StringConverter::Filter #139

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
merged 1 commit into from
Sep 26, 2019

Conversation

waybeforenow
Copy link
Contributor

Fixes #137

Note that the documentation says

The first character will be uppercase, all others lowercase.

Here we look for the first (alphanumeric) character, pass it to std::toupper, then pass the remaining alphanumeric characters through std::tolower. Let me know if the desired behavior of the filter is different from this and I'll modify the PR accordingly.

@flexferrum
Copy link
Collaborator

flexferrum commented Sep 25, 2019

Thanks for contributing!

Here we look for the first (alphanumeric) character, pass it to std::toupper, then pass the remaining alphanumeric characters through std::tolower.

I've checked the behavior of this filter in the python Jinja2 and looks like this implementation corresponds to the original capitalize filter behavior in general. So, everything is ok with it except of 'first alphanumeric'. ' user Name1' | capitalize gives user name1.

@codecov
Copy link

codecov bot commented Sep 25, 2019

Codecov Report

Merging #139 into master will increase coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #139      +/-   ##
==========================================
+ Coverage   87.25%   87.26%   +0.01%     
==========================================
  Files          64       64              
  Lines        7209     7219      +10     
==========================================
+ Hits         6290     6300      +10     
  Misses        919      919
Impacted Files Coverage Δ
src/string_converter_filter.cpp 92.85% <100%> (+0.44%) ⬆️
test/filters_test.cpp 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2b26969...763fccd. Read the comment docs.

@waybeforenow
Copy link
Contributor Author

I changed it so that capitalize only capitalizes the first character of the string regardless of whether it's alphanumeric.

(And if !isAlpha(ch), we don't need to invoke std::toupper or std::tolower at all.)

@flexferrum
Copy link
Collaborator

@waybeforenow , do you want to wait until October, 1st with this PR or I can merge it now?

@waybeforenow
Copy link
Contributor Author

waybeforenow commented Sep 26, 2019 via email

@flexferrum flexferrum merged commit 3dc7857 into jinja2cpp:master Sep 26, 2019
@flexferrum
Copy link
Collaborator

Thank you!

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

Successfully merging this pull request may close these issues.

Implement 'capitalize' filter
2 participants