name-parser
name-parser copied to clipboard
theiconic
Make `InitialMapper` multibyte proof
Coverage remained the same at 100.0% when pulling 5a32bd3f12e1b921ac147c3df51287a3e2421db4 on scribbr:multibyte-initials into 9a54a713bf5b2e7fd990828147d42de16bf8a253 on theiconic:master.
Hey @rvanlaak . Sorry, been busy - are you still working on this? I was thinking i'm a bit hesitent to blanco-require the mbstring extension as it's not active by default. I would therefore prefer we added an abstraction file (or two) for all string operations - could be a class or just namespaced functions so we can use the mbstring versions when that extension is loaded and the standard one when not and thereby degrade gracefully.
What do you think?
Names by their nature simply can be multibyte. Abstracting out the multibyte implementation might cause problems when users don't get that. I would not advise to do that.
This PR is mostly to give you an idea and get your feedback before we continue implementing fixes and adding test names.
@rvanlaak Please have a look at #39 . I believe this is a good way of doing this while still providing fallback (inspired by the function abstractions in GuzzleHttp).
I'm ok with calling these methods if you decide that name-parser isn't going to be multibyte-proof by default 👍 Could you write a not about that in the README?
Yes, i'm happy to add a note to the README. But i would like to have coverage for all the string functions used in the parser. Do you think you could expand on this and provide a PR that has all the native calls replaced with these?