johannschopplich / kirby-highlighter

🌐 Server-side syntax highlighting for the code block & KirbyText
https://kirbyseo.com
MIT License
20 stars 4 forks source link

fix: deprecated mb_convert_encoding() #18

Closed bogdancondorachi closed 1 year ago

bogdancondorachi commented 1 year ago

Hi @johannschopplich,

On PHP 8.2, the plugin throws a deprecated error: mb_convert_encoding(): Handling HTML entities via mbstring is deprecated

I found that using the htmlentities() achieves the same result and works as expected.

In addition, you could use htmlspecialchars() that only converts specific characters, not sure what would be the best fit for this.

johannschopplich commented 1 year ago

I'm not a PHP expert, so I'm unsure what the best replacement is. I found:

htmlspecialchars_decode(iconv('UTF-8', 'ISO-8859-1', htmlentities($source, ENT_COMPAT, 'UTF-8')), ENT_QUOTES);

May that suit better?

bogdancondorachi commented 1 year ago

In this case I get the error message iconv(): Detected an illegal character in input string, this could happen that the iconv() function has encountered a character in the input string that it cannot convert to the specified ISO-8859-1 encoding.

Why not using UTF-8 as the target encoding? As it is a more capable encoding that can handle a wide range of characters.

htmlspecialchars_decode(htmlentities($source, ENT_COMPAT, 'UTF-8'), ENT_QUOTES);

This way the special characters are converted to their corresponding HTML entities using htmlentities() function with ENT_COMPAT quote style and UTF-8 character encoding, then the function htmlspecialchars_decode() is used to convert special HTML entities back to their original characters using ENT_QUOTES quote style, without using iconv() function.

What do you think?

johannschopplich commented 1 year ago

Count me in for UTF-8 as the default encoding! Sounds good. Would you mind applying the changes from above to the PR?

bogdancondorachi commented 1 year ago

I've updated the pr with the final solution. Thanks :)

P.S. releasing #chore: remove kirby- prefix from plugin name as well would solve the composer update status that's currently not working

johannschopplich commented 1 year ago

Thank you for the PR and thorough investigation!

johannschopplich commented 1 year ago

@bogdancondorachi New version just hit the market. 🙂