Skip to content
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

Ignore curly brackets inside string literals in data expressions #190

Merged
merged 4 commits into from
May 21, 2021

Conversation

Dakror
Copy link
Contributor

@Dakror Dakror commented May 20, 2021

Data expressions like {{ 'test {} with format library' }} currently dont work with libraries like fmt, because curly braces inside the string trigger an error. This PR fixes this.

@Dakror
Copy link
Contributor Author

Dakror commented May 20, 2021

I'm currently integrating sprintf as a function for data expressions (in combination with my i18n strings) based on the fmt lib.
I guess you wouldn't want to add a dependency, but when I'm done i could send a PR. It involves defining a formatter for the Variant type

@Dakror Dakror marked this pull request as draft May 20, 2021 12:47
@Dakror Dakror marked this pull request as ready for review May 20, 2021 12:50
@mikke89
Copy link
Owner

mikke89 commented May 20, 2021

Thanks!

I think it makes more sense to move in_string as an out parameter into XMLParseTools::ParseDataBrackets and do this parsing there. Also, is DataViewText::Initialize affected by this? Maybe we want to use the same XMLParseTools::ParseDataBrackets in there?

Your translation stuff sounds cool, I'd love to see more of it. We wouldn't want fmt as a core dependency, could it be integrated into the databinding sample perhaps?

@mikke89 mikke89 added data binding enhancement New feature or request labels May 20, 2021
@Dakror
Copy link
Contributor Author

Dakror commented May 21, 2021

The issue definitely is the variant type and non-variadic printf-usage. That's why i had to use the fmt library.

Copy link
Owner

@mikke89 mikke89 left a comment

Choose a reason for hiding this comment

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

Thanks!

Looks good to me, only there is an issue with the XMLParseTools.cpp diff, is it the line endings again?

@Dakror
Copy link
Contributor Author

Dakror commented May 21, 2021

yeah thanks, i dont know why. i have autocrlf off...

@mikke89 mikke89 merged commit 3383c01 into mikke89:master May 21, 2021
@mikke89
Copy link
Owner

mikke89 commented May 21, 2021

I think that should be on if you're on Windows?

@Dakror
Copy link
Contributor Author

Dakror commented May 21, 2021

oh does it? okay

@Dakror Dakror deleted the fix-curly branch May 21, 2021 18:44
mikke89 added a commit that referenced this pull request May 26, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
data binding enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants