-
-
Notifications
You must be signed in to change notification settings - Fork 73
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 PHP Error/Warnning #279
Conversation
Looks great, you can squash your commits 👍 |
9221e1b
to
23aed1e
Compare
Thanks @girishpanchal30! |
if (\count($currentConfig->find_string) !== \count($currentConfig->replace_string)) { | ||
return $currentConfig; | ||
} |
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.
This looks weird to me. Sohuld not we rather return $newConfig
? And should not we check the variables from $newConfig
? I would assume the $currentConfig
has been already validated upon construction.
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.
You're right, I'll move that check in parseLines
instead
@@ -201,8 +201,8 @@ public function process(string $html, string $url, SiteConfig $siteConfig = null | |||
$this->readability = $this->getReadability($html, $url, $parser, $this->siteConfig->tidy() && $smartTidy); | |||
$tidied = $this->readability->tidied; | |||
|
|||
$this->logger->info('Body size after Readability: {length}', ['length' => \strlen((string) $this->readability->dom->saveXML())]); | |||
$this->logger->debug('Body after Readability', ['dom_saveXML' => $this->readability->dom->saveXML()]); | |||
$this->logger->info('Body size after Readability: {length}', ['length' => \strlen((string) $this->readability->dom->saveXML($this->readability->dom->documentElement))]); |
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.
If https://www.php.net/manual/en/domdocument.savexml.php#44422 is correct, this would just save a page from unknown encoding as UTF-8. No idea how would it try to do that, possibly leading to some corruption.
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.
You're right, but that's not really a problem as it's inside the log, maybe the log will only be unreadable?
PHP Warning: array_combine(): Both parameters should have an equal number
PHP Warning: DOMDocument::saveXML(): unknown encoding
PHP Fatal error: Uncaught TypeError: Typed property Graby\Extractor\ContentExtractor::$title must be string or null
issue - #278