ArthurSonzogni / FTXUI

:computer: C++ Functional Terminal User Interface. :heart:
MIT License
6.64k stars 399 forks source link

Drop support for wstring and use utf-8 only? #153

Closed cmorganBE closed 3 years ago

cmorganBE commented 3 years ago

Thoughts on dropping wstring and switching to using utf-8 only? It looks like you've got the utf-8 parser in the code. The use of wstring is interesting, I haven't seen it before, but I'm not sure who its aimed at. The windows unicode stuff is, I thought, deprecated at this point in favor of utf-8. Is there any benefit to using wstring over a normal utf-8 string?

ArthurSonzogni commented 3 years ago

Initially, this was for having an easy way to break a string into multiple characters. There is one character per element of the wstring => simple. With a simple std::string, a character might span multiple elements.

Then, it turned out you can have a single character made from multiple wchar_t. That's because of combining characters. Since we are supporting combining characters, then there is not a strong reason to keep working on std::wstring. I just would like to avoid breaking existing users as much as I can.

I momentarily tried that here: https://github.com/ArthurSonzogni/FTXUI/pull/121#discussion_r654918223

but never when beyond a simple prototype.

cmorganBE commented 3 years ago

Ahh gotcha.

I try to see why utf-16 sounded like a good idea but its still unclear to me. utf-8 is such a cleaner approach.

imo the simplest approach is to break existing users vs. trying to introduce wstring->string conversions all over the place. They'd have to drop L everywhere and switch some types but then we'd be utf-8 without using wstring.

BurningEnlightenment commented 3 years ago

Have you thought about supporting char8_t on the surface API when C++20 is available?

I try to see why ~utf-16~ utf-32 sounded like a good idea

sizeof(wchar_t) == 4 holds on most platforms with windows being the notable exception 😉

ArthurSonzogni commented 3 years ago

Have you thought about supporting char8_t on the surface API when C++20 is available?

Requiring C++20 would prevents many people from using the library. Would char8_t bring something useful to FTXUI?

Anyway, I don't believe FTXUI use char in its API:

grep "char " -r include

return nothing.

Of course, the API is full of std::string. I think I am going to keep it, since that the most universal string type in C++.

ArthurSonzogni commented 3 years ago

@cmorganBE, I addressed this issue. FTXUI still support std::wstring, but this is deprecated and I will remove them later to let developers time.

ArthurSonzogni commented 3 years ago

Fixed.