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

Remove parent divs only once for each ul element #2080

Merged
merged 1 commit into from Jan 5, 2024

Conversation

mateusdeap
Copy link
Contributor

Fixes #2039

The issue was that, in Docs::Cpref::CleanHtmlFilter, we had the following:

        css('div > ul').each do |node|
          node.parent.before(node.parent.children).remove
        end

This was fine for almost all cases, however, the page for std::vector::vector had more than one ul element inside the same div (in the Complexity section).

This meant that, as we iterated over each ul element, because 2 or more had the same parent div, for the first ul with a sibling, we'd correctly pop all it's siblings out of the parent div, but when we came around to the second sibling ul, it's parent div was now some outer div on the page, and that would get removed. This would repeat itself how many sibling ul elements there might be on the page.

I modified the code to simply grab the parent divs, remove the duplicates and then execute the logic to pop out it's children and remove itself.

All tests passed locally, but when I went ahead to add tests to prevent this bug from reappearing, I noticed that there doesn't seem to be any tests for specific scrapers or filters, is that by choice?

@thewheat
Copy link
Contributor

This also sees to fix #1610

@jasonbtiller
Copy link

This looks great. Why hasn't it been merged?

@mateusdeap
Copy link
Contributor Author

mateusdeap commented Dec 9, 2023

I do not know hahaha I just assumed that, eventually, the maintainer would se it. Do you know who I need to ping, maybe?

Copy link
Contributor

@simon04 simon04 left a comment

Choose a reason for hiding this comment

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

Thank you!

@simon04 simon04 merged commit 6c81f97 into freeCodeCamp:main Jan 5, 2024
1 check passed
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.

Documentation on C++ std::vector::vector shows no content
4 participants