Re: KEncodingProber vs "unexisting" codecs
On Friday 03 July 2015 01:18:33 Albert Astals Cid wrote: > Question is, where do we want that? Do we want it on probing time so we > don't try to probe codecs that don't exist (for this installation), or > should we actually mention in the docs that it may happen that the returned > codec does not exist and thus you should check the result in the "caller" > side? Hmm, good point. If we get a "correct" result from the prober, but pointing to an unavailable codec, the user is screwed. If on the other hand we return a "second best guess" which is actually available, the user gets something. It's not like we have knewstuff for codecs, recompiling Qt is a little bit out of scope for most users :-) So I guess it should be checked at probing time. -- David Faure, fa...@kde.org, http://www.davidfaure.fr Working on KDE Frameworks 5 ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: KEncodingProber vs "unexisting" codecs
El Dissabte, 6 de juny de 2015, a les 10:30:52, David Faure va escriure: > On Sunday 24 May 2015 07:18:21 Albert Astals Cid wrote: > > One of the codecs that KEncodingProber can return is x-euc-tw > > > > But neither the Qt4 nor the Qt5 installed in my distribution understand > > that codec, so doing the suggested > > > > QTextCodec::codecForName(prober.encoding())->toUnicode(data); > > > > Will crash. > > Qt's ICU codec indeed doesn't support euc-tw, because it's not even > registered at IANA: > http://www.iana.org/assignments/character-sets/character-sets.xhtml > (and that's what qicucodec.cpp uses as the base list) > > I would just remove it from the prober. I just removed it. > You are right, however, that a runtime check is needed anyway, because one > can compile Qt without ICU support, and then the list of supported codecs > might be different. Question is, where do we want that? Do we want it on probing time so we don't try to probe codecs that don't exist (for this installation), or should we actually mention in the docs that it may happen that the returned codec does not exist and thus you should check the result in the "caller" side? Cheers, Albert ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: KEncodingProber vs "unexisting" codecs
On Sunday 24 May 2015 07:18:21 Albert Astals Cid wrote: > One of the codecs that KEncodingProber can return is x-euc-tw > > But neither the Qt4 nor the Qt5 installed in my distribution understand that > codec, so doing the suggested > > QTextCodec::codecForName(prober.encoding())->toUnicode(data); > > Will crash. Qt's ICU codec indeed doesn't support euc-tw, because it's not even registered at IANA: http://www.iana.org/assignments/character-sets/character-sets.xhtml (and that's what qicucodec.cpp uses as the base list) I would just remove it from the prober. You are right, however, that a runtime check is needed anyway, because one can compile Qt without ICU support, and then the list of supported codecs might be different. -- David Faure, fa...@kde.org, http://www.davidfaure.fr Working on KDE Frameworks 5 ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: KEncodingProber vs "unexisting" codecs
El Diumenge, 24 de maig de 2015, a les 16:57:38, Aleix Pol va escriure: > On Sun, May 24, 2015 at 7:18 AM, Albert Astals Cid wrote: > > One of the codecs that KEncodingProber can return is x-euc-tw > > > > But neither the Qt4 nor the Qt5 installed in my distribution understand > > that codec, so doing the suggested > > > > QTextCodec::codecForName(prober.encoding())->toUnicode(data); > > > > Will crash. > > > > Not good. > > > > Two ways to fix this that i can think of: > > * Go through all the probers and remove the ones that "don't exist" in Qt > > * On runtime when creating the prober check if Qt understands it before > > > > testing if the data matches it > > > > The first is defenitely faster on execution time but is also less > > versatile > > and i don't know enough about QTextCoded internals to be sure all the > > codecs that are available in my ditro are available on distro X and > > viceversa, so maybe option 2 is better. > > > > Opinions and comments before i start implementing? > > > > Cheers, > > > > Albert > > > > ___ > > Kde-frameworks-devel mailing list > > Kde-frameworks-devel@kde.org > > https://mail.kde.org/mailman/listinfo/kde-frameworks-devel > > Hi, > Wouldn't that make the code slightly worse? I mean, if we can detect > the codec properly, removing that feature because Qt doesn't support > it seems odd. Well basically the only purpose of the class is giving you a codec name to use on QTextCodec, so it's not that weird. > Maybe we could create a wrapper function in KEncodingProber that > returns the codec? Something like: KEncodingProber.codec(). Then you > can check if it returns nullptr over there and use whatever feels > standard (and even notify somewhere that it wasn't found). The problem with that solution is that it does not fix all the code we already have with the suggested "typical use" from crashing, that's why i'd prefer to fix that instead of adding ::codec. If we end up agreeing on implementing a ::codec, I think it should not return QTextCodec::codecForName(encoding()) and if null return QTextCodec::codecForName(default), it should return the existing-most-matching codec for the feeded data. FWIW this is motivated by https://bugs.kde.org/show_bug.cgi?id=346129 Cheers, Albert > > Aleix > ___ > Kde-frameworks-devel mailing list > Kde-frameworks-devel@kde.org > https://mail.kde.org/mailman/listinfo/kde-frameworks-devel ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: KEncodingProber vs "unexisting" codecs
On Sun, May 24, 2015 at 7:18 AM, Albert Astals Cid wrote: > One of the codecs that KEncodingProber can return is x-euc-tw > > But neither the Qt4 nor the Qt5 installed in my distribution understand that > codec, so doing the suggested > > QTextCodec::codecForName(prober.encoding())->toUnicode(data); > > Will crash. > > Not good. > > Two ways to fix this that i can think of: > * Go through all the probers and remove the ones that "don't exist" in Qt > * On runtime when creating the prober check if Qt understands it before > testing if the data matches it > > The first is defenitely faster on execution time but is also less versatile > and i don't know enough about QTextCoded internals to be sure all the codecs > that are available in my ditro are available on distro X and viceversa, so > maybe option 2 is better. > > Opinions and comments before i start implementing? > > Cheers, > Albert > ___ > Kde-frameworks-devel mailing list > Kde-frameworks-devel@kde.org > https://mail.kde.org/mailman/listinfo/kde-frameworks-devel Hi, Wouldn't that make the code slightly worse? I mean, if we can detect the codec properly, removing that feature because Qt doesn't support it seems odd. Maybe we could create a wrapper function in KEncodingProber that returns the codec? Something like: KEncodingProber.codec(). Then you can check if it returns nullptr over there and use whatever feels standard (and even notify somewhere that it wasn't found). Aleix ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
