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 ignoring subtrees in to satisfy #198

Merged
merged 2 commits into from
Mar 18, 2018

Conversation

sunesimonsen
Copy link
Member

You need to use a special comment <!--ignore--> on the rhs. That was the least intrusive solution I could come up with. Using an <ignore/> element does not work as it is not a void element, so you would have to type <ignore></ignore> - that is a little too much ignore :-)

You need to use a special comment `<!--ignore-->` on the rhs. That was the least
intrusive solution I could come up with. Using an `<ignore/>` element does not
work as it is not a void element, so you would have to type `<ignore></ignore>`
- that is a little too much ignore :-)
@sunesimonsen sunesimonsen force-pushed the ssimonsen/ignore-subtrees branch from 18d9071 to b331683 Compare March 18, 2018 22:17
@coveralls
Copy link

coveralls commented Mar 18, 2018

Coverage Status

Coverage increased (+0.02%) to 93.81% when pulling 4617ab5 on ssimonsen/ignore-subtrees into c69265e on master.

Copy link
Member

@papandreou papandreou left a comment

Choose a reason for hiding this comment

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

Looks fine. Since it's not "elastic" I'd like to see how it works in the presence of whitespace text nodes, eg.

<div>
   <span>foo</span>
   <div>something we don't care about</div>
   <span>bar</span>
</div>

If you want to ignore the inner div in the above, I think you'd have to satisfy it against an HTML string where the whitespace between the nodes is exactly the same. So <div><span>foo</span><!--ignore--><span>bar</span></div> wouldn't work, it would have to be:

<div>
   <span>foo</span>
   <ignore>
   <span>bar</span>
</div>

This probably is probably already present with the existing to satisfy, though, but I think it's worse here because you'd assume that the <!--ignore--> eats up the whitespace differences, too.

@@ -1293,6 +1293,50 @@ describe('unexpected-dom', function() {
' ]'
);
});

describe('and it contain an <ignore/> tag', () => {
Copy link
Member

Choose a reason for hiding this comment

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

<!--ignore-->

@sunesimonsen
Copy link
Member Author

Let's wait with documenting this till we know it is a good approach. If we merge it I'll try to use it on some real projects.

@sunesimonsen sunesimonsen merged commit 6ee67c2 into master Mar 18, 2018
@sunesimonsen sunesimonsen deleted the ssimonsen/ignore-subtrees branch March 18, 2018 22:42
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.

3 participants