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 #225 Improve compiler error for attributes with missing quotes #307

Closed
wants to merge 1 commit into from

Conversation

marclaval
Copy link
Contributor

This PR adds error messages for wrong attributes of HTML elements and components.
Please have a look at the tests first.

It doesn't takes care of allowing single quotes, @olaf-k is working on this part.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.08%) when pulling 4a27f25 on mlaval:issue225 into 711d77d on ariatemplates:master.

@@ -191,22 +191,58 @@ HTMLName
{return first + next.join("");}

HTMLAttName
= first:[a-zA-Z#] next:([a-zA-Z] / [0-9] / "-")* endString:(":" end:([a-zA-Z] / [0-9] / "-")+ {return ":" + end.join("")})?
= first:[a-zA-Z#] next:([a-zA-Z] / [0-9] / "-" / "_")* endString:(":" end:([a-zA-Z] / [0-9] / "-" / "_")+ {return ":" + end.join("")})?
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need _ in attributes name ? Sounds weird to me as it is not standard... - would be recommanded wherever you are tempted to use an _

Copy link
Member

Choose a reason for hiding this comment

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

Ok, got it... 😃 !!

@benouat
Copy link
Member

benouat commented Sep 29, 2014

This is fine for me !

this._logError(msg, error);
}
continue;
}
Copy link
Collaborator

Choose a reason for hiding this comment

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

Just a note, I haven't read the whole commit, but first thing I noticed is that _elementOrComponent becomes a 160-liner after this change. Maybe it would make sense to split some parts of it using helper functions? (either now or in a separate refactor commit).

@coveralls
Copy link

Coverage Status

Coverage increased (+0.08%) when pulling 8e8334d on mlaval:issue225 into d91db78 on ariatemplates:master.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.08%) when pulling d82c62f on mlaval:issue225 into a9710a4 on ariatemplates:master.

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.

None yet

4 participants