Guillaume Munch wrote: > I made it work with libc++ too, which was not straightforward. In this > case, the template is undefined and cannot be inherited from. > > Could you or somebody else please double-check the following, which I am > not sure to understand correctly: > > * Why ascii_num_get_facet::do_get uses from_local8bit at some point.
The encoding does not matter here: Our own numpunct_facet does not override truename() and falsename(), so std::numpunct::truename() and std::numpunct::falsename() are used. I don't know why Enrico chose from_local8bit and not from_ascii() here. > * Is the numpunct<lyx::char_type> facet correctly set with the way it > occurs in the code? I think so. Otherwise we would have the classical separator problem, e.g. outputting 2.3 as "2,3" when using a german locale. Do you see any specific problem? >> I would like it more simple as well, but it seems this is not easily >> possible without a C++11 compliant std::string. > > I agree for std::string, but for docstring I have demonstrated that it > is very simple. Moreover I find it reassuring that versa_string is the > precursor of the new basic_string. If you look at docstring alone I agree, but if you look at LyX as a whole this would mean to use 3 string base classes (std::basic_string, ext::versa_string and lyx::trivial_string) instead of two (std::basic_string and lyx::trivial_string). This is more complicated IMHO. Besides that I do not trust versa_string, e.g. bug https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67557 is likely to exist in gcc older than 5.1. If trivstring had a (yet unknown) bug this would be annoying, but its usage is mostly limited to work done in threads (e.g. export), so data loss is not that likely. If we based our central workhorse docstring on a class which we do not know how widely it is used and what bugs might exist then I would be nervous. > There is also a built-in conversion function between utf-8 and utf-32 in > C++11 which could replace iconv. But I don't know whether there are > clear advantages in replacing iconv. Iconv might still be needed for > other encodings. We definitely need other conversions. Even if we decided to drop support for non-utf8 LaTeX output (which we might do one day) we would still need it for importing .tex files. > But then I don't understand why for from_local8bit it > uses QString::fromLocal8Bit instead of iconv. Because it is easier. If you want to use iconv, you need to know the name of the local encoding, and determining it is platform dependent. With qt all this is nicely abstracted away. >>>> The only thing I would do differently is the replacement of >>>> empty_string() and empty_docstring(): Those were introduced to avoid >>>> including <string>. Since <string> is now required, it would be better >>>> to initialize the default arguments with docstring() or std::string(). >>> >>> There is not much a difference from what I did, other than matters of >>> taste, is there? >> >> There is an additional conversion from char * to std::string. I was once >> told that this is less efficient (here on the LyX mailing list, but I >> forgot who wrote that), but I never tested that myself. > > Surely, but the combined amount of time and energy spent in this > conversion, over all user's computers present and future, is surely less > than the time and energy spent writing this reply... Well, having these changes combined with the string type changes in one patch causes all interested persons to spend more time. If you do not want comments on the empty_string changes then please do such changes in a separate patch next time (the string type changes can be done independently of removing empty_string() and empty_docstring(). Unfortunately I cannot try the patch, since I get undefined refernces to the std::codecvt functions that we previously defined for gcc on windows: g++ -fPIC -O2 -std=c++14 -std=c++14 -fmessage-length=0 -g -Wunused-result -Wuninitialized -Winit-self -o lyx main.o BiblioInfo.o Box.o Compare.o Dimension.o EnchantChecker.o PersonalWordList.o LaTeXFonts.o PrinterParams.o Thesaurus.o liblyxcore.a liblyxmathed.a liblyxinsets.a frontends/liblyxfrontends.a frontends/qt4/liblyxqt4.a liblyxgraphics.a support/liblyxsupport.a -lmythes-1.2 -lenchant -lmagic -lz - lQt5Concurrent -lQt5Svg -lQt5Widgets -lQt5X11Extras -lQt5Gui -lQt5Core -lxcb support/liblyxsupport.a(docstream.o): (.data.rel.ro._ZTVSt7codecvtIDic11__mbstate_tE[_ZTVSt7codecvtIDic11__mbstate_tE]+0x20): undefined reference to `std::codecvt<char32_t, char, __mbstate_t>::do_out(__mbstate_t&, char32_t const*, char32_t const*, char32_t const*&, char*, char*, char*&) const' support/liblyxsupport.a(docstream.o): (.data.rel.ro._ZTVSt7codecvtIDic11__mbstate_tE[_ZTVSt7codecvtIDic11__mbstate_tE]+0x28): undefined reference to `std::codecvt<char32_t, char, __mbstate_t>::do_unshift(__mbstate_t&, char*, char*, char*&) const' support/liblyxsupport.a(docstream.o): (.data.rel.ro._ZTVSt7codecvtIDic11__mbstate_tE[_ZTVSt7codecvtIDic11__mbstate_tE]+0x30): undefined reference to `std::codecvt<char32_t, char, __mbstate_t>::do_in(__mbstate_t&, char const*, char const*, char const*&, char32_t*, char32_t*, char32_t*&) const' support/liblyxsupport.a(docstream.o): (.data.rel.ro._ZTVSt7codecvtIDic11__mbstate_tE[_ZTVSt7codecvtIDic11__mbstate_tE]+0x38): undefined reference to `std::codecvt<char32_t, char, __mbstate_t>::do_encoding() const' support/liblyxsupport.a(docstream.o): (.data.rel.ro._ZTVSt7codecvtIDic11__mbstate_tE[_ZTVSt7codecvtIDic11__mbstate_tE]+0x40): undefined reference to `std::codecvt<char32_t, char, __mbstate_t>::do_always_noconv() const' support/liblyxsupport.a(docstream.o): (.data.rel.ro._ZTVSt7codecvtIDic11__mbstate_tE[_ZTVSt7codecvtIDic11__mbstate_tE]+0x48): undefined reference to `std::codecvt<char32_t, char, __mbstate_t>::do_length(__mbstate_t&, char const*, char const*, unsigned long) const' support/liblyxsupport.a(docstream.o): (.data.rel.ro._ZTVSt7codecvtIDic11__mbstate_tE[_ZTVSt7codecvtIDic11__mbstate_tE]+0x50): undefined reference to `std::codecvt<char32_t, char, __mbstate_t>::do_max_length() const' collect2: error: ld returned 1 exit status Makefile:2142: recipe for target 'lyx' failed Georg
