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

Add Gotenberg option generateDocumentOutline to $options array #92

Open
wants to merge 2 commits into
base: 1.x
Choose a base branch
from

Conversation

muratbinerbay
Copy link
Contributor

This should allow creating a document outline based on the HTML heading structure in the PDF file

@kingjia90
Copy link
Contributor

kingjia90 commented Jan 23, 2025

Thank you for the contribution!

Unfortunately, this PR cannot be merged as is, this generateDocumentOutline addition is quite recent (Nov 2024), on Pimcore side it needs to be in a minor release 1.x and the minimum required composer version have to be bumped to from ^2.2 to ^2.10. Another challenge would be to exclude/ignore it in case of gotenberg-php 1.x is required

See also
https://github.com/gotenberg/gotenberg-php/releases/tag/v2.10.0
https://github.com/gotenberg/gotenberg/releases/tag/v8.14.0
gotenberg/gotenberg#1044

@kingjia90 kingjia90 added this to the 1.5.0 milestone Jan 23, 2025
@kingjia90 kingjia90 changed the base branch from 1.4 to 1.x January 23, 2025 11:37
@muratbinerbay
Copy link
Contributor Author

@kingjia90 thank you for checking!

I did not realise the exception for gotenberg-php because we were already on a recent version. But for gotenberg itself, it looks like that will ignore flag if it running a version older than 8.14.0.

Maybe a method_exists check would be more convenient than bumping composer versions for a new flag? It would also make it safer to support new flags in the future.

];

foreach ($options as $option) {
if (isset($params[$option]) && $params[$option] != false) {
if (isset($params[$option]) && $params[$option] != false && method_exists($chromium, $option)) {
Copy link
Contributor

@kingjia90 kingjia90 Jan 23, 2025

Choose a reason for hiding this comment

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

maybe let's move generateDocumentOutline outside the $options array and have a standalone IF block for it with a comment on the fact that on v1 it's not supported, it would be easier to remember for future clean up when for example dropping support to v1 and to avoid checking methodExists on the ones that are surely true

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants