> On Nov. 18, 2014, 10:58 p.m., David Faure wrote: > > src/kcodecs.h, line 786 > > <https://git.reviewboard.kde.org/r/121047/diff/1-2/?file=327084#file327084line786> > > > > should be private, maybe? or is it used by subclasses?
The d pointers are actually accessed by implementations, so it has to be protected. - Daniel ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121047/#review70616 ----------------------------------------------------------- On Nov. 18, 2014, 2:31 p.m., Daniel Vrátil wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/121047/ > ----------------------------------------------------------- > > (Updated Nov. 18, 2014, 2:31 p.m.) > > > Review request for KDE Frameworks, KDEPIM-Libraries and David Faure. > > > Repository: kcodecs > > > Description > ------- > > KCodecs currently provides only decodeRFC2047String(), and even that has a > comment saying "more robust implementation available in KMime". This patch > makes KCodecs::decodeRFC2047String() use the "more robust" implementation > from KMime, and adds KCodecs::encodeRFC2047String() counterpart - also using > implementation from KMime. > > Since KMime also provides codecs implementing various MIME encodings > (quoted-printable, base64, uuencode), and the KMmime implementation of > decodeRFC2047String() depends on them, I also brought over all the MIME > codecs, and make use of them in all the KCodecs static methods. I believe > that having a proper and more robust implementation available in a more > generic framework like KCodecs is better, than if it was hidden in the KMime > framework. As a result, we can make KMime framework to use KCodecs (it > already depends on it anyway) for all the encodings, and only leave classes > for storing and parsing the "high-level" message/rfc822 messages, and > headers. It also reduces code duplication, and makes all applications use the > same encode/decode implementations (which is cool, right? :-)) > > The main reason for this move is that I'm trying to move over to KCodecs one > small class from KPimUtils framework in kdepimlibs (which is our universal > dumping ground in kdepimlibs that I'm trying to get rid of) for generic email > address validation, but it depends on proper implementations of > encode/decodeRFC2047String methods, so I ended up moving all this stuff :-) > The tiny Email class is not part of this review though, I'll wait for this to > get in, then post another review. I believe that this is a rather useful tool > that could find wide adoption outside PIM, therefor I decided not to move it > from KPimUtils to KMime, but instead move all the interesting, and > multi-purpose parts from KMime into KCodecs. > > I also moved over related KMime unit-tests (that's why the review is so big). > > > Diffs > ----- > > autotests/data/codec_x-kmime-rfc2231/basic-encode.expected PRE-CREATION > autotests/data/codec_x-kmime-rfc2231/null-decode.x-kmime-rfc2231 > PRE-CREATION > autotests/data/codec_x-kmime-rfc2231/null-decode.x-kmime-rfc2231.expected > PRE-CREATION > autotests/data/codec_x-kmime-rfc2231/null-encode PRE-CREATION > autotests/data/codec_x-kmime-rfc2231/null-encode.expected PRE-CREATION > autotests/data/codec_x-uuencode/basic-decode.x-uuencode PRE-CREATION > autotests/data/codec_x-uuencode/basic-decode.x-uuencode.expected > PRE-CREATION > autotests/kcodecstest.h PRE-CREATION > autotests/kcodecstest.cpp PRE-CREATION > autotests/rfc2047test.h PRE-CREATION > autotests/rfc2047test.cpp PRE-CREATION > src/CMakeLists.txt adc0f2a > src/kcodecs.h 48effbb > src/kcodecs.cpp 4fd660d > src/kcodecs_p.h PRE-CREATION > src/kcodecsbase64.h PRE-CREATION > src/kcodecsbase64.cpp PRE-CREATION > src/kcodecsidentity.h PRE-CREATION > src/kcodecsidentity.cpp PRE-CREATION > src/kcodecsqp.h PRE-CREATION > src/kcodecsqp.cpp PRE-CREATION > src/kcodecsuuencode.h PRE-CREATION > src/kcodecsuuencode.cpp PRE-CREATION > autotests/CMakeLists.txt 4c41ba2 > autotests/base64benchmark.cpp PRE-CREATION > autotests/codectest.h PRE-CREATION > autotests/codectest.cpp PRE-CREATION > autotests/data/codec_b/basic-decode.b PRE-CREATION > autotests/data/codec_b/basic-decode.b.expected PRE-CREATION > autotests/data/codec_b/basic-encode PRE-CREATION > autotests/data/codec_b/basic-encode.expected PRE-CREATION > autotests/data/codec_b/null-decode.b PRE-CREATION > autotests/data/codec_b/null-decode.b.expected PRE-CREATION > autotests/data/codec_b/null-encode PRE-CREATION > autotests/data/codec_b/null-encode.expected PRE-CREATION > autotests/data/codec_b/padding0-encode PRE-CREATION > autotests/data/codec_b/padding0-encode.expected PRE-CREATION > autotests/data/codec_b/padding1-encode PRE-CREATION > autotests/data/codec_b/padding1-encode.expected PRE-CREATION > autotests/data/codec_b/padding2-encode PRE-CREATION > autotests/data/codec_b/padding2-encode.expected PRE-CREATION > autotests/data/codec_base64/basic-decode.base64 PRE-CREATION > autotests/data/codec_base64/basic-decode.base64.expected PRE-CREATION > autotests/data/codec_base64/basic-encode PRE-CREATION > autotests/data/codec_base64/basic-encode.expected PRE-CREATION > autotests/data/codec_base64/corrupt.base64 PRE-CREATION > autotests/data/codec_base64/corrupt.base64.expected PRE-CREATION > autotests/data/codec_base64/very_small-encode PRE-CREATION > autotests/data/codec_base64/very_small-encode.expected PRE-CREATION > autotests/data/codec_q/all-encoded-decode.q PRE-CREATION > autotests/data/codec_q/all-encoded-decode.q.expected PRE-CREATION > autotests/data/codec_q/basic-encode PRE-CREATION > autotests/data/codec_q/basic-encode.expected PRE-CREATION > autotests/data/codec_q/null-decode.q PRE-CREATION > autotests/data/codec_q/null-decode.q.expected PRE-CREATION > autotests/data/codec_q/null-encode PRE-CREATION > autotests/data/codec_q/null-encode.expected PRE-CREATION > autotests/data/codec_quoted-printable/basic-decode.quoted-printable > PRE-CREATION > > autotests/data/codec_quoted-printable/basic-decode.quoted-printable.expected > PRE-CREATION > autotests/data/codec_quoted-printable/basic-encode PRE-CREATION > autotests/data/codec_quoted-printable/basic-encode.expected PRE-CREATION > autotests/data/codec_quoted-printable/corrupt.quoted-printable PRE-CREATION > autotests/data/codec_quoted-printable/corrupt.quoted-printable.expected > PRE-CREATION > autotests/data/codec_quoted-printable/corrupt2.quoted-printable > PRE-CREATION > autotests/data/codec_quoted-printable/corrupt2.quoted-printable.expected > PRE-CREATION > autotests/data/codec_quoted-printable/corrupt3.quoted-printable > PRE-CREATION > autotests/data/codec_quoted-printable/corrupt3.quoted-printable.expected > PRE-CREATION > autotests/data/codec_quoted-printable/corrupt4.quoted-printable > PRE-CREATION > autotests/data/codec_quoted-printable/corrupt4.quoted-printable.expected > PRE-CREATION > autotests/data/codec_quoted-printable/wrap-encode PRE-CREATION > autotests/data/codec_quoted-printable/wrap-encode.expected PRE-CREATION > autotests/data/codec_x-kmime-rfc2231/all-encoded.x-kmime-rfc2231-decode > PRE-CREATION > > autotests/data/codec_x-kmime-rfc2231/all-encoded.x-kmime-rfc2231-decode.expected > PRE-CREATION > autotests/data/codec_x-kmime-rfc2231/basic-encode PRE-CREATION > > Diff: https://git.reviewboard.kde.org/r/121047/diff/ > > > Testing > ------- > > Builds, all unit-tests pass. > > > Thanks, > > Daniel Vrátil > >
_______________________________________________ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
