-
Notifications
You must be signed in to change notification settings - Fork 439
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
Update AbstractGenerator.php #470
base: master
Are you sure you want to change the base?
Conversation
stop null warnings/errors when null options are set, noticed these in php8.1
Hey guys, thanks for taking the time to review this, if you believe the errors are due to misuse of the library etc happy for some pushback, but suspect these are worthwhile warning silencers. |
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.
Thank you for the contribution!
$command .= ' ' . $key . ' ' . \escapeshellarg($option); | ||
$command .= ' ' . $key . ' ' . \escapeshellarg($option ?? ''); | ||
} elseif (\in_array($key, ['image-dpi', 'image-quality'])) { | ||
$command .= ' --' . $key . ' ' . (int) $option; | ||
} else { | ||
$command .= ' --' . $key . ' ' . \escapeshellarg($option); | ||
$command .= ' --' . $key . ' ' . \escapeshellarg($option ?? ''); |
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.
There is no need for this as we already check if an option is neither null or false at the beginning.
$command .= ' --' . $key . ' ' . \escapeshellarg($k) . ' ' . \escapeshellarg($v); | ||
$command .= ' --' . $key . ' ' . \escapeshellarg($k) . ' ' . \escapeshellarg($v ?? ''); | ||
} | ||
} else { | ||
foreach ($option as $v) { | ||
$command .= ' --' . $key . ' ' . \escapeshellarg($v); | ||
$command .= ' --' . $key . ' ' . \escapeshellarg($v ?? ''); |
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.
I need to double check on the wkhtmltopdf options but, IMHO, we should simply skip invalid array entries.
I think we should skip options with key/value that are not valid strings.
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.
I'm saying to skip it as it's the current behavior for null/false options.
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
stop null warnings/errors when null options are set, noticed these occuring in php8.1