> On Sept. 8, 2013, 8:45 a.m., David Faure wrote: > > staging/kemoticons/src/core/kemoticonsprovider.cpp, line 146 > > <http://git.reviewboard.kde.org/r/112527/diff/4/?file=188119#file188119line146> > > > > file.fileName() is the same as emo. Yes, the name of the methods in > > QFile are a bit confusing. > > > > If this code was meant to extract the actual filename only > > (/tmp/foo.png -> foo.png) then you have to use QFileInfo::fileName(). > > > > Hence my questions about absolute or relative paths, I don't actually > > know what "emo" contains in this method. > > David Gil Oliva wrote: > As I said before, emo is an absolute path.
OK then this line of code is broken :) file.fileName() is the same as emo, i.e. an absolute path. Concatenating that to themePath + '/' makes no sense, you'd end up with /usr/path/to/theme/usr/full/path/to/emoticon. > On Sept. 8, 2013, 8:45 a.m., David Faure wrote: > > staging/kemoticons/src/core/kemoticonsprovider.h, line 85 > > <http://git.reviewboard.kde.org/r/112527/diff/4/?file=188118#file188118line85> > > > > absolute path, or relative path? > > David Gil Oliva wrote: > All of them are absolute. Should I specify it in the docs? Or should I > modify the code so that it also accepts relative paths and converts them in > absolute? I guess I got confused by the broken line of code below. Path in KDE usually means absolute, yes. Add "full" before "path", or leave it as is. Definitely don't add support for relative paths if this wasn't there before, it means the base dir is unclear :) - David ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112527/#review39561 ----------------------------------------------------------- On Sept. 8, 2013, 12:58 a.m., David Gil Oliva wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/112527/ > ----------------------------------------------------------- > > (Updated Sept. 8, 2013, 12:58 a.m.) > > > Review request for KDE Frameworks. > > > Description > ------- > > Clean up KEmoticons framework (prior to splitting) > > --Clean up includes > --Turn KEmoticonsProvider abstract and reorganize some code > --Revise documentation > --Use QScopedPointer where useful > --Port away from KIO in KEmoticonsProvider > --Put const where fit > > TODO: > --Port away from KServiceTypeTrader. Sebas is working on a way to get > the plugin list without KServiceTypeTrader. > > > Diffs > ----- > > staging/kemoticons/src/core/kemoticons.h > 57912b550c5e941f751d93c084b37764635e11c7 > staging/kemoticons/autotests/kemoticontest.cpp > 0ca1671d26970998c13022daa839e1aae4907220 > staging/kemoticons/src/core/kemoticons.cpp > 317089ed94179aac2b7e448414df930dcfd0c0dd > staging/kemoticons/src/core/kemoticonsprovider.h > 3f43ca4c8a1fe74ce01a1aac1abdeb84b9da08d2 > staging/kemoticons/src/core/kemoticonsprovider.cpp > 1fcad79bd49919058bb21b262b57b7a9bb6ac5c9 > staging/kemoticons/src/core/kemoticonstheme.h > d0e1a899e2f8a89b34e6337848c1aeab7f60ef88 > staging/kemoticons/src/core/kemoticonstheme.cpp > cd385fde6095b2da930de9ec03a501ae3d1ab0f7 > staging/kemoticons/src/providers/adium/adium_emoticons.cpp > f465b64e230639f16ca073ed7ab08a79f04afd4a > staging/kemoticons/src/providers/kde/kde_emoticons.cpp > 7d777b5aa0a869a0009814bb43f84f802444d5d6 > staging/kemoticons/src/providers/pidgin/pidgin_emoticons.cpp > 65b2c5e979eeccbc98986432962e3ab05f39ca57 > staging/kemoticons/src/providers/xmpp/xmpp_emoticons.cpp > 2535b71263f468dacc830e7cf92fead5e61528e8 > staging/kemoticons/tests/main.cpp f46265e97243aa776f901120d832368154c5c2ed > > Diff: http://git.reviewboard.kde.org/r/112527/diff/ > > > Testing > ------- > > It compiles and test passes > > > Thanks, > > David Gil Oliva > >
_______________________________________________ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
