D15714: add a string suffix to test data and use for unicode testing of taglibwriter

2018-10-10 Thread Alexander Stippich
This revision was automatically updated to reflect the committed changes. astippich marked an inline comment as done. Closed by commit R286:196b1289152c: add a string suffix to test data and use for unicode testing of taglibwriter (authored by astippich). CHANGED PRIOR TO COMMIT

D15714: add a string suffix to test data and use for unicode testing of taglibwriter

2018-10-10 Thread Alexander Stippich
astippich marked an inline comment as done. astippich added inline comments. INLINE COMMENTS > bruns wrote in taglibwritertest.cpp:75 > can you change this to `data[2]` ... > Facepalm myself ... I should have spotted this myself :) REPOSITORY R286 KFileMetaData BRANCH taglib_write_unicode

D15714: add a string suffix to test data and use for unicode testing of taglibwriter

2018-10-10 Thread Alexander Stippich
astippich edited the summary of this revision. REPOSITORY R286 KFileMetaData BRANCH taglib_write_unicode REVISION DETAIL https://phabricator.kde.org/D15714 To: astippich, mgallien, bruns Cc: smithjd, svuorela, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun,

D15714: add a string suffix to test data and use for unicode testing of taglibwriter

2018-10-09 Thread Stefan Brüns
bruns accepted this revision. bruns added a comment. This revision is now accepted and ready to land. Otherwise good to go ... INLINE COMMENTS > taglibwritertest.cpp:75 > +// source encoding: "€µ" > +static const QChar data[4] = { 0x20ac, 0xb5 }; > +QString

D15714: add a string suffix to test data and use for unicode testing of taglibwriter

2018-09-29 Thread Alexander Stippich
astippich marked 5 inline comments as done. REPOSITORY R286 KFileMetaData REVISION DETAIL https://phabricator.kde.org/D15714 To: astippich, mgallien, bruns Cc: smithjd, svuorela, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, abrahams

D15714: add a string suffix to test data and use for unicode testing of taglibwriter

2018-09-29 Thread Alexander Stippich
astippich updated this revision to Diff 42541. astippich added a comment. - fix after rebase REPOSITORY R286 KFileMetaData CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D15714?vs=42525=42541 BRANCH taglib_write_unicode REVISION DETAIL https://phabricator.kde.org/D15714

D15714: add a string suffix to test data and use for unicode testing of taglibwriter

2018-09-28 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > taglibwritertest.cpp:152 > +<< QStringLiteral("audio/opus") > +<< QStringLiteral("€") > ; ^ unicodeTestStringSuffix ? REPOSITORY R286 KFileMetaData REVISION DETAIL https://phabricator.kde.org/D15714 To: astippich,

D15714: add a string suffix to test data and use for unicode testing of taglibwriter

2018-09-28 Thread Stefan Brüns
bruns added a comment. +1 @svuorela ? REPOSITORY R286 KFileMetaData REVISION DETAIL https://phabricator.kde.org/D15714 To: astippich, mgallien, bruns Cc: smithjd, svuorela, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns, abrahams

D15714: add a string suffix to test data and use for unicode testing of taglibwriter

2018-09-28 Thread Alexander Stippich
astippich updated this revision to Diff 42525. astippich added a comment. - use suggested test pattern REPOSITORY R286 KFileMetaData CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D15714?vs=42270=42525 BRANCH taglib_write_unicode REVISION DETAIL

D15714: add a string suffix to test data and use for unicode testing of taglibwriter

2018-09-28 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > astippich wrote in taglibwritertest.cpp:73 > Can this give failures on windows similar to D14122 > ? Yes, probably. Although, this gives also a pointer to a different approach: // Add some unicode characters,

D15714: add a string suffix to test data and use for unicode testing of taglibwriter

2018-09-28 Thread Alexander Stippich
astippich added inline comments. INLINE COMMENTS > bruns wrote in taglibwritertest.cpp:73 > If safeguarding against bad editors is really necessary, better do it here. > Alternatively, you can use C++11 unicode string literals, e.g. > `QString(u"€")` > If you save that one as e.g. latin1 and try

D15714: add a string suffix to test data and use for unicode testing of taglibwriter

2018-09-27 Thread Stefan Brüns
bruns requested changes to this revision. This revision now requires changes to proceed. REPOSITORY R286 KFileMetaData REVISION DETAIL https://phabricator.kde.org/D15714 To: astippich, mgallien, bruns Cc: smithjd, svuorela, kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich,

D15714: add a string suffix to test data and use for unicode testing of taglibwriter

2018-09-25 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > taglibwritertest.cpp:60 > > -QCOMPARE(extractedTitle, QStringLiteral("Title1")); > -QCOMPARE(extractedArtist, QStringLiteral("Artist1")); > -QCOMPARE(extractedAlbum, QStringLiteral("Album1")); > +QCOMPARE(extractedTitle,

D15714: add a string suffix to test data and use for unicode testing of taglibwriter

2018-09-24 Thread Alexander Stippich
astippich added inline comments. INLINE COMMENTS > smithjd wrote in taglibwritertest.cpp:60 > Is wrapping in a QString necessary? It does not compile otherwise. REPOSITORY R286 KFileMetaData REVISION DETAIL https://phabricator.kde.org/D15714 To: astippich, mgallien, bruns Cc: smithjd,

D15714: add a string suffix to test data and use for unicode testing of taglibwriter

2018-09-24 Thread Alexander Stippich
astippich updated this revision to Diff 42270. astippich added a comment. - actually use temporary string variable for unicode REPOSITORY R286 KFileMetaData CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D15714?vs=42264=42270 BRANCH taglib_write_unicode REVISION DETAIL

D15714: add a string suffix to test data and use for unicode testing of taglibwriter

2018-09-24 Thread Alexander Stippich
astippich updated this revision to Diff 42264. astippich added a comment. - implement feedback REPOSITORY R286 KFileMetaData CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D15714?vs=42197=42264 BRANCH taglib_write_unicode REVISION DETAIL https://phabricator.kde.org/D15714

D15714: add a string suffix to test data and use for unicode testing of taglibwriter

2018-09-23 Thread James Smith
smithjd added inline comments. INLINE COMMENTS > taglibwritertest.cpp:60 > > -QCOMPARE(extractedTitle, QStringLiteral("Title1")); > -QCOMPARE(extractedArtist, QStringLiteral("Artist1")); > -QCOMPARE(extractedAlbum, QStringLiteral("Album1")); > +QCOMPARE(extractedTitle,

D15714: add a string suffix to test data and use for unicode testing of taglibwriter

2018-09-23 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > svuorela wrote in taglibwritertest.cpp:66 > yeah. given you write and read it, if somehow it gets encoded e.g. as > iso-8859-15 rather than utf8, the euro sign would be encoded as 0xa4 rather > than 0x20ac. > > As you write and read it in the

D15714: add a string suffix to test data and use for unicode testing of taglibwriter

2018-09-23 Thread Sune Vuorela
svuorela added inline comments. INLINE COMMENTS > astippich wrote in taglibwritertest.cpp:66 > To be sure I understand correctly, using stringSuffix.toUTF8() is what you > would like to see here? Something along those lines, yes please, to compare with. REPOSITORY R286 KFileMetaData

D15714: add a string suffix to test data and use for unicode testing of taglibwriter

2018-09-23 Thread Alexander Stippich
astippich added inline comments. INLINE COMMENTS > svuorela wrote in taglibwritertest.cpp:66 > yeah. given you write and read it, if somehow it gets encoded e.g. as > iso-8859-15 rather than utf8, the euro sign would be encoded as 0xa4 rather > than 0x20ac. > > As you write and read it in the

D15714: add a string suffix to test data and use for unicode testing of taglibwriter

2018-09-23 Thread Sune Vuorela
svuorela added inline comments. INLINE COMMENTS > bruns wrote in taglibwritertest.cpp:66 > ? > The file/tags are written inside this test. yeah. given you write and read it, if somehow it gets encoded e.g. as iso-8859-15 rather than utf8, the euro sign would be encoded as 0xa4 rather than

D15714: add a string suffix to test data and use for unicode testing of taglibwriter

2018-09-23 Thread Stefan Brüns
bruns added inline comments. INLINE COMMENTS > svuorela wrote in taglibwritertest.cpp:66 > I suggest checking that the last bytes of all these extracted things is the > actual actual utf8 bytes, so that if someone compiles or saves this file in a > broken encoding that the testing doesn't

D15714: add a string suffix to test data and use for unicode testing of taglibwriter

2018-09-23 Thread Sune Vuorela
svuorela added inline comments. INLINE COMMENTS > taglibwritertest.cpp:66 > +QCOMPARE(extractedGenre, QString(QStringLiteral("Genre1") + > stringSuffix)); > +QCOMPARE(extractedComment, QString(QStringLiteral("Comment1") + > stringSuffix)); > I suggest checking that the last bytes of

D15714: add a string suffix to test data and use for unicode testing of taglibwriter

2018-09-23 Thread Alexander Stippich
astippich created this revision. astippich added reviewers: mgallien, bruns. Herald added projects: Frameworks, Baloo. Herald added subscribers: Baloo, kde-frameworks-devel. astippich requested review of this revision. REVISION SUMMARY append a unicode character to all test string such that

D15714: add a string suffix to test data and use for unicode testing of taglibwriter

2018-09-23 Thread Alexander Stippich
astippich added a dependency: D15704: increase test coverage of taglibwriter. REPOSITORY R286 KFileMetaData REVISION DETAIL https://phabricator.kde.org/D15714 To: astippich, mgallien, bruns Cc: kde-frameworks-devel, #baloo, ashaposhnikov, michaelh, astippich, spoorun, ngraham, bruns,