search

LaravelDaily / laravel-invoices

Laravel package to generate PDF invoices from various customizable parameters
GNU General Public License v3.0
1.4k stars 304 forks source link

Remove setOptions to prevent override config #127

Closed dodaydream closed 2 years ago

dodaydream commented 3 years ago

Dompdf's setOptions will override the settings instead of merging them.

For example, when specify the chroot option in config/dompdf.php, after the setOptions was called, it will be reverted to the default vendor directory.

It seems that the enable_php option was never used, so it could possibly be removed.

dodaydream commented 3 years ago

Reference: https://github.com/barryvdh/laravel-dompdf/blob/4e805f5df26d14f6b9d4acb2bfa4cd67f10e2be3/src/PDF.php#L143 https://github.com/dompdf/dompdf/blob/61c86c04d2a483187ff9f6a73c50d42669be5b4d/src/Dompdf.php#L1377

mc0de commented 2 years ago

@dodaydream actually enable_php is used: https://github.com/LaravelDaily/laravel-invoices/blob/88c472680951acc57ccf179711add7d8dda36821/resources/views/templates/default.blade.php#L374

Otherwise there would be no pagination text on multipage invoices. By fixing one thing, and breaking another one. PR needs to be aware of that, and probably merge options instead of just removing them.

dodaydream commented 2 years ago

Ah, okay, sorry I wasn't aware of that usage. I opened an issue on dompdf, and they told me how to set an options.

I will also try to make a pr on laravel-dompdf to implement this.

https://github.com/dompdf/dompdf/issues/2690

mc0de commented 2 years ago

no problem, all cool :)