Bacon / BaconQrCode

QR Code Generator for PHP
BSD 2-Clause "Simplified" License
1.82k stars 208 forks source link

fix: remove iconv conversion #178

Open shyim opened 4 months ago

shyim commented 4 months ago

$encoding is hard-code to ISO-8859-1

converting utf-8 to ISO-8859-1 throws a warning. https://3v4l.org/veXqX

on newer systems even a exception:

PHP Fatal error:  Uncaught BaconQrCode\Exception\WriterException: Could not encode content to ISO-8859-1 in /workspace/BaconQrCode/src/Encoder/Encoder.php:619
Stack trace:
#0 /workspace/BaconQrCode/src/Encoder/Encoder.php(532): BaconQrCode\Encoder\Encoder::append8BitBytes()
#1 /workspace/BaconQrCode/src/Encoder/Encoder.php(82): BaconQrCode\Encoder\Encoder::appendBytes()
#2 /workspace/BaconQrCode/src/Writer.php(46): BaconQrCode\Encoder\Encoder::encode()
#3 /workspace/BaconQrCode/src/Writer.php(61): BaconQrCode\Writer->writeString()
#4 /workspace/BaconQrCode/test.php(16): BaconQrCode\Writer->writeFile()
DASPRiD commented 4 months ago

That would remove that feature completely, which is not desired. I can't find much information about this online, do you have any source for the reason of this issue?

shyim commented 4 months ago

You mean converting utf-8 to ISO-8859-1 ? I have no clue, it throws a fatal error in my docker image 😅

shyim commented 4 months ago

Maybe use mbstring instead of iconv here? https://3v4l.org/2NQjo#v8.2.18

seems to work 🤔

shyim commented 4 months ago

If you don't want require mbstring directly, we can use https://packagist.org/packages/symfony/polyfill-mbstring

DASPRiD commented 4 months ago

Would mbstring introduce a new dependency or is that built into PHP by now?

shyim commented 4 months ago

mbstring is a php extension by php itself, but not installed by default. When it's not installed, we can use the polyfill which is kinda the same but without having the need to install it

DASPRiD commented 4 months ago

I'd like to hear some input from some other people on this topic, before we continue.

williamdes commented 4 months ago

mbstring is a php extension by php itself, but not installed by default. When it's not installed, we can use the polyfill which is kinda the same but without having the need to install it

Not installed by default on Debian for example ? All this years iconv seems for me not as good as mbstring