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.

Sign up for GitHub

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

Jump to bottom

Improve Updater to not remove useful README titles #770

Merged
merged 5 commits into from Mar 6, 2017
Merged

Improve Updater to not remove useful README titles #770

Changes from 2 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
3ea99fe
Improve Updater to not remove useful README titles
javiereguiluz Feb 24, 2017
08e772c
Remove h2 if needed too
javiereguiluz Feb 24, 2017
c0898b1
Improved code
javiereguiluz Feb 24, 2017
70de25c
Fixed code (thanks to the failing test)
javiereguiluz Feb 24, 2017
efde14c
Added a new test for the proposed feature
javiereguiluz Feb 24, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
7 changes: 4 additions & 3 deletions src/Packagist/WebBundle/Package/Updater.php
View file
Open in desktop
Expand Up @@ -629,11 +629,12 @@ private function prepareReadme($readme, $isGithub = false, $owner = null, $repo
}
}

// remove first title as it's usually the project name which we don't need
if ($dom->getElementsByTagName('h1')->length) {
// remove first page element if it's a <h1> or <h2>, because it's usually
// the project name or the `README` string which we don't need
if ('h1' === $dom->childNodes->item(0)->nodeName) {
$first = $dom->getElementsByTagName('h1')->item(0);
Copy link
Contributor

Choose a reason for hiding this comment

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

could be simplified:

$first = $dom->childNodes->item(0);

if ($first && ('h1' === $first->nodeName || 'h2' === $first->nodeName)) {
    $first->parentNode->removeChild($first);
}

My snippet also account for the case of empty readme, where there could be no first node.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Much better! Thank you.

$first->parentNode->removeChild($first);
} elseif ($dom->getElementsByTagName('h2')->length) {
} elseif ('h2' === $dom->childNodes->item(0)->nodeName) {
$first = $dom->getElementsByTagName('h2')->item(0);
$first->parentNode->removeChild($first);
}
Expand Down