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

Fix treatment of standalone tags. #15

Merged
merged 5 commits into from
Aug 19, 2014

Conversation

estan
Copy link
Contributor

@estan estan commented Aug 19, 2014

Lines containing ^, /, !, <, or = tags should be stripped from the output if the tags are standalone. The least intrusive way to handle this I could find was to simply "expand" the endpoints of such tags to fill the line - all existing logic will continue to just work.

This fixes 29 spec test failures.

I think all the 13 remaining failures fall into one of these categories:

  • Dotted names syntax is not supported.
  • Implicit iterators is not supported.
  • Partials rendered from standalone > tags should be indented.

I'll might try to tackle the last one next.

Lines containing ^, /, !, <, or = tags should be stripped from the
output if the tags are standalone. The least intrusive way to handle
this I could find was to simply "expand" the endpoints of such tags
to fill the line - all existing logic will continue to just work.

This fixes 29 spec test failures.
It's @p to reference a parameter, not @A.
@@ -386,20 +386,26 @@ Tag Renderer::findTag(const QString& content, int pos, int endPos)
if (typeChar == '#') {
tag.type = Tag::SectionStart;
tag.key = readTagName(content, pos+1, endPos);
expandTag(tag, content);
Copy link
Owner

Choose a reason for hiding this comment

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

Could these duplicated expandTag() calls be replaced with a 'if (tag.type != Tag::Value) { /* expand tag */ }' test after if () else ... block?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good idea. Fixed.

(I guess I was thinking in case a new tag type appears it's less likely to bring in unwanted behavior for it. But that's probably very unlikely.)

*
* A tag is standalone if it is the only non-whitespace token on the the line.
*/
void expandTag(Tag& tag, const QString &content) const;
Copy link
Owner

Choose a reason for hiding this comment

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

Small nit - coding convention in mustache.h/cpp is 'const T&' rather than 'const T &'. This function can also be static since it doesn't read or modify any fields.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

@estan
Copy link
Contributor Author

estan commented Aug 19, 2014

Good to go? I have a fix for standalone partials indentation to follow this up with :)

robertknight added a commit that referenced this pull request Aug 19, 2014
Fix treatment of standalone tags.
@robertknight robertknight merged commit ccec5fc into robertknight:master Aug 19, 2014
@robertknight
Copy link
Owner

Yup - Thanks again @estan!

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.

2 participants