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 support for *-empty-line-before in HTML #2854
Conversation
@@ -59,7 +59,7 @@ const rule = function(expectation, options, context) { | |||
const isNested = atRule.parent !== root; | |||
|
|||
// Ignore the first node | |||
if (atRule === root.first) { | |||
if (atRule === root.first || atRule === atRule.root().first) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add comment around atRule === atRule.root().first
and good work!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gucong3000 Thanks - tests look great. I've made a request out creating a util.
@@ -51,8 +51,12 @@ const rule = function(expectation, options, context) { | |||
return; | |||
} | |||
|
|||
// Ignore the first node | |||
if (rule === root.first) { | |||
if ( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we please move this into a util
, e.g. isFirstNodeOfRoot(node)
as I suspect other rules will need too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@gucong3000 Thanks for making the util change. I've added one query.
@@ -52,7 +53,7 @@ const rule = function(expectation, options, context) { | |||
} | |||
|
|||
// Ignore the first node | |||
if (rule === root.first) { | |||
if (isFirstNodeOfRoot(rule, root)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did you mean to pass root
in as a second parameter?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was a mistake.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
#2853
"No, it's self explanatory."