search

DALTCORE / lara-pdf-merger

Laravel PDF merger, that works on Laravel 7.0 and PHP 7.4
MIT License
27 stars 36 forks source link

PHP 7.4 - Tcpdf issue #5

Closed code2prog closed 4 years ago

code2prog commented 4 years ago

chr() expects parameter 1 to be int, string given {"userId":1,"exception":"[object] (ErrorException(code: 0): chr() expects parameter 1 to be int, string given at /.../vendor/daltcore/lara-pdf-merger/src/LynX39/LaraPdfMerger/tcpdf/include/tcpdf_fonts.php:1671)

php 7.4 Probably tcpdf 6.3.x fix this issue

RamonSmit commented 4 years ago

Going to try to fix this tonight. If you, in the meanwhile can fix it via a PR, that would be appreciated.

code2prog commented 4 years ago

Going to try to fix this tonight. If you, in the meanwhile can fix it via a PR, that would be appreciated.

I tried, but I came across problems that I couldn't solve with tcpdfi. Wouldn't it be better if tcpdf was in composer.json as a dependency?

RamonSmit commented 4 years ago

Hmm, that does sound like a seriously good idea. Going to try that, gimme a second!

RamonSmit commented 4 years ago

I've pushed it to: https://github.com/DALTCORE/lara-pdf-merger/tree/tcpdf-as-composer-dep but I can't really test it at this point. Do you have any time to test this?

code2prog commented 4 years ago

I've pushed it to: https://github.com/DALTCORE/lara-pdf-merger/tree/tcpdf-as-composer-dep but I can't really test it at this point. Do you have any time to test this?

I will test it in the next 2 hours and let you know if everything works

RamonSmit commented 4 years ago

Thats fine! Hope I'll be at home around 0:00 AM CET, so I can debug/test it a little more too

code2prog commented 4 years ago

Thats fine! Hope I'll be at home around 0:00 AM CET, so I can debug/test it a little more too

so... this library still use deprecated each() function https://github.com/RafikHaceb/tcpdi/blob/master/tcpdi.php line 628

From PdfManage we should remove those lines: require_once(DIR.'/../../../vendor/tecnickcom/tcpdf/tcpdf.php'); require_once(DIR.'/../../../vendor/rafikhaceb/tcpdi/tcpdi.php');

and use composer autoload instead.

code2prog commented 4 years ago

6

RamonSmit commented 4 years ago

Oh thats a great one! Let me merge #6

RamonSmit commented 4 years ago

Merged everything and released under 3.0.0

code2prog commented 4 years ago

Merged everything and released under 3.0.0

did you test it yourself before releasing? I am asking because I updated the code to version 3.0 and I have the error message: Trying to access array offset on value of type bool {"userId":1,"exception":"[object] (ErrorException(code: 0): Trying to access array offset on value of type bool at /.../vendor/wfeller/tcpdi/tcpdi_parser.php:1428)

RamonSmit commented 4 years ago

Just have setup a 7.4 dev box for myself, and seeing what you mean. I shall try to fix.

edit: I have retracted the 3.x release for now

edit 2: I dont see the same errors as you:

Psy Shell v0.9.11 (PHP 7.4.0 — cli) by Justin Hileman
>>> use LynX39\LaraPdfMerger\Facades\PdfMerger;
>>> $pdfMerger = PDFMerger::init();
=> LynX39\LaraPdfMerger\PdfManage {#3708}
>>> $pdfMerger->addPDF('Desktop/Factuur-DC-2019-78.pdf');
=> LynX39\LaraPdfMerger\PdfManage {#3708}
>>> $pdfMerger->addPDF('Desktop/Factuur-DC-2019-77.pdf');
=> LynX39\LaraPdfMerger\PdfManage {#3708}
>>> $pdfMerger->save('Desktop/test.pdf');
=> true
cliffordjames commented 4 years ago

Any updates on this?

Got the same error on PHP 7.4

message: "chr() expects parameter 1 to be int, string given" exception: "ErrorException" file: ".../vendor/daltcore/lara-pdf-merger/src/LynX39/LaraPdfMerger/tcpdf/include/tcpdf_fonts.php" line: 1671

RamonSmit commented 4 years ago

When I have some spare time I will take a look. I'm extremely busy at this moment sadly. If someone has some spare time, please do open a PR for this.

sxzamael commented 4 years ago

Merged everything and released under 3.0.0

did you test it yourself before releasing? I am asking because I updated the code to version 3.0 and I have the error message: Trying to access array offset on value of type bool {"userId":1,"exception":"[object] (ErrorException(code: 0): Trying to access array offset on value of type bool at /.../vendor/wfeller/tcpdi/tcpdi_parser.php:1428)

I modified line 1427, it seems to work. Greetings and thanks. -- $res = $this->_getPageRotation($obj[1][1]['/Parent']); ++ $res = $this->getObjectVal($obj[1][1]['/Parent']);

kohlerdominik commented 4 years ago

This seems a little stuck here... @RamonSmit I prepared everything so you just need to release my fix as 2.0.4 and we have PHP7.4 and Laravel7 compatibility here. I would appreciate if you can release a fixed version.

Besides: a composer dependency based version seems a great improvement for me, too. But probably to be discussed in a new thread?

RamonSmit commented 4 years ago

@kohlerdominik I've pushed to the 2.0 branch now, Can you please take a look if I did it correctly like this? I'm a bit unsure.

kohlerdominik commented 4 years ago

Hi Ramon

your 2.0 branch is correctly created. I can already use it by defining 2.0.x-dev in composer.

I created an additional PR #10 as response to #9. I don't know in which cases we hit curly braces (for me it didn't happen), but i made a regex-search for curly braces and I hope to have found them all.

So can you please merge PR #10 and then make the 2.0.4 release? Thanks very much for your effort.

RamonSmit commented 4 years ago

Hi Dominik,

Merged and tagged 2.0.4 as release, hopefully I did this right.

Also, thank you very much for your effort! 😎.

lewiswilbynm commented 4 years ago

Hi all,

I believe I am still having the same issue on PHP 7.4 using v2.0.5, is this the latest version of the repo with PHP 7.4 fixes?

Error message: Error: Trying to access array offset on value of type bool Line: vendor/daltcore/lara-pdf-merger/src/LynX39/LaraPdfMerger/tcpdf/tcpdi_parser.php:1393

RamonSmit commented 4 years ago

Trying to access array offset on value of type bool

Can try to debug this tonight 😄

RamonSmit commented 4 years ago

I'm a bit unsure how this happens... Need to dive deeper i think

SenatorMatt commented 4 years ago

I'm using version 2.0.5 with PHP 7.4.5 and Laravel v7, and I too get the issue below:

Error: Trying to access array offset on value of type bool Line: vendor/daltcore/lara-pdf-merger/src/LynX39/LaraPdfMerger/tcpdf/tcpdi_parser.php#1393

I've only just got this message after updating to PHP 7.4.5

nreimers commented 4 years ago

I created a fork that fixes the v2 branch here, so that it runs with PHP 7.4. For my local application, it works fine: https://github.com/nreimers/lara-pdf-merger/tree/php7.4-fix

It has still tcpdf included and not as a dependency. But as a quick fix it might be helpful for some.

RamonSmit commented 4 years ago

Merged in 2.0.6

peterjaap commented 3 years ago

Can't we just remove tcpdf from this package and set tecnickcom/tcpdf as a dependency? I mean, this is the whole point of Composer, right?