Skip to content

Update src/EasySVG.php#22

Open
EzikMarconi wants to merge 2 commits intokartsims:masterfrom
EzikMarconi:master
Open

Update src/EasySVG.php#22
EzikMarconi wants to merge 2 commits intokartsims:masterfrom
EzikMarconi:master

Conversation

@EzikMarconi
Copy link
Copy Markdown

new line symbol - bug fix

new line symbol - bug fix
This library did not work correctly with multibyte characters.

I made the appropriate correct operation with multibyte text.
Before PHP 7.2.0, you need to use the utf8_ord () alternative function.
If you have PHP version 7.2.0+, you can limit the work to the mb_ord () function.

I also attach a font to check the work.
@kartsims
Copy link
Copy Markdown
Owner

kartsims commented Oct 8, 2018

please explain the purpose of this PR

I see many things :

  • a new font
  • indentation changed everywhere (why?!)
  • @author tag changed to your own name (is this a significant modification ?)
  • a new UTF8 mapping function : this seems to be the original reason for your PR, could you please elaborate ?

@EzikMarconi
Copy link
Copy Markdown
Author

EzikMarconi commented Oct 8, 2018

Hello.
I installed your library and discovered that it does not work with multibyte encodings. I can show screenshots of what happened. The problem was that the ORD () PHP function does not work correctly with multibyte characters. The function does not return the character code correctly.

Text from the documentation: " However, note that this function is not aware of any string encoding, and in particular will never identify a Unicode code point in a multi-byte encoding such as UTF-8 or UTF-16." http://php.net/manual/en/function.ord.php

Currently in PHP Does not have a valid function. Therefore, I wrote the function function utf8_ord for proper operation. The problem has been fixed.

I also attached a free font that contains Cyrillic characters. Script works can be checked.

Sorry for changing indents. Apparently my code editor did this automatically :(

@ricardoboss
Copy link
Copy Markdown
Collaborator

@EzikMarconi do you still want to merge this? Otherwise, this will be closed.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants