[Differential] [Commented On] D4082: parse the desktop file 2 times

2017-01-11 Thread David Edmundson
davidedmundson added a comment. See https://phabricator.kde.org/D4084 REPOSITORY R244 KCoreAddons REVISION DETAIL https://phabricator.kde.org/D4082 EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To: mart, #plasma, dfaure, davidedmundson Cc: apol,

[Differential] [Commented On] D4082: parse the desktop file 2 times

2017-01-11 Thread Aleix Pol Gonzalez
apol added a comment. How about adding a test? Other than that, the patch looks good. It's horrible that we need to do so, but I guess it's the price of backwards compatibility. There's the possibility of doing the processing in two steps (desktop to pairs, pairs to json), but we

[Differential] [Commented On] D4082: parse the desktop file 2 times

2017-01-11 Thread David Edmundson
davidedmundson added a comment. +1 REPOSITORY R244 KCoreAddons REVISION DETAIL https://phabricator.kde.org/D4082 EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To: mart, #plasma, dfaure, davidedmundson Cc: plasma-devel, #frameworks, lesliezhai,

[Differential] [Commented On] D4082: parse the desktop file 2 times

2017-01-11 Thread Marco Martin
mart added inline comments. INLINE COMMENTS > desktopfileparser.cpp:237 > > +bool tokenizeKeyValue(QFile , const QString , QByteArray , > QString , int ) > +{ maybe instead a bool it could have error codes to more easily assure the behavior is 100% the same as before REPOSITORY R244