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

Support some new features from the updated spec #25

Merged
merged 1 commit into from Sep 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion .gitmodules
@@ -1,3 +1,3 @@
[submodule "spec"]
path = spec
url = https://github.com/jbboehr/mustache-spec.git
url = https://github.com/mustache/spec.git
2 changes: 1 addition & 1 deletion src/data.cpp
Expand Up @@ -86,7 +86,7 @@ int Data::isEmpty()
ret = 1;
break;
case Data::TypeString:
if( val == NULL || val->length() <= 0 ) {
if( val == NULL || val->length() <= 0 || *val == "null" ) {
Copy link
Owner

@jbboehr jbboehr Sep 13, 2023

Choose a reason for hiding this comment

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

I think this may be better handled in the specification parsing code, as I don't think it's optimal to generally treat a string containing "null" as empty. Unfortunately, libyaml, being a c, not a c++, library, is fairly low level and doesn't handle any type conversions automatically. See e.g. #18.

This (probably) isn't an issue with the PHP extension as it has it's own conversion code using native PHP types.

I've opened another PR in #26 with this change applied but let me know if you disagree or if it breaks your use-case or w/e.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Makes sense, I think I just misinterpreted the spec there and didn't realize that was only for literal nulls.

ret = 1;
}
break;
Expand Down
2 changes: 2 additions & 0 deletions src/renderer.cpp
Expand Up @@ -184,9 +184,11 @@ void Renderer::_renderNode(Node * node)
switch( val->type ) {
default:
case Data::TypeString:
_stack->push_back(val);
for( Node::Children::iterator it = node->children.begin() ; it != node->children.end(); it++ ) {
_renderNode(*it);
}
_stack->pop_back();
break;
case Data::TypeList:
for( Data::List::iterator childrenIt = val->children.begin() ; childrenIt != val->children.end(); childrenIt++ ) {
Expand Down
7 changes: 7 additions & 0 deletions tests/test_spec.cpp
Expand Up @@ -158,6 +158,11 @@ void mustache_spec_parse_test(yaml_document_t * document, yaml_node_t * node)
if( node->type != YAML_MAPPING_NODE ) {
return;
}

// Support for inheritance and dynamic names is not implemented yet.
if (strcmp(currentSuite, "~inheritance.yml") == 0 || strcmp(currentSuite, "~dynamic-names.yml") == 0) {
return;
}

MustacheSpecTest * test = new MustacheSpecTest;

Expand All @@ -177,6 +182,8 @@ void mustache_spec_parse_test(yaml_document_t * document, yaml_node_t * node)
test->tmpl.assign(valueValue);
} else if( strcmp(keyValue, "expected") == 0 ) {
test->expected.assign(valueValue);
} else if (strcmp(keyValue, "data") == 0) {
mustache_spec_parse_data(document, valueNode, &test->data);
}
} else if( valueNode->type == YAML_MAPPING_NODE ) {
if( strcmp(keyValue, "data") == 0 ) {
Expand Down