D4716: Add some more directives to MIPS assembler highlighting
arichardson added a comment. Sorry, was busy with other stuff so completely forgot about this. I'll update this soon. REPOSITORY R216 Syntax Highlighting REVISION DETAIL https://phabricator.kde.org/D4716 To: arichardson, dhaumann, vkrause Cc: kwrite-devel, kde-frameworks-devel, hase, michaelh, ngraham, bruns, demsking, cullmann, sars, dhaumann
D15479: fix for macOS
arichardson accepted this revision. arichardson added a comment. This revision is now accepted and ready to land. I can confirm that this fixes the build for me. REPOSITORY R272 KDNSSD REVISION DETAIL https://phabricator.kde.org/D15479 To: yurikoles, arichardson Cc: arichardson, yurikoles, kde-frameworks-devel, michaelh, ngraham, bruns
D14722: KPluginMetaData: convert empty string to empty stringlist.
arichardson accepted this revision. arichardson added a comment. This revision is now accepted and ready to land. LGTM REPOSITORY R244 KCoreAddons BRANCH convert_empty_string_to_stringlist REVISION DETAIL https://phabricator.kde.org/D14722 To: dfaure, apol, mpyne, davidedmundson, arichardson Cc: kde-frameworks-devel, michaelh, ngraham, bruns
D4716: Add some more directives to MIPS assembler highlighting
arichardson updated this revision to Diff 30212. arichardson marked 2 inline comments as done. arichardson added a comment. address comments will try to add tests soon REPOSITORY R216 Syntax Highlighting CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D4716?vs=11618=30212 BRANCH arcpatch-D4716 REVISION DETAIL https://phabricator.kde.org/D4716 AFFECTED FILES data/syntax/c-preprocessor.xml data/syntax/gnuassembler.xml data/syntax/mips.xml To: arichardson, dhaumann, vkrause Cc: #frameworks, michaelh, ngraham
D7236: DesktopFileParser: add fallback lookup in ":/kservicetypes5/*"
arichardson accepted this revision. arichardson added a comment. This revision is now accepted and ready to land. Looks good to me. Test would be great but the patch is pretty trivial so not really needed. REPOSITORY R244 KCoreAddons BRANCH master REVISION DETAIL https://phabricator.kde.org/D7236 To: dfaure, mart, arichardson, davidedmundson Cc: #frameworks
D7170: Fix errorneous whitespace handling in Desktop Entry parsing from DesktopFileParser
arichardson added a comment. Looks good to me. Using a regex engine for this seems like overkill but I guess compared to the I/O overhead of reading the desktop file it shouldn't matter. REPOSITORY R244 KCoreAddons REVISION DETAIL https://phabricator.kde.org/D7170 To: mpyne, #frameworks, arichardson Cc: apol
D6180: Makefile: Remove invalid keyword entries in makefile.xml
arichardson accepted this revision. arichardson added a comment. This revision is now accepted and ready to land. Looks good to me! I forgot to remove these when I commited the bmake support. REPOSITORY R216 Syntax Highlighting REVISION DETAIL https://phabricator.kde.org/D6180 To: orgads, #framework_syntax_hightlighting, arichardson Cc: #frameworks
[Differential] [Request, 255 lines] D4716: Add some more directives to MIPS assembler highlighting
arichardson created this revision. Restricted Application added a project: Frameworks. Restricted Application added a subscriber: Frameworks. REVISION SUMMARY Add a few 64-bit MIPS instruction mnemonics Add c-preprocessor.xml highlighting file to be included Extracted and slightly modified from isocpp.xml Make GNU Assembler and MIPS assembler use c-preprocessor.xml This fixes highlighting of e.g. #include REPOSITORY R216 Syntax Highlighting BRANCH master REVISION DETAIL https://phabricator.kde.org/D4716 AFFECTED FILES data/syntax/c-preprocessor.xml data/syntax/gnuassembler.xml data/syntax/mips.xml EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To: arichardson, dhaumann, vkrause Cc: #frameworks
Re: Review Request 122732: Add std::function overloads for KServiceTypeTrader
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122732/ --- (Updated Feb. 8, 2017, 3:30 p.m.) Status -- This change has been discarded. Review request for KDE Frameworks, Marco Martin and Sebastian Kügler. Repository: kservice Description --- These are a lot more flexible and less error-prone and will eventually allow us to remove the trader query language in KF6 once all users in KService are gone. REVIEW: 122732 Diffs - autotests/kservicetest.cpp d46f868185c3bf45138d80d04f4eb0d2840de9ca autotests/ksycocathreadtest.cpp fbd889b28a32397fbf9245827ff8b54405b82e3d src/services/kservicetypetrader.h 8e46812c2eeddca225e978a4dd55aa4cc5e902d0 src/services/kservicetypetrader.cpp 290e44e9161c8db47278543714426fdd3b5a87af Diff: https://git.reviewboard.kde.org/r/122732/diff/ Testing --- Unit tests pass There are still some more classes that use the string based checks, I'll add a std::function overload to them as well once this has been approved. Thanks, Alex Richardson
Re: Review Request 121283: Allow using new style connect in KActionCollection::add[Action]()
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121283/ --- (Updated Oct. 23, 2016, 8:57 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks, David Faure and Nicolás Alvarez. Changes --- Submitted with commit 538a66fdab8194e860b98f0276f2b4e257e60507 by Albert Astals Cid on behalf of Alex Richardson to branch master. Repository: kxmlgui Description --- Allow using new style connect in KActionCollection::add[Action]() Diffs - src/kactioncollection.h 5ff9a7781a4455c5d1ed79d366efd290f0879e9d Diff: https://git.reviewboard.kde.org/r/121283/diff/ Testing --- Thanks, Alex Richardson
Re: Review Request 122796: Print a warning message when loading an invalid KDeclarative::QmlObject
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122796/ --- (Updated Oct. 20, 2016, 11:53 a.m.) Status -- This change has been discarded. Review request for KDE Frameworks. Repository: kdeclarative Description --- Print a warning message when loading an invalid KDeclarative::QmlObject Diffs - src/kdeclarative/qmlobject.cpp 00478b469159dfdee3a05a9aa833bfe615e861e6 Diff: https://git.reviewboard.kde.org/r/122796/diff/ Testing --- Thanks, Alex Richardson
Re: Review Request 128944: Reduce temporary allocations in the DesktopFileParser
> On Sept. 19, 2016, 9:08 p.m., Alex Richardson wrote: > > src/lib/plugin/desktopfileparser.cpp, line 478 > > <https://git.reviewboard.kde.org/r/128944/diff/1/?file=477140#file477140line478> > > > > If I read this correctly we no longer handle leading/trailing spaces > > properly. Does the test still pass? Or maybe I forgot to add tests for this > > case. > > trimmed() will add another allocation, maybe we can change the string > > in place? > > > > There is `QStringRef::trimmed()` so `line.midRef(0).trimmed()` should > > work without any extra allocations, right? `leftRef(-1)` is slightly more efficient so rather use that I guess - Alex --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/128944/#review99278 --- On Sept. 19, 2016, 4:20 p.m., Aleix Pol Gonzalez wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/128944/ > --- > > (Updated Sept. 19, 2016, 4:20 p.m.) > > > Review request for KDE Frameworks. > > > Repository: kcoreaddons > > > Description > --- > > While analising plasmashell under heaptrack, one of the sore spots is > temporary allocations within DesktopFileParser. This improves the situation > by: > > * Only converting to QString/utf8 once. > * Using QStringRef instead of fully splitting QString to parse them. > > > Diffs > - > > src/lib/plugin/desktopfileparser.cpp 2eb198d > src/lib/plugin/desktopfileparser_p.h c61b297 > > Diff: https://git.reviewboard.kde.org/r/128944/diff/ > > > Testing > --- > > tests still pass, plasma still works normally. > > heaptrack plasmashell: > > after: > allocations:4169312 > leaked allocations: 83225 > temporary allocations: 606902 > > before: > allocations:4680691 > leaked allocations: 84825 > temporary allocations: 819292 > > > Thanks, > > Aleix Pol Gonzalez > >
Re: Review Request 128944: Reduce temporary allocations in the DesktopFileParser
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/128944/#review99278 --- Thanks for looking into this! When I wrote the code I thought it would be mostly transitional so I didn't pay much attention to efficiency. src/lib/plugin/desktopfileparser.cpp (line 215) <https://git.reviewboard.kde.org/r/128944/#comment66850> I guess the same optimization could be applied here. src/lib/plugin/desktopfileparser.cpp (line 477) <https://git.reviewboard.kde.org/r/128944/#comment66849> Using readLineInto() is really useful to reduce unnecessary allocations. However the function is quite new: `This function was introduced in Qt 5.5` Do we depend on Qt 5.5 already? src/lib/plugin/desktopfileparser.cpp (line 478) <https://git.reviewboard.kde.org/r/128944/#comment66851> If I read this correctly we no longer handle leading/trailing spaces properly. Does the test still pass? Or maybe I forgot to add tests for this case. trimmed() will add another allocation, maybe we can change the string in place? There is `QStringRef::trimmed()` so `line.midRef(0).trimmed()` should work without any extra allocations, right? src/lib/plugin/desktopfileparser.cpp (line 506) <https://git.reviewboard.kde.org/r/128944/#comment66848> We can remove valueEscaped now that both are of type QString - Alex Richardson On Sept. 19, 2016, 4:20 p.m., Aleix Pol Gonzalez wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/128944/ > --- > > (Updated Sept. 19, 2016, 4:20 p.m.) > > > Review request for KDE Frameworks. > > > Repository: kcoreaddons > > > Description > --- > > While analising plasmashell under heaptrack, one of the sore spots is > temporary allocations within DesktopFileParser. This improves the situation > by: > > * Only converting to QString/utf8 once. > * Using QStringRef instead of fully splitting QString to parse them. > > > Diffs > - > > src/lib/plugin/desktopfileparser.cpp 2eb198d > src/lib/plugin/desktopfileparser_p.h c61b297 > > Diff: https://git.reviewboard.kde.org/r/128944/diff/ > > > Testing > --- > > tests still pass, plasma still works normally. > > heaptrack plasmashell: > > after: > allocations:4169312 > leaked allocations: 83225 > temporary allocations: 606902 > > before: > allocations:4680691 > leaked allocations: 84825 > temporary allocations: 819292 > > > Thanks, > > Aleix Pol Gonzalez > >
Re: Review Request 126880: Fix QFileDialog::openUrl() for remote files
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126880/ --- (Updated July 29, 2016, 10:19 a.m.) Status -- This change has been discarded. Review request for KDE Frameworks and David Faure. Bugs: 345253 https://bugs.kde.org/show_bug.cgi?id=345253 Repository: frameworkintegration Description --- QFileDialog doesn't make an attempt to determine whether remote URLs are files or directories so it just passes the file URL to the platform file dialog which caused an error popup and a nonexistent directory to be selected. BUG: 345253 Supersedes https://git.reviewboard.kde.org/r/126831/ Diffs - src/platformtheme/kdeplatformfiledialoghelper.cpp 11e7efbb66948da40bf19f430f8020f95d0233f8 Diff: https://git.reviewboard.kde.org/r/126880/diff/ Testing --- Thanks, Alex Richardson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 121218: Allow using new style connect syntax with KStandardAction::create()
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121218/ --- (Updated May 31, 2016, 2:44 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks, David Faure and Nicolás Alvarez. Changes --- Submitted with commit 9395482e8b74d9954ea3b333eca286360688d0d4 by Gleb Popov to branch master. Repository: kconfigwidgets Description --- Not sure if MSVC has the necessary type_traits working, but can't test that now since it don't have a Windows machine available. Diffs - autotests/kstandardactiontest.h 0008d281c0353e872feb7483c75762a0012a7b76 autotests/kstandardactiontest.cpp 09ae35db05467d61b8baf50fac70c6228e324492 src/kstandardaction.h d511778b7a24b1ec2e546949dab21f1ec2fea96f src/kstandardaction.cpp e5bea7965032355501b4c238e37abcc0f883c0b7 Diff: https://git.reviewboard.kde.org/r/121218/diff/ Testing --- The newly added unit test passes Thanks, Alex Richardson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 121218: Allow using new style connect syntax with KStandardAction::create()
> On April 5, 2015, 3:26 p.m., David Faure wrote: > > Well, +1 for the idea. But I wonder what the apidox will look like, the > > macro+template probably don't make it work. > > > > Also missing @since 5.10. > > > > Ship it from me once the apidox issue is resolved. > > Gleb Popov wrote: > What apidox issue is being talked about? If it is about > `KSTANDARDACTION_WITH_NEW_STYLE_CONNECT`, then i was able to make it work > with some simple Doxyfile options. > > See `save()` overload for example: > http://arrowd.name/html/namespace_k_standard_action.html#abd0ad3c1f3ee5c9d2ff068a06c8a6ac1 > and > http://arrowd.name/html/namespace_k_standard_action.html#a92c0df356ef011d4a7ae9a844d4a0bc7 > > If this is OK, i can document all new connects with `@since@` too. Can > this be commited then? > > David Faure wrote: > Well that's ugly docs :) You should use #ifdef DOXYGEN_SHOULD_SKIP_THIS > around the enable_if and in the #else use the readable version (QAction *, > IIUC) so that doxygen sees something cleaner and readable by the developer > using this API. > > But actually I don't even understand the use of enable_if here. How can a > pointer-to-function or a lambda be ambiguous with const char * ? I don't remember why I needed it because it's been a long time since I wrote that code. But I think the compiler might try to substitute the template parameter Func with `const char*` in some cases. The other way to solve this might be to use like `const typename QtPrivate::FunctionPointer::Object *receiver` instead of `QObject*` to constrain the type of the Func argument. - Alex --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121218/#review78527 --- On Dec. 24, 2014, 1:23 p.m., Alex Richardson wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/121218/ > --- > > (Updated Dec. 24, 2014, 1:23 p.m.) > > > Review request for KDE Frameworks, David Faure and Nicolás Alvarez. > > > Repository: kconfigwidgets > > > Description > --- > > Not sure if MSVC has the necessary type_traits working, but can't test that > now since it don't have a Windows machine available. > > > Diffs > - > > autotests/kstandardactiontest.h 0008d281c0353e872feb7483c75762a0012a7b76 > autotests/kstandardactiontest.cpp 09ae35db05467d61b8baf50fac70c6228e324492 > src/kstandardaction.h d511778b7a24b1ec2e546949dab21f1ec2fea96f > src/kstandardaction.cpp e5bea7965032355501b4c238e37abcc0f883c0b7 > > Diff: https://git.reviewboard.kde.org/r/121218/diff/ > > > Testing > --- > > The newly added unit test passes > > > Thanks, > > Alex Richardson > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 127514: Skip category test if no restriction is set.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127514/#review94223 --- Ship it! Ship It! - Alex Richardson On April 3, 2016, 9:54 a.m., Andreas Cord-Landwehr wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/127514/ > --- > > (Updated April 3, 2016, 9:54 a.m.) > > > Review request for KDE Frameworks and Alex Richardson. > > > Repository: kcmutils > > > Description > --- > > Align the method's logic with the API documentation for > KPluginSelector::addPlugins. The intended behavior is that > if no categoryKey is set, the category key test is skipped. > > > Diffs > - > > src/kpluginselector.cpp a893e381d1376ccc5c8189b638609e141c198282 > > Diff: https://git.reviewboard.kde.org/r/127514/diff/ > > > Testing > --- > > Manual testing in Parley. > > > Thanks, > > Andreas Cord-Landwehr > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 126618: Make CMake macro kcoreaddons_desktop_to_json() work on windows.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126618/#review93315 --- +1 Another solution would be to have a config-desktopfileparser.h.cmake with `#define KDE_INSTALL_KSERVICETYPES5DIR @KDE_INSTALL_KSERVICETYPES5DIR@` and then check `if QFile::exists(KDE_INSTALL_KSERVICETYPES5DIR + path)` before checking applicationDirPath. But that can be done in a separate commit. KF5CoreAddonsMacros.cmake (line 54) <https://git.reviewboard.kde.org/r/126618/#comment63632> Use KDE_INSTALL_FULL_KSERVICETYPES5DIR instead of prepending CMAKE_INSTALL_PREFIX to the relative path. - Alex Richardson On March 8, 2016, 8:35 p.m., Kåre Särs wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/126618/ > --- > > (Updated March 8, 2016, 8:35 p.m.) > > > Review request for KDE Frameworks. > > > Repository: kcoreaddons > > > Description > --- > > Without this patch kcoreaddons_desktop_to_json() will not find the service > type destop file. > > On windows GenericDataLocation returns "C:/Users//AppData/Local" or > "C:/ProgramData". That is not a path that contains any destop files ;) > > This patch adds KDE_INSTALL_KSERVICETYPES5DIR as fist item in the search > paths and if not found searches like before and adds ../share/ if the > previous searches do not return a match. > > > Diffs > - > > KF5CoreAddonsMacros.cmake 5d8e3d4 > src/lib/plugin/desktopfileparser.cpp 319d29f > > Diff: https://git.reviewboard.kde.org/r/126618/diff/ > > > Testing > --- > > KTextEditor compiles on windows > > > Thanks, > > Kåre Särs > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 127205: Add stubs to allow compilation on Android.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127205/#review93290 --- Fix it, then Ship it! Looks good src/lib/util/kuser_unix.cpp (line 38) <https://git.reviewboard.kde.org/r/127205/#comment63616> I'd say these should be static inline so that the compiler doesn't need to emit a copy of the function in the library. - Alex Richardson On March 2, 2016, 8:18 p.m., Andreas Cord-Landwehr wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/127205/ > --- > > (Updated March 2, 2016, 8:18 p.m.) > > > Review request for KDE Frameworks, Aleix Pol Gonzalez and Alex Richardson. > > > Repository: kcoreaddons > > > Description > --- > > Android's libc implementation lacks several features required by the Unix > backend implementation. This patch allows compilation by adding stubs for all > missing struct members and functions. Result of this patch is that on Android > KUser returns empty GUID and UID lists. > > > Diffs > - > > CMakeLists.txt da67071 > src/lib/CMakeLists.txt cf57f09 > src/lib/util/kuser_unix.cpp de5acde > > Diff: https://git.reviewboard.kde.org/r/127205/diff/ > > > Testing > --- > > Compilation tested on Linux and Android. > > > Thanks, > > Andreas Cord-Landwehr > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 127205: Add stubs to allow compilation on Android.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127205/#review93073 --- Looks good to me. I'd prefer inline noop functions to macros, but I don't really mind. src/lib/util/kuser_unix.cpp (line 68) <https://git.reviewboard.kde.org/r/127205/#comment63467> I'd change this to `__BIONIC__` too - Alex Richardson On March 1, 2016, 9:38 p.m., Andreas Cord-Landwehr wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/127205/ > --- > > (Updated March 1, 2016, 9:38 p.m.) > > > Review request for KDE Frameworks, Aleix Pol Gonzalez and Alex Richardson. > > > Repository: kcoreaddons > > > Description > --- > > Android's libc implementation lacks several features required by the Unix > backend implementation. This patch allows compilation by adding stubs for all > missing struct members and functions. Result of this patch is that on Android > KUser returns empty GUID and UID lists. > > > Diffs > - > > CMakeLists.txt da67071 > src/lib/CMakeLists.txt cf57f09 > src/lib/util/kuser_unix.cpp de5acde > > Diff: https://git.reviewboard.kde.org/r/127205/diff/ > > > Testing > --- > > Compilation tested on Linux and Android. > > > Thanks, > > Andreas Cord-Landwehr > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 127205: Add stubs to allow compilation on Android.
> On March 1, 2016, 12:26 a.m., Aleix Pol Gonzalez wrote: > > src/lib/CMakeLists.txt, line 116 > > <https://git.reviewboard.kde.org/r/127205/diff/1/?file=445780#file445780line116> > > > > Why does pthread make a difference? > > Andreas Cord-Landwehr wrote: > Bionic provides native built-in support for (limited set of) pthread > functionality. So no linking is required: > > http://mobilepearls.com/labs/native-android-api/#pthreads Should be possible to use the CMake builtin `find_package(Threads)` and link to `Threads::Threads` instead of using `pthread` directly. We might even be able to get rid of the if(WIN32) this way, but I haven't got a Windows system to test. - Alex --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127205/#review92959 --- On Feb. 28, 2016, 10:14 a.m., Andreas Cord-Landwehr wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/127205/ > --- > > (Updated Feb. 28, 2016, 10:14 a.m.) > > > Review request for KDE Frameworks, Aleix Pol Gonzalez and Alex Richardson. > > > Repository: kcoreaddons > > > Description > --- > > Android's libc implementation lacks several features required by the Unix > backend implementation. This patch allows compilation by adding stubs for all > missing struct members and functions. Result of this patch is that on Android > KUser returns empty GUID and UID lists. > > > Diffs > - > > src/lib/CMakeLists.txt cf57f0947f13b126f658774209363480013a2e2a > src/lib/util/kuser_unix.cpp de5acde07f4cb8425f03c455bc55d03dfd2579e9 > > Diff: https://git.reviewboard.kde.org/r/127205/diff/ > > > Testing > --- > > Compilation tested on Linux and Android. > > > Thanks, > > Andreas Cord-Landwehr > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 126813: Fix build with older polkit
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126813/ --- (Updated Feb. 26, 2016, 3:59 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks. Changes --- Submitted with commit 15c7867e88c5278f61896b5531ac2b544add8220 by Alex Richardson to branch master. Repository: polkit-qt-1 Description --- Seems like the function exists, but the header declaration is missing Diffs - CMakeLists.txt bb91bdedc96b8211eb29a1180c2e451dc60fae18 core/polkitqt1-subject.h 03028f636d7efc154138821419a704b661a7aeac core/polkitqt1-subject.cpp ecb4c0e216d5bacf5dff5cf100611b941d3e8fbd polkitqt1-config.h.cmake PRE-CREATION Diff: https://git.reviewboard.kde.org/r/126813/diff/ Testing --- compiles now Thanks, Alex Richardson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 126876: Fix QFileDialog::openUrl() for remote files
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126876/#review92350 --- Looks good to me. Possibly switch the if/else to have the short case at the top. src/platformtheme/kdeplatformfiledialoghelper.cpp (line 202) <https://git.reviewboard.kde.org/r/126876/#comment62983> Maybe also pass `QUrl::StripTrailingSlash` here? - Alex Richardson On Feb. 14, 2016, 6:25 a.m., Kåre Särs wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/126876/ > --- > > (Updated Feb. 14, 2016, 6:25 a.m.) > > > Review request for KDE Frameworks, Alex Richardson and David Faure. > > > Repository: frameworkintegration > > > Description > --- > > Qt does not know if the remote URL points to a file or directory. that is why > options()->initialDirectory() returns the full URL even if it is a file. > > > This fix is a bit like Alex Richardson workaround in KIO > (https://git.reviewboard.kde.org/r/126831/), but in frameworkintegration in > stead (I did not see his/Your KIO fix before now...) > > I check the remote url in setDirectory() because setDirectoy() is called from > two places. > > > Diffs > - > > src/platformtheme/kdeplatformfiledialoghelper.cpp 11e7efb > > Diff: https://git.reviewboard.kde.org/r/126876/diff/ > > > Testing > --- > > Kate now happily opens local and remote folders :) > > > File Attachments > > > firefox.desktop > > https://git.reviewboard.kde.org/media/uploaded/files/2016/01/25/60c21962-396e-468e-add9-e7112c49d7ba__firefox.desktop > > > Thanks, > > Kåre Särs > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 126940: Move .protocol files of all io slaves bundled in kio to JSON meta data
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126940/#review92162 --- Looks good to me src/ioslaves/file/file.cpp (line 80) <https://git.reviewboard.kde.org/r/126940/#comment62873> embed - Alex Richardson On Jan. 31, 2016, 3:41 p.m., Christoph Cullmann wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/126940/ > --- > > (Updated Jan. 31, 2016, 3:41 p.m.) > > > Review request for KDE Frameworks, Alex Richardson and David Faure. > > > Repository: kio > > > Description > --- > > Move .protocol files of all io slaves bundled in kio to JSON meta data. > Second try, this time with i18n aware protocoltojson used. > > > Diffs > - > > autotests/kprotocolinfotest.cpp 812f7f7 > src/ioslaves/file/CMakeLists.txt cb85cfb > src/ioslaves/file/file.cpp 381e488 > src/ioslaves/file/file.json PRE-CREATION > src/ioslaves/file/file.protocol 523c0f5 > src/ioslaves/ftp/CMakeLists.txt 04f5600 > src/ioslaves/ftp/ftp.cpp 7477a6a > src/ioslaves/ftp/ftp.json PRE-CREATION > src/ioslaves/ftp/ftp.protocol 4c5f80c > src/ioslaves/help/CMakeLists.txt 867b59d > src/ioslaves/help/ghelp.json PRE-CREATION > src/ioslaves/help/ghelp.protocol d2a642a > src/ioslaves/help/help.json PRE-CREATION > src/ioslaves/help/help.protocol 1deefe5 > src/ioslaves/help/main.cpp 9939196 > src/ioslaves/help/main_ghelp.cpp 59c8558 > src/ioslaves/http/CMakeLists.txt da5601e > src/ioslaves/http/http.cpp a84129f > src/ioslaves/http/http.protocol 49e5dc5 > src/ioslaves/http/https.protocol c15d06f > src/ioslaves/http/webdav.protocol 05c977a > src/ioslaves/http/webdavs.protocol d5e4b2f > src/ioslaves/trash/CMakeLists.txt 05161cd > src/ioslaves/trash/kio_trash.cpp 81cc036 > src/ioslaves/trash/trash.json PRE-CREATION > src/ioslaves/trash/trash.protocol 7430575 > > Diff: https://git.reviewboard.kde.org/r/126940/diff/ > > > Testing > --- > > Compile and tests seem to work, this time the i18n strings look better in > trash.json, compared to my first try in > https://git.reviewboard.kde.org/r/125869/ > > > Thanks, > > Christoph Cullmann > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 126876: Fix QFileDialog::openUrl() for remote files
> On Jan. 24, 2016, 10:01 p.m., David Faure wrote: > > src/platformtheme/kdeplatformfiledialoghelper.cpp, line 195 > > <https://git.reviewboard.kde.org/r/126876/diff/1/?file=439596#file439596line195> > > > > Yes, this definitely looks like it should be fixed in Qt instead (and > > this code can be kept with a #ifdef QT_VERSION if you want). > > > > The method is called setDirectory, I expect it to get a directory. > > Kåre Särs wrote: > OK, but what whould I put as QT_VERSION? ;) The bug is present in at > least Qt 5.5.1. I have not tried with the Qt 5.6 branch. > > I wonder if it is actually fixable in Qt. How does Qt determine what is a > file and what is a directory? What if you name a directory "foo.doc" and you > cannot use QFileInfo::isDir() on it. > > David Faure wrote: > Sure, but how do we end up with a file passed to setDirectory? The API > (e.g. getOpenFileUrls) takes a QUrl dir, surely that's supposed to be a dir. > Is that where the file comes from? Or is QFileDialog getting confused > somewhere? I just don't see why we have to determine "is this url a file or a > directory" when the API is clear about the directory argument. But I assume > I'm looking at the wrong place, setDirectory is also called with some other > URL? > I'm asking for some debugging inside Qt, to find out how we end up with > setDirectory called with a file URL, and then if it's indeed a bug in Qt > fixing it there, and only then this will answer the QT_VERSION question ;) > > Kåre Särs wrote: > Now that I read the documentation again I don't see any mention of it, > but if you provide a local file url to getOpenFileUrls()'s dir parameter it > will open the directory of the file and pre-select the file. While this works > for local files it does not work for remote protocols. > > I looked into this last night for a couple of hours, but I have not dug > deep enough to find it. I do suspect that this maybe hidden feature just can > not work for remote protocols and that the "fix" has to be in the integration. The docs say `The file dialog's working directory will be set to dir. If dir includes a file name, the file will be selected.` So I think this should also work for remote files. - Alex --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126876/#review91545 --- On Jan. 24, 2016, 9:19 p.m., Kåre Särs wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/126876/ > --- > > (Updated Jan. 24, 2016, 9:19 p.m.) > > > Review request for KDE Frameworks, Alex Richardson and David Faure. > > > Repository: frameworkintegration > > > Description > --- > > This fix is a bit like Alex Richardson workaround in KIO > (https://git.reviewboard.kde.org/r/126831/), but in frameworkintegration in > stead (I did not see his/Your KIO fix before now...) > > I check the remote url in setDirectory() because setDirectoy() is called from > two places. > > > Diffs > - > > src/platformtheme/kdeplatformfiledialoghelper.cpp 11e7efb > > Diff: https://git.reviewboard.kde.org/r/126876/diff/ > > > Testing > --- > > Kate now happily opens local and remote folders :) > > > Thanks, > > Kåre Särs > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 126831: WIP: Fix QFileDialog::openUrl() for remote files
ion.h 177 0x7fce7574b821
19 QToolButton::nextCheckState qtoolbutton.cpp 954 0x7fce759a6bd3
20 QAbstractButtonPrivate::click qabstractbutton.cpp 515
0x7fce758aab1c
...
```
causing another call (possibly fixable by blocking signals). Stats
QUrl("sftp://user@host/home/user/;) again:
```
1 KIO::stat statjob.cpp 180 0x7fce732928c8
2 KFileWidget::setUrl kfilewidget.cpp 14760x7fce663f6e6c
3 KFileWidgetPrivate::_k_enterUrl kfilewidget.cpp 15450x7fce663f759a
4 KFileWidget::qt_static_metacall moc_kfilewidget.cpp 176
0x7fce663fe3e2
5 QMetaObject::activate qobject.cpp 37300x7fce7490fc54
6 QMetaObject::activate qobject.cpp 35950x7fce7490f442
7 KUrlNavigator::urlChanged moc_kurlnavigator.cpp 299
0x7fce66439dcf
8 KUrlNavigator::setLocationUrl kurlnavigator.cpp 1080
0x7fce66438eb4
9 KFileWidgetPrivate::_k_urlEntered kfilewidget.cpp 1515
0x7fce663f73a7
10 KFileWidget::qt_static_metacall moc_kfilewidget.cpp 175
0x7fce663fe3bf
11 QMetaObject::activate qobject.cpp 37300x7fce7490fc54
12 QMetaObject::activate qobject.cpp 35950x7fce7490f442
13 KDirOperator::urlEnteredmoc_kdiroperator.cpp586
0x7fce663de2b3
14 KDirOperator::setUrlkdiroperator.cpp10390x7fce663d2599
15 KFileWidget::setUrl kfilewidget.cpp 14910x7fce663f7207
16 KDEPlatformFileDialog::setDirectory kdeplatformfiledialoghelper.cpp
189 0x7fce666b0ce0
17 KDEPlatformFileDialogHelper::setDirectory
kdeplatformfiledialoghelper.cpp 344 0x7fce666b1c06
18 KDEPlatformFileDialogHelper::initializeDialog
kdeplatformfiledialoghelper.cpp 238 0x7fce666b147f
19 KDEPlatformFileDialogHelper::show kdeplatformfiledialoghelper.cpp
310 0x7fce666b1a36
20 QDialogPrivate::setNativeDialogVisible qdialog.cpp 129
0x7fce759eb10c
21 QFileDialog::setVisible qfiledialog.cpp 842 0x7fce759f3571
22 QWidget::show qwidget.cpp 77280x7fce757a8921
23 QDialog::exec qdialog.cpp 533 0x7fce759eb8c7
24 QFileDialog::getOpenFileUrlsqfiledialog.cpp 22750x7fce759f7e08
25 KWrite::slotOpenkwrite.cpp 250 0x41a0c1
...
```
Finally, when selecting a new file (but this one is required as we need to
check whether the new file can be opened)
Stats QUrl("sftp://user@host/home/user/bar.txt;)
```
1 KIO::stat statjob.cpp 180 0x7fce732928c8
2 KFileWidget::slotOk kfilewidget.cpp 993 0x7fce663f4005
3 KFileWidget::qt_static_metacall moc_kfilewidget.cpp 171
0x7fce663fe357
4 QMetaObject::activate qobject.cpp 37300x7fce7490fc54
5 QMetaObject::activate qobject.cpp 35950x7fce7490f442
6 QAbstractButton::clickedmoc_qabstractbutton.cpp 306
0x7fce75bec92a
7 QAbstractButtonPrivate::emitClicked qabstractbutton.cpp 533
0x7fce758aac1f
8 QAbstractButtonPrivate::click qabstractbutton.cpp 526
0x7fce758aabae
9 QAbstractButton::mouseReleaseEvent qabstractbutton.cpp 1131
0x7fce758ac08c
10 QWidget::event qwidget.cpp 87380x7fce757ab018
11 QAbstractButton::event qabstractbutton.cpp 10880x7fce758abeca
12 QPushButton::event qpushbutton.cpp 673 0x7fce7596c071
13 QApplicationPrivate::notify_helper qapplication.cpp3712
0x7fce7575b278
14 QApplication::notifyqapplication.cpp32700x7fce75758fdf
15 QCoreApplication::notifyInternal2 qcoreapplication.cpp1013
0x7fce748cf642
16 QCoreApplication::sendSpontaneousEvent qcoreapplication.h 230
0x7fce7575e226
17 QApplicationPrivate::sendMouseEvent qapplication.cpp2765
0x7fce75757997
18 QWidgetWindow::handleMouseEvent qwidgetwindow.cpp 554
0x7fce757d7a99
19 QWidgetWindow::eventqwidgetwindow.cpp 210 0x7fce757d671f
20 QApplicationPrivate::notify_helper qapplication.cpp3712
0x7fce7575b278
...
```
Thanks,
Alex Richardson
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 126880: Fix QFileDialog::openUrl() for remote files
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126880/ --- Review request for KDE Frameworks and David Faure. Bugs: 345253 https://bugs.kde.org/show_bug.cgi?id=345253 Repository: frameworkintegration Description --- QFileDialog doesn't make an attempt to determine whether remote URLs are files or directories so it just passes the file URL to the platform file dialog which caused an error popup and a nonexistent directory to be selected. BUG: 345253 Supersedes https://git.reviewboard.kde.org/r/126831/ Diffs - src/platformtheme/kdeplatformfiledialoghelper.cpp 11e7efbb66948da40bf19f430f8020f95d0233f8 Diff: https://git.reviewboard.kde.org/r/126880/diff/ Testing --- Thanks, Alex Richardson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 126740: Add a script for optimizing svgs
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126740/#review91416
---
optimize.svg.sh (line 1)
<https://git.reviewboard.kde.org/r/126740/#comment62489>
won't work on BSD (/usr/local/bin/bash)
/usr/bin/env bash should work everywhere
optimize.svg.sh (line 4)
<https://git.reviewboard.kde.org/r/126740/#comment62487>
if command -v svgo >/dev/null; then
optimize.svg.sh (line 14)
<https://git.reviewboard.kde.org/r/126740/#comment62488>
Paths with spaces will break this.
```
find . -name "*.svg" -size 4k -print0 | while IFS= read -r -d '' file; do
svgo -i $file -o $file $ARGS
done
```
(http://mywiki.wooledge.org/BashFAQ/001)
optimize.svg.sh (line 16)
<https://git.reviewboard.kde.org/r/126740/#comment62491>
Here an everywhere else
$v -> "$v" https://github.com/koalaman/shellcheck/wiki/SC2086
optimize.svg.sh (line 20)
<https://git.reviewboard.kde.org/r/126740/#comment62490>
spaces in a parent dir will break this loop
- Alex Richardson
On Jan. 19, 2016, 10:54 p.m., Aleix Pol Gonzalez wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126740/
> ---
>
> (Updated Jan. 19, 2016, 10:54 p.m.)
>
>
> Review request for KDE Frameworks and Plasma.
>
>
> Repository: breeze-icons
>
>
> Description
> ---
>
> Dirk's review (https://git.reviewboard.kde.org/r/126738/) gave me the idea
> that right now we're serving right away the svg's from inkscape and there's
> room for improvement, potentially.
>
> This patch just introduces a script that optimizes the svg's using `svgo`.
>
> More could be done, like using gzip files, we can look into that if anyone's
> interested. In fact, we used to use svgz for the icons, I wonder why that
> changed.
>
> This will change the files in-place rather than as a build step, which is
> what I considered first. The process to run svgo on every file was about 30
> minutes to 1h on my system, so I doubt it's really desirable.
>
> A reduced file size is important because it will greatly reduce disk IO,
> which is a bottle-neck we have.
>
>
> Diffs
> -
>
> optimize.svg.sh PRE-CREATION
>
> Diff: https://git.reviewboard.kde.org/r/126740/diff/
>
>
> Testing
> ---
>
> ```
> kde-devel@oliver:~/frameworks/breeze-icons (master)$ du -sh icons icons-dark/
> 32M icons
> 32M icons-dark/
>
> #run the script
>
> kde-devel@oliver:~/frameworks/breeze-icons (master)$ du -sh icons icons-dark/
> 17M icons
> 17M icons-dark/
> ```
>
>
> Thanks,
>
> Aleix Pol Gonzalez
>
>
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 126831: WIP: Fix QFileDialog::openUrl() for remote files
e qtoolbutton.cpp 954 0x7fce759a6bd3
20 QAbstractButtonPrivate::click qabstractbutton.cpp 515
0x7fce758aab1c
...
```
causing another call (possibly fixable by blocking signals). Stats
QUrl("sftp://user@host/home/user/;) again:
```
1 KIO::stat statjob.cpp 180 0x7fce732928c8
2 KFileWidget::setUrl kfilewidget.cpp 14760x7fce663f6e6c
3 KFileWidgetPrivate::_k_enterUrl kfilewidget.cpp 15450x7fce663f759a
4 KFileWidget::qt_static_metacall moc_kfilewidget.cpp 176
0x7fce663fe3e2
5 QMetaObject::activate qobject.cpp 37300x7fce7490fc54
6 QMetaObject::activate qobject.cpp 35950x7fce7490f442
7 KUrlNavigator::urlChanged moc_kurlnavigator.cpp 299
0x7fce66439dcf
8 KUrlNavigator::setLocationUrl kurlnavigator.cpp 1080
0x7fce66438eb4
9 KFileWidgetPrivate::_k_urlEntered kfilewidget.cpp 1515
0x7fce663f73a7
10 KFileWidget::qt_static_metacall moc_kfilewidget.cpp 175
0x7fce663fe3bf
11 QMetaObject::activate qobject.cpp 37300x7fce7490fc54
12 QMetaObject::activate qobject.cpp 35950x7fce7490f442
13 KDirOperator::urlEnteredmoc_kdiroperator.cpp586
0x7fce663de2b3
14 KDirOperator::setUrlkdiroperator.cpp10390x7fce663d2599
15 KFileWidget::setUrl kfilewidget.cpp 14910x7fce663f7207
16 KDEPlatformFileDialog::setDirectory kdeplatformfiledialoghelper.cpp
189 0x7fce666b0ce0
17 KDEPlatformFileDialogHelper::setDirectory
kdeplatformfiledialoghelper.cpp 344 0x7fce666b1c06
18 KDEPlatformFileDialogHelper::initializeDialog
kdeplatformfiledialoghelper.cpp 238 0x7fce666b147f
19 KDEPlatformFileDialogHelper::show kdeplatformfiledialoghelper.cpp
310 0x7fce666b1a36
20 QDialogPrivate::setNativeDialogVisible qdialog.cpp 129
0x7fce759eb10c
21 QFileDialog::setVisible qfiledialog.cpp 842 0x7fce759f3571
22 QWidget::show qwidget.cpp 77280x7fce757a8921
23 QDialog::exec qdialog.cpp 533 0x7fce759eb8c7
24 QFileDialog::getOpenFileUrlsqfiledialog.cpp 22750x7fce759f7e08
25 KWrite::slotOpenkwrite.cpp 250 0x41a0c1
...
```
Finally, when selecting a new file (but this one is required as we need to
check whether the new file can be opened)
Stats QUrl("sftp://user@host/home/user/bar.txt;)
```
1 KIO::stat statjob.cpp 180 0x7fce732928c8
2 KFileWidget::slotOk kfilewidget.cpp 993 0x7fce663f4005
3 KFileWidget::qt_static_metacall moc_kfilewidget.cpp 171
0x7fce663fe357
4 QMetaObject::activate qobject.cpp 37300x7fce7490fc54
5 QMetaObject::activate qobject.cpp 35950x7fce7490f442
6 QAbstractButton::clickedmoc_qabstractbutton.cpp 306
0x7fce75bec92a
7 QAbstractButtonPrivate::emitClicked qabstractbutton.cpp 533
0x7fce758aac1f
8 QAbstractButtonPrivate::click qabstractbutton.cpp 526
0x7fce758aabae
9 QAbstractButton::mouseReleaseEvent qabstractbutton.cpp 1131
0x7fce758ac08c
10 QWidget::event qwidget.cpp 87380x7fce757ab018
11 QAbstractButton::event qabstractbutton.cpp 10880x7fce758abeca
12 QPushButton::event qpushbutton.cpp 673 0x7fce7596c071
13 QApplicationPrivate::notify_helper qapplication.cpp3712
0x7fce7575b278
14 QApplication::notifyqapplication.cpp32700x7fce75758fdf
15 QCoreApplication::notifyInternal2 qcoreapplication.cpp1013
0x7fce748cf642
16 QCoreApplication::sendSpontaneousEvent qcoreapplication.h 230
0x7fce7575e226
17 QApplicationPrivate::sendMouseEvent qapplication.cpp2765
0x7fce75757997
18 QWidgetWindow::handleMouseEvent qwidgetwindow.cpp 554
0x7fce757d7a99
19 QWidgetWindow::eventqwidgetwindow.cpp 210 0x7fce757d671f
20 QApplicationPrivate::notify_helper qapplication.cpp3712
0x7fce7575b278
...
```
Thanks,
Alex Richardson
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 126831: WIP: Fix QFileDialog::openUrl() for remote files
QToolButton::nextCheckState qtoolbutton.cpp 954 0x7fce759a6bd3
20 QAbstractButtonPrivate::click qabstractbutton.cpp 515
0x7fce758aab1c
...
```
causing another call (possibly fixable by blocking signals). Stats
QUrl("sftp://user@host/home/user/;) again:
```
1 KIO::stat statjob.cpp 180 0x7fce732928c8
2 KFileWidget::setUrl kfilewidget.cpp 14760x7fce663f6e6c
3 KFileWidgetPrivate::_k_enterUrl kfilewidget.cpp 15450x7fce663f759a
4 KFileWidget::qt_static_metacall moc_kfilewidget.cpp 176
0x7fce663fe3e2
5 QMetaObject::activate qobject.cpp 37300x7fce7490fc54
6 QMetaObject::activate qobject.cpp 35950x7fce7490f442
7 KUrlNavigator::urlChanged moc_kurlnavigator.cpp 299
0x7fce66439dcf
8 KUrlNavigator::setLocationUrl kurlnavigator.cpp 1080
0x7fce66438eb4
9 KFileWidgetPrivate::_k_urlEntered kfilewidget.cpp 1515
0x7fce663f73a7
10 KFileWidget::qt_static_metacall moc_kfilewidget.cpp 175
0x7fce663fe3bf
11 QMetaObject::activate qobject.cpp 37300x7fce7490fc54
12 QMetaObject::activate qobject.cpp 35950x7fce7490f442
13 KDirOperator::urlEnteredmoc_kdiroperator.cpp586
0x7fce663de2b3
14 KDirOperator::setUrlkdiroperator.cpp10390x7fce663d2599
15 KFileWidget::setUrl kfilewidget.cpp 14910x7fce663f7207
16 KDEPlatformFileDialog::setDirectory kdeplatformfiledialoghelper.cpp
189 0x7fce666b0ce0
17 KDEPlatformFileDialogHelper::setDirectory
kdeplatformfiledialoghelper.cpp 344 0x7fce666b1c06
18 KDEPlatformFileDialogHelper::initializeDialog
kdeplatformfiledialoghelper.cpp 238 0x7fce666b147f
19 KDEPlatformFileDialogHelper::show kdeplatformfiledialoghelper.cpp
310 0x7fce666b1a36
20 QDialogPrivate::setNativeDialogVisible qdialog.cpp 129
0x7fce759eb10c
21 QFileDialog::setVisible qfiledialog.cpp 842 0x7fce759f3571
22 QWidget::show qwidget.cpp 77280x7fce757a8921
23 QDialog::exec qdialog.cpp 533 0x7fce759eb8c7
24 QFileDialog::getOpenFileUrlsqfiledialog.cpp 22750x7fce759f7e08
25 KWrite::slotOpenkwrite.cpp 250 0x41a0c1
...
```
Finally, when selecting a new file (but this one is required as we need to
check whether the new file can be opened)
Stats QUrl("sftp://user@host/home/user/bar.txt;)
```
1 KIO::stat statjob.cpp 180 0x7fce732928c8
2 KFileWidget::slotOk kfilewidget.cpp 993 0x7fce663f4005
3 KFileWidget::qt_static_metacall moc_kfilewidget.cpp 171
0x7fce663fe357
4 QMetaObject::activate qobject.cpp 37300x7fce7490fc54
5 QMetaObject::activate qobject.cpp 35950x7fce7490f442
6 QAbstractButton::clickedmoc_qabstractbutton.cpp 306
0x7fce75bec92a
7 QAbstractButtonPrivate::emitClicked qabstractbutton.cpp 533
0x7fce758aac1f
8 QAbstractButtonPrivate::click qabstractbutton.cpp 526
0x7fce758aabae
9 QAbstractButton::mouseReleaseEvent qabstractbutton.cpp 1131
0x7fce758ac08c
10 QWidget::event qwidget.cpp 87380x7fce757ab018
11 QAbstractButton::event qabstractbutton.cpp 10880x7fce758abeca
12 QPushButton::event qpushbutton.cpp 673 0x7fce7596c071
13 QApplicationPrivate::notify_helper qapplication.cpp3712
0x7fce7575b278
14 QApplication::notifyqapplication.cpp32700x7fce75758fdf
15 QCoreApplication::notifyInternal2 qcoreapplication.cpp1013
0x7fce748cf642
16 QCoreApplication::sendSpontaneousEvent qcoreapplication.h 230
0x7fce7575e226
17 QApplicationPrivate::sendMouseEvent qapplication.cpp2765
0x7fce75757997
18 QWidgetWindow::handleMouseEvent qwidgetwindow.cpp 554
0x7fce757d7a99
19 QWidgetWindow::eventqwidgetwindow.cpp 210 0x7fce757d671f
20 QApplicationPrivate::notify_helper qapplication.cpp3712
0x7fce7575b278
...
```
Thanks,
Alex Richardson
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 126813: Fix build with older polkit
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126813/ --- Review request for KDE Frameworks. Repository: polkit-qt-1 Description --- Seems like the function exists, but the header declaration is missing Diffs - core/polkitqt1-subject.cpp ecb4c0e216d5bacf5dff5cf100611b941d3e8fbd Diff: https://git.reviewboard.kde.org/r/126813/diff/ Testing --- compiles now Thanks, Alex Richardson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 125048: Use JSON files directly instead of kcoreaddons_desktop_to_json()
> On Jan. 10, 2016, 7:04 p.m., David Faure wrote: > > src/ioslaves/http/kcookiejar/kcookiejar.json, line 96 > > <https://git.reviewboard.kde.org/r/125048/diff/1/?file=400284#file400284line96> > > > > Why was this set to false (and autoload to true) when the > > kcookiejar.desktop file had > > X-KDE-Kded-autoload=false > > X-KDE-Kded-load-on-demand=true > > ? > > > > I assume it wasn't intentional, but a copy/paste error? Yes looks like it was a copy paste error. I feared it could be a bug in desktoptojson, but I just added a new unit test that shows that this is not the case. - Alex --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/125048/#review90842 ------- On Sept. 8, 2015, 8:28 p.m., Alex Richardson wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/125048/ > --- > > (Updated Sept. 8, 2015, 8:28 p.m.) > > > Review request for KDE Frameworks and David Faure. > > > Repository: kio > > > Description > --- > > Use JSON files directly instead of kcoreaddons_desktop_to_json() > > > Diffs > - > > src/ioslaves/http/kcookiejar/CMakeLists.txt > 7b4778d1f67c1ad9f9edcaa4692b39ee6fe3f365 > src/ioslaves/http/kcookiejar/kcookiejar.desktop > 3ea56abdc134cae22b69d7b7636ce6dd415a3d9b > src/ioslaves/http/kcookiejar/kcookiejar.json PRE-CREATION > src/kpac/CMakeLists.txt fc5989714480ca49b5bd72e1c7b458b26bd0d9bc > src/kpac/proxyscout.desktop a545f3d6ef2fd18b1a2c85ebff15c9f2513d87f1 > src/kpac/proxyscout.json PRE-CREATION > src/kpasswdserver/CMakeLists.txt 11c30201012fbe413ff58561b54255e88c2c55b9 > src/kpasswdserver/kpasswdserver.desktop > bc788e5665e3d9d43309da241c3a3f5ac3cd0fc9 > src/kpasswdserver/kpasswdserver.json PRE-CREATION > src/kssld/CMakeLists.txt bf6970c2741a6edd01e36b86744d643e70046889 > src/kssld/kssld.desktop 86581b208ffc89fa5235f9284395d9d7ccebc472 > src/kssld/kssld.json PRE-CREATION > > Diff: https://git.reviewboard.kde.org/r/125048/diff/ > > > Testing > --- > > > Thanks, > > Alex Richardson > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 126618: Make CMake macro kcoreaddons_desktop_to_json() work on windows.
> On Jan. 3, 2016, 3:22 p.m., Alex Richardson wrote:
> > According to the documentation AppLocalDataLocation is the following:
> > "C:/Users//AppData/Local/", "C:/ProgramData/",
> > "", "/data"
> >
> > In which directory are the desktop files? Unfortunately I can't check as my
> > Windows machine broke a while back and I haven't compiled KF5 on Windows
> > since.
> >
> > Patch looks good to me otherwise as it will still check the same
> > directories change behaviour on Linux. Only minor issue is that the error
> > message is a little bit confusing now.
>
> Christoph Cullmann wrote:
> hi, same problem occurs on mac, too, i think a better fallback would be
> something install prefix relative, that would work on win + mac.
>
> Alex Richardson wrote:
> Would checking `${KDE_INSTALL_FULL_KSERVICETYPES5DIR} + path` first and
> then fall back to GenericDataLocation work on Windows and Mac? Or do we still
> need the AppDataLocation for runtime detection of the paths?
>
> Ralf Habacker wrote:
> From
> https://build.opensuse.org/build/home:rhabacker:branches:windows:mingw:win32:KF516/openSUSE_13.2/x86_64/mingw32-umbrello5/_log
> ...
> Installing:
> /home/abuild/rpmbuild/BUILDROOT/mingw32-umbrello5-2.18.99.6f6891a-2.25.x86_64/usr/i686-w64-mingw32/sys-root/mingw/share/applications/org.kde.umbrello.desktop
>
> Christoph Cullmann wrote:
> KDE_INSTALL_FULL_KSERVICETYPES5DIR should be good enough to have
> something working to be able to build KF5 on win/mac without patching. If the
> desktoptojson program should work even after packed into some
> installer/application bundle I guess we would need the appdata fallback, too.
> The question is: is that a use case needed. For me it is enough to be able to
> have it working in a devel setup.
>
> Alex Richardson wrote:
> This code can also be used at runtime through
> [KPluginMetaData::fromDesktopFile()](http://api.kde.org/frameworks-api/frameworks5-apidocs/kcoreaddons/html/classKPluginMetaData.html#ac6e87c110b4743ce1b16044c649838ba)
> although any users should probably be using JSON files directly.
>
> Kåre Särs wrote:
> The kpart.desktop file that KTextEditor was looking for is in
> /share/kservicetypes5/.
>
> I added AppLocalData to get the path which is normally
> "/bin/" for KDE aplications. I could also live with
> KDE_INSTALL_FULL_KSERVICETYPES5DIR, but generally I would try to avoid
> absolute paths hardcoded into binaries...
>
> I'll update the error printout.
>
> Sebastian Kügler wrote:
> Plasma packages use KPluginMetaData::fromDesktopFile(), this cannot be
> changed easily, as almost all packages around (our own, and third party) are
> using .desktop files. We're slowly transitioning, but it will take time.
> Removing that would mean a lot of our stuff would break, and even more 3rd
> party plasmoids, kwin scripts, etc..
>
> Christoph Cullmann wrote:
> We won't remove what works atm and I guess it will not matter that much
> if plasmoids/kwin stuff works "better" on win/mac or like they do atm.
>
> Sebastian Kügler wrote:
> It was just an example, the code in question is in the kpackage
> framework, meaning it's not plasma or kwin specific, but holds for any app
> using it. (I understand those don't work on Windows, due to this problem?)
>From what I can see by grepping over kpackage it doesn't use the service type
>parameter but just interprets it as a standard KPluginInfo .desktop file with
>no custom types. So that's fine. I'd vote for adding
>KDE_INSTALL_FULL_KSERVICETYPES5DIR as a first check, and then falling back to
>GenericDataLocation/kservicetypes5 (and/or possibly
>AppLocalDataLocation/../share on Mac/Windows)
- Alex
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126618/#review90496
---
On Jan. 3, 2016, 1:22 p.m., Kåre Särs wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126618/
> ---
>
> (Updated Jan. 3, 2016, 1:22 p.m.)
>
>
> Review request for KDE Frameworks.
>
>
> Repository: kcoreaddons
>
>
> Description
> ---
>
> Without this patch kcoreaddons_desktop_to_json() will not find the destop
> file.
>
> On windows GenericDataLocation returns "C:/Users//AppData/Local" or
> "C:/Progra
Re: Review Request 121672: Properly convert .desktop files that have an associated servicetype
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/121672/ --- (Updated Jan. 4, 2016, 2:14 p.m.) Status -- This change has been discarded. Review request for KDE Frameworks. Repository: kcoreaddons Description --- This ensures that properties that are defined to be of type QStringList or int or bool are properly converted to the right JSON type. Not sure if this code should also be part of KF5CoreAddons.so, since it does increase the library size quite a bit. It would however be very useful for kcoreaddons_desktop_to_json(), so that the initial conversion to JSON does not have to be done by hand. I probably don't have all the service types that exist installed on my system so I might be missing some properties. I included the script to generate the list of these properties, so that missing properties can be added by anyone who has the required servicetypes/*.desktop files installed. Diffs - src/lib/plugin/read-servicetypes.py PRE-CREATION autotests/desktoptojsontest.cpp 64373d5be930426dd8a1f8e455e33c411a4795fd src/lib/plugin/desktopfileparser.cpp b1b5440b48e4fd412932a7d7e794d641b1406699 Diff: https://git.reviewboard.kde.org/r/121672/diff/ Testing --- Unit test works Thanks, Alex Richardson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 126618: Make CMake macro kcoreaddons_desktop_to_json() work on windows.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126618/#review90496 --- According to the documentation AppLocalDataLocation is the following: "C:/Users//AppData/Local/", "C:/ProgramData/", "", "/data" In which directory are the desktop files? Unfortunately I can't check as my Windows machine broke a while back and I haven't compiled KF5 on Windows since. Patch looks good to me otherwise as it will still check the same directories change behaviour on Linux. Only minor issue is that the error message is a little bit confusing now. - Alex Richardson On Jan. 3, 2016, 1:22 p.m., Kåre Särs wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/126618/ > --- > > (Updated Jan. 3, 2016, 1:22 p.m.) > > > Review request for KDE Frameworks. > > > Repository: kcoreaddons > > > Description > --- > > Without this patch kcoreaddons_desktop_to_json() will not find the destop > file. > > On windows GenericDataLocation returns "C:/Users//AppData/Local" or > "C:/ProgramData". That is not a path that contains any destip files ;) > > This patch adds AppLocalDataLocation to the seach if the previous search does > not return a match. > > Another option would be to hardcode the absolute path to all places that uses > kcoreaddons_desktop_to_json(), but that does not feel too nice... > > What other options do we have? > > > Diffs > - > > src/lib/plugin/desktopfileparser.cpp 1122af8 > > Diff: https://git.reviewboard.kde.org/r/126618/diff/ > > > Testing > --- > > KTextEditor compiles again on windows > > > Thanks, > > Kåre Särs > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 126618: Make CMake macro kcoreaddons_desktop_to_json() work on windows.
> On Jan. 3, 2016, 3:22 p.m., Alex Richardson wrote:
> > According to the documentation AppLocalDataLocation is the following:
> > "C:/Users//AppData/Local/", "C:/ProgramData/",
> > "", "/data"
> >
> > In which directory are the desktop files? Unfortunately I can't check as my
> > Windows machine broke a while back and I haven't compiled KF5 on Windows
> > since.
> >
> > Patch looks good to me otherwise as it will still check the same
> > directories change behaviour on Linux. Only minor issue is that the error
> > message is a little bit confusing now.
>
> Christoph Cullmann wrote:
> hi, same problem occurs on mac, too, i think a better fallback would be
> something install prefix relative, that would work on win + mac.
Would checking `${KDE_INSTALL_FULL_KSERVICETYPES5DIR} + path` first and then
fall back to GenericDataLocation work on Windows and Mac? Or do we still need
the AppDataLocation for runtime detection of the paths?
- Alex
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126618/#review90496
---
On Jan. 3, 2016, 1:22 p.m., Kåre Särs wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126618/
> ---
>
> (Updated Jan. 3, 2016, 1:22 p.m.)
>
>
> Review request for KDE Frameworks.
>
>
> Repository: kcoreaddons
>
>
> Description
> ---
>
> Without this patch kcoreaddons_desktop_to_json() will not find the destop
> file.
>
> On windows GenericDataLocation returns "C:/Users//AppData/Local" or
> "C:/ProgramData". That is not a path that contains any destip files ;)
>
> This patch adds AppLocalDataLocation to the seach if the previous search does
> not return a match.
>
> Another option would be to hardcode the absolute path to all places that uses
> kcoreaddons_desktop_to_json(), but that does not feel too nice...
>
> What other options do we have?
>
>
> Diffs
> -
>
> src/lib/plugin/desktopfileparser.cpp 1122af8
>
> Diff: https://git.reviewboard.kde.org/r/126618/diff/
>
>
> Testing
> ---
>
> KTextEditor compiles again on windows
>
>
> Thanks,
>
> Kåre Särs
>
>
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 126618: Make CMake macro kcoreaddons_desktop_to_json() work on windows.
> On Jan. 3, 2016, 3:22 p.m., Alex Richardson wrote:
> > According to the documentation AppLocalDataLocation is the following:
> > "C:/Users//AppData/Local/", "C:/ProgramData/",
> > "", "/data"
> >
> > In which directory are the desktop files? Unfortunately I can't check as my
> > Windows machine broke a while back and I haven't compiled KF5 on Windows
> > since.
> >
> > Patch looks good to me otherwise as it will still check the same
> > directories change behaviour on Linux. Only minor issue is that the error
> > message is a little bit confusing now.
>
> Christoph Cullmann wrote:
> hi, same problem occurs on mac, too, i think a better fallback would be
> something install prefix relative, that would work on win + mac.
>
> Alex Richardson wrote:
> Would checking `${KDE_INSTALL_FULL_KSERVICETYPES5DIR} + path` first and
> then fall back to GenericDataLocation work on Windows and Mac? Or do we still
> need the AppDataLocation for runtime detection of the paths?
>
> Ralf Habacker wrote:
> From
> https://build.opensuse.org/build/home:rhabacker:branches:windows:mingw:win32:KF516/openSUSE_13.2/x86_64/mingw32-umbrello5/_log
> ...
> Installing:
> /home/abuild/rpmbuild/BUILDROOT/mingw32-umbrello5-2.18.99.6f6891a-2.25.x86_64/usr/i686-w64-mingw32/sys-root/mingw/share/applications/org.kde.umbrello.desktop
>
> Christoph Cullmann wrote:
> KDE_INSTALL_FULL_KSERVICETYPES5DIR should be good enough to have
> something working to be able to build KF5 on win/mac without patching. If the
> desktoptojson program should work even after packed into some
> installer/application bundle I guess we would need the appdata fallback, too.
> The question is: is that a use case needed. For me it is enough to be able to
> have it working in a devel setup.
This code can also be used at runtime through
[KPluginMetaData::fromDesktopFile()](http://api.kde.org/frameworks-api/frameworks5-apidocs/kcoreaddons/html/classKPluginMetaData.html#ac6e87c110b4743ce1b16044c649838ba)
although any users should probably be using JSON files directly.
- Alex
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126618/#review90496
---
On Jan. 3, 2016, 1:22 p.m., Kåre Särs wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126618/
> ---
>
> (Updated Jan. 3, 2016, 1:22 p.m.)
>
>
> Review request for KDE Frameworks.
>
>
> Repository: kcoreaddons
>
>
> Description
> ---
>
> Without this patch kcoreaddons_desktop_to_json() will not find the destop
> file.
>
> On windows GenericDataLocation returns "C:/Users//AppData/Local" or
> "C:/ProgramData". That is not a path that contains any destip files ;)
>
> This patch adds AppLocalDataLocation to the seach if the previous search does
> not return a match.
>
> Another option would be to hardcode the absolute path to all places that uses
> kcoreaddons_desktop_to_json(), but that does not feel too nice...
>
> What other options do we have?
>
>
> Diffs
> -
>
> src/lib/plugin/desktopfileparser.cpp 1122af8
>
> Diff: https://git.reviewboard.kde.org/r/126618/diff/
>
>
> Testing
> ---
>
> KTextEditor compiles again on windows
>
>
> Thanks,
>
> Kåre Särs
>
>
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 125869: Convert all io slave .protocol data to json and embed it.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/125869/#review90411 --- I added support for translating the ExtraNames key here: https://svn.reviewboard.kde.org/r/7154/ - Alex Richardson On Dec. 28, 2015, 7:25 p.m., Christoph Cullmann wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/125869/ > --- > > (Updated Dec. 28, 2015, 7:25 p.m.) > > > Review request for KDE Frameworks, Albert Astals Cid, Alex Richardson, David > Faure, and Luigi Toscano. > > > Repository: kio > > > Description > --- > > Convert all io slave .protocol data to json and embed it. > Allows easier deployment of the slaves. > > > Diffs > - > > src/core/kprotocolinfo.cpp 7bfb9ad > src/protocoltojson/main.cpp 05b9364 > > Diff: https://git.reviewboard.kde.org/r/125869/diff/ > > > Testing > --- > > Tests still work (one needed patching, as the exec line contains now the full > path). > > Correction: Somehow the ./autotests/jobtest test is unstable for me here, > sometimes it works, sometimes not :/ but even without this change. > > > File Attachments > > > trash.json > > https://git.reviewboard.kde.org/media/uploaded/files/2015/12/28/6ab2cd95-b0bd-4347-80f2-6f753fa50425__trash.json > > > Thanks, > > Christoph Cullmann > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 126343: KPluginMetaData: Add fields used by KAboutData and introduce conversion function
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126343/ --- (Updated Dec. 28, 2015, 4:08 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks. Changes --- Submitted with commit 218084df9f5266306045fc6876e8f0a6886808dd by Alex Richardson to branch master. Repository: kcoreaddons Description --- Add KPluginMetaData::translators() and KAboutPerson::fromJson() Plugins could have a list of translators in KDE4 through KAboutData so this commit restores this for KF5. As a side effect of now having authors() and translators() returning a list of KAboutPerson I also added KAboutPerson::fromJSON() as publc function but I can make that internal API instead if it's preferred. --- Add KPluginMetaData::copyrightText(), extraInformation() and otherContributors() Now KPluginMetaData has well defined keys for all the information used by KAboutData. This means we can easily create KAboutData objects from a KPluginMetaData object e.g. for use in an KAboutApplicationDialog. For example Okular uses this to display information about generator that is being used for the current document. --- Add KAboutData::fromPluginMetaData(const KPluginMetaData ) This is useful e.g. to create an KAboutApplicationDialog for plugins Diffs - autotests/kpluginmetadatatest.cpp 25c8b8d5351eb5c5c784d528f9e90dfc8ea77982 src/lib/kaboutdata.h 0dbd7a0ce7ec9220614c5f824a0f379ea5f45559 src/lib/kaboutdata.cpp 5d9a55ea027fc1b8fb6e6394d72cf48f142bb664 src/lib/plugin/kpluginmetadata.h a91b94ad404b6839aa3b8d13b8da19f407708c1b src/lib/plugin/kpluginmetadata.cpp 36743600788693b6449d065f0448e3195445772e Diff: https://git.reviewboard.kde.org/r/126343/diff/ Testing --- Tests pass, used by KAboutApplicationDialog in okular: https://git.reviewboard.kde.org/r/126193/ Thanks, Alex Richardson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 125869: Convert all io slave .protocol data to json and embed it.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/125869/#review90237 --- Could you post a patch with the JSON files? I'll look into implementing the translations then. - Alex Richardson On Dec. 27, 2015, 5:14 p.m., Christoph Cullmann wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/125869/ > --- > > (Updated Dec. 27, 2015, 5:14 p.m.) > > > Review request for KDE Frameworks, Albert Astals Cid, Alex Richardson, David > Faure, and Luigi Toscano. > > > Repository: kio > > > Description > --- > > Convert all io slave .protocol data to json and embed it. > Allows easier deployment of the slaves. > > > Diffs > - > > src/core/kprotocolinfo.cpp 7bfb9ad > src/protocoltojson/main.cpp 05b9364 > > Diff: https://git.reviewboard.kde.org/r/125869/diff/ > > > Testing > --- > > Tests still work (one needed patching, as the exec line contains now the full > path). > > Correction: Somehow the ./autotests/jobtest test is unstable for me here, > sometimes it works, sometimes not :/ but even without this change. > > > Thanks, > > Christoph Cullmann > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 126343: KPluginMetaData: Add fields used by KAboutData and introduce conversion function
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126343/ --- (Updated Dec. 21, 2015, 9:45 a.m.) Review request for KDE Frameworks. Repository: kcoreaddons Description --- Add KPluginMetaData::translators() and KAboutPerson::fromJson() Plugins could have a list of translators in KDE4 through KAboutData so this commit restores this for KF5. As a side effect of now having authors() and translators() returning a list of KAboutPerson I also added KAboutPerson::fromJSON() as publc function but I can make that internal API instead if it's preferred. --- Add KPluginMetaData::copyrightText(), extraInformation() and otherContributors() Now KPluginMetaData has well defined keys for all the information used by KAboutData. This means we can easily create KAboutData objects from a KPluginMetaData object e.g. for use in an KAboutApplicationDialog. For example Okular uses this to display information about generator that is being used for the current document. --- Add KAboutData::fromPluginMetaData(const KPluginMetaData ) This is useful e.g. to create an KAboutApplicationDialog for plugins Diffs (updated) - autotests/kpluginmetadatatest.cpp 25c8b8d5351eb5c5c784d528f9e90dfc8ea77982 src/lib/kaboutdata.h 0dbd7a0ce7ec9220614c5f824a0f379ea5f45559 src/lib/kaboutdata.cpp 5d9a55ea027fc1b8fb6e6394d72cf48f142bb664 src/lib/plugin/kpluginmetadata.h a91b94ad404b6839aa3b8d13b8da19f407708c1b src/lib/plugin/kpluginmetadata.cpp 36743600788693b6449d065f0448e3195445772e Diff: https://git.reviewboard.kde.org/r/126343/diff/ Testing --- Tests pass, used by KAboutApplicationDialog in okular: https://git.reviewboard.kde.org/r/126193/ Thanks, Alex Richardson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 126409: Fix use-after-free in .desktop file parser
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126409/#review89808 --- Ship it! Ship It! - Alex Richardson On Dec. 18, 2015, 3:44 a.m., Michael Pyne wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/126409/ > --- > > (Updated Dec. 18, 2015, 3:44 a.m.) > > > Review request for KDE Frameworks and Alex Richardson. > > > Repository: kcoreaddons > > > Description > --- > > Patch to fix a use-after-free issue in > kcoreaddons/src/lib/plugin/desktopfileparser.cpp, noted by Coverity (CID > 1336157) > > The issue is that if `defs << *def` happens after the call to > s_serviceTypes->insert(), the `*def` might already have been deleted by > QCache. Attached patch fixes by making the copy before the call to insert(). > > Really, it's probably better to not use QCache here if we can avoid it, but > I'm not sure enough of the code to go that route. But if this is something > that's only run once or twice anyways then there's no real need to use QCache > instead of QMap, I would think. > > > Diffs > - > > src/lib/plugin/desktopfileparser.cpp 06a4a1d > > Diff: https://git.reviewboard.kde.org/r/126409/diff/ > > > Testing > --- > > desktoptojsontest still passes, code compiles. > > > Thanks, > > Michael Pyne > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 126397: [xcb] Safety check whether we have a QApplication in mapViewport
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126397/#review89663 --- src/platforms/xcb/kwindowsystem.cpp (line 355) <https://git.reviewboard.kde.org/r/126397/#comment61407> qobject_cast? - Alex Richardson On Dec. 17, 2015, 9:21 a.m., Martin Gräßlin wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/126397/ > --- > > (Updated Dec. 17, 2015, 9:21 a.m.) > > > Review request for KDE Frameworks, kwin and Albert Astals Cid. > > > Bugs: 354811 > https://bugs.kde.org/show_bug.cgi?id=354811 > > > Repository: kwindowsystem > > > Description > --- > > We observed that with Compiz as window manager various applications > crash if they use QGuiApplication instead of QApplication. The reason > for this is that on Compiz the mapViewport code paths are used and > that has a widgets dependency. > > This change should ensure that applications do at least not crash > in this condition. > > BUG: 354811 > > > Diffs > - > > src/platforms/xcb/kwindowsystem.cpp > 9d287043c24894ca3c29c439c7939b139da055e8 > > Diff: https://git.reviewboard.kde.org/r/126397/diff/ > > > Testing > --- > > User in referenced bug report tested it, works. > > > Thanks, > > Martin Gräßlin > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 126366: Make KPluginMetaData constructible from a json path.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126366/#review89555 --- Ship it! If you have time, a minimal unit test would be nice to make sure we don't break this in future. - Alex Richardson On Dec. 15, 2015, 3:27 p.m., Aleix Pol Gonzalez wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/126366/ > --- > > (Updated Dec. 15, 2015, 3:27 p.m.) > > > Review request for KDE Frameworks. > > > Repository: kcoreaddons > > > Description > --- > > Loads the json file upon construction. Simplifies adoption of json metadata > files for KPackage. > > > Diffs > - > > src/lib/plugin/kpluginmetadata.h a91b94a > src/lib/plugin/kpluginmetadata.cpp 3674360 > > Diff: https://git.reviewboard.kde.org/r/126366/diff/ > > > Testing > --- > > Tests still pass. > > > Thanks, > > Aleix Pol Gonzalez > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 126348: Make it possible to provide the metadata in json
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126348/#review89556
---
src/kpackage/package.cpp (line 24)
<https://git.reviewboard.kde.org/r/126348/#comment61280>
no longer required, right?
src/kpackage/package.cpp (line 916)
<https://git.reviewboard.kde.org/r/126348/#comment61281>
`path.endsWith("/metadata.desktop") || path.endsWith("/metadata.json")`?
QRegExp seems like overkill here.
- Alex Richardson
On Dec. 15, 2015, 3:24 p.m., Aleix Pol Gonzalez wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126348/
> ---
>
> (Updated Dec. 15, 2015, 3:24 p.m.)
>
>
> Review request for KDE Frameworks and Plasma.
>
>
> Repository: kpackage
>
>
> Description
> ---
>
> Due to KCoreAddons transition from using desktop files to json files,
> KPackage ended in a weird state where desktop files were allowed to use
> desktop files but not json.
>
> This patch makes sure that manifest.json files are also acceptable.
>
>
> Diffs
> -
>
> autotests/data/testjsonmetadatapackage/contents/ui/main.qml PRE-CREATION
> autotests/data/testjsonmetadatapackage/metadata.json PRE-CREATION
> autotests/querytest.cpp 0a2f11a
> src/kpackage/package.cpp 8416054
> src/kpackage/packageloader.cpp 1e1e382
> src/kpackage/private/packagejobthread.cpp 444dacc
> src/kpackage/private/packages.cpp 2f6bbcf
>
> Diff: https://git.reviewboard.kde.org/r/126348/diff/
>
>
> Testing
> ---
>
> Tests still pass (except for
> https://build.kde.org/job/kpackage%20master%20stable-kf5-qt5/37/PLATFORM=Linux,compiler=gcc/testReport/,
> that is not passing in master).
> Adds a test plugin with a json file in it.
>
>
> Thanks,
>
> Aleix Pol Gonzalez
>
>
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 126320: Read KPluginMetada's property X-Plasma-ComponentTypes as a stringlist
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126320/#review89458 --- Ship it! src/plasma/scripting/scriptengine.cpp (line 89) <https://git.reviewboard.kde.org/r/126320/#comment61223> readStringList() is static so KPluginMetaData:: is probably the better way to call it - Alex Richardson On Dec. 11, 2015, 6:48 p.m., Martin Klapetek wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/126320/ > --- > > (Updated Dec. 11, 2015, 6:48 p.m.) > > > Review request for KDE Frameworks, Plasma and Alex Richardson. > > > Repository: plasma-framework > > > Description > --- > > plasma-scriptengine.desktop defines the property "X-Plasma-ComponentTypes" > as Type=QStringList. When reading it using KPluginMetaData::value(..) it > expects a QString back. This used to work but regressed in kcoreaddons in > commit cfd18cf09b559a050fd6a2680ad4e71eeb950383. Now I'm not sure if calling > KPluginMetaData::value(..) on a property that is known to be a stringlist > should actually return a QString (Alex?), but making it read the property > as a stringlist works and is correct and also fixes Plasma startup for me. > > > Diffs > - > > src/plasma/scripting/scriptengine.cpp 1b132de > > Diff: https://git.reviewboard.kde.org/r/126320/diff/ > > > Testing > --- > > Plasma would get stuck on startup, now I can run Plasma again. > > > Thanks, > > Martin Klapetek > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 126343: KPluginMetaData: Add fields used by KAboutData and introduce conversion function
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126343/ --- Review request for KDE Frameworks. Repository: kcoreaddons Description --- Add KPluginMetaData::translators() and KAboutPerson::fromJson() Plugins could have a list of translators in KDE4 through KAboutData so this commit restores this for KF5. As a side effect of now having authors() and translators() returning a list of KAboutPerson I also added KAboutPerson::fromJSON() as publc function but I can make that internal API instead if it's preferred. --- Add KPluginMetaData::copyrightText(), extraInformation() and otherContributors() Now KPluginMetaData has well defined keys for all the information used by KAboutData. This means we can easily create KAboutData objects from a KPluginMetaData object e.g. for use in an KAboutApplicationDialog. For example Okular uses this to display information about generator that is being used for the current document. --- Add KAboutData::fromPluginMetaData(const KPluginMetaData ) This is useful e.g. to create an KAboutApplicationDialog for plugins Diffs - autotests/kpluginmetadatatest.cpp 25c8b8d5351eb5c5c784d528f9e90dfc8ea77982 src/lib/kaboutdata.h 0dbd7a0ce7ec9220614c5f824a0f379ea5f45559 src/lib/kaboutdata.cpp 5d9a55ea027fc1b8fb6e6394d72cf48f142bb664 src/lib/plugin/kpluginmetadata.h a91b94ad404b6839aa3b8d13b8da19f407708c1b src/lib/plugin/kpluginmetadata.cpp 36743600788693b6449d065f0448e3195445772e Diff: https://git.reviewboard.kde.org/r/126343/diff/ Testing --- Tests pass, used by KAboutApplicationDialog in okular: https://git.reviewboard.kde.org/r/126193/ Thanks, Alex Richardson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 126339: remove kdewin dependency
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126339/#review89464 --- Ship it! Looks good to me - Alex Richardson On Dec. 14, 2015, 10:37 a.m., Patrick Spendrin wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/126339/ > --- > > (Updated Dec. 14, 2015, 10:37 a.m.) > > > Review request for KDE Frameworks and Martin Tobias Holmedahl Sandsmark. > > > Repository: khtml > > > Description > --- > > This removes the direct kdewin dependency by replacing problematic > function calls (uname, snprintf) with their Qt counterparts. > Some leftover unix header includes removed too. > > > Diffs > - > > CMakeLists.txt 51fe02a01c8166046d1c6085ec4a5b6e617e1fea > src/ecma/kjs_binding.cpp 5e122f0f0d70e6734565e7ab205d14a92c79d287 > src/ecma/kjs_navigator.cpp 2f7be8d11e5af84e08ac640ccbc97f70aeac8abd > src/ecma/kjs_proxy.h 24abd1e1bac0932f8829f02185953142c74aadc8 > src/ecma/kjs_proxy.cpp 20430f48fce986ca654c49c5771ad839845f11ab > src/khtml_pagecache.cpp 8e1841f6b0e816dfd8faa76f2191b269c4716011 > src/khtml_part.cpp adbcd800a526e9f8fd92a553e62ee64791872938 > src/kmultipart/kmultipart.cpp 1ad3bbb9b6d6a022799d5ef85f426fc9f911d45b > > Diff: https://git.reviewboard.kde.org/r/126339/diff/ > > > Testing > --- > > Windows, Linux compiles. > > > Thanks, > > Patrick Spendrin > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 126320: Read KPluginMetada's property X-Plasma-ComponentTypes as a stringlist
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126320/#review89459 --- src/plasma/scripting/scriptengine.cpp (line 67) <https://git.reviewboard.kde.org/r/126320/#comment61224> This should probably also be changed to QStringList? - Alex Richardson On Dec. 11, 2015, 6:48 p.m., Martin Klapetek wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/126320/ > --- > > (Updated Dec. 11, 2015, 6:48 p.m.) > > > Review request for KDE Frameworks, Plasma and Alex Richardson. > > > Repository: plasma-framework > > > Description > --- > > plasma-scriptengine.desktop defines the property "X-Plasma-ComponentTypes" > as Type=QStringList. When reading it using KPluginMetaData::value(..) it > expects a QString back. This used to work but regressed in kcoreaddons in > commit cfd18cf09b559a050fd6a2680ad4e71eeb950383. Now I'm not sure if calling > KPluginMetaData::value(..) on a property that is known to be a stringlist > should actually return a QString (Alex?), but making it read the property > as a stringlist works and is correct and also fixes Plasma startup for me. > > > Diffs > - > > src/plasma/scripting/scriptengine.cpp 1b132de > > Diff: https://git.reviewboard.kde.org/r/126320/diff/ > > > Testing > --- > > Plasma would get stuck on startup, now I can run Plasma again. > > > Thanks, > > Martin Klapetek > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 126348: Make it possible to provide the metadata in json
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126348/#review89482
---
Can't really comment on KPackage internals, but looks good to me other than
this one issue
src/kpackage/private/packagejobthread.cpp (line 143)
<https://git.reviewboard.kde.org/r/126348/#comment61242>
I think it would be better if this code was added to the KPluginMetaData
ctor (in an `if (path.endswith(".json"))` branch.
- Alex Richardson
On Dec. 14, 2015, 5:11 p.m., Aleix Pol Gonzalez wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126348/
> ---
>
> (Updated Dec. 14, 2015, 5:11 p.m.)
>
>
> Review request for KDE Frameworks and Plasma.
>
>
> Repository: kpackage
>
>
> Description
> ---
>
> Due to KCoreAddons transition from using desktop files to json files,
> KPackage ended in a weird state where desktop files were allowed to use
> desktop files but not json.
>
> This patch makes sure that manifest.json files are also acceptable.
>
>
> Diffs
> -
>
> autotests/querytest.cpp 0a2f11a
> src/kpackage/package.cpp 8416054
> src/kpackage/packageloader.cpp 1e1e382
> src/kpackage/private/packagejobthread.cpp 444dacc
> src/kpackage/private/packagejobthread_p.h 1babf0b
> src/kpackage/private/packages.cpp 2f6bbcf
>
> Diff: https://git.reviewboard.kde.org/r/126348/diff/
>
>
> Testing
> ---
>
> Tests still pass (except for
> https://build.kde.org/job/kpackage%20master%20stable-kf5-qt5/37/PLATFORM=Linux,compiler=gcc/testReport/,
> that is not passing in master).
> Adds a test plugin with a json file in it.
>
>
> Thanks,
>
> Aleix Pol Gonzalez
>
>
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 126196: Fix regression caused by RR 125527
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126196/ --- (Updated Nov. 30, 2015, 8:40 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks and Jarosław Staniek. Changes --- Submitted with commit 78d740aece90b676086412f977d4cd691791bd8f by Alex Richardson to branch master. Repository: kcoreaddons Description --- Make sure that applications using kcoreaddons_desktop_to_json() that depend on reading the MimeType property of the root object still work See https://git.reviewboard.kde.org/r/125527/ Diffs - src/lib/plugin/desktopfileparser.cpp eae91881bafc93b047b81a2b21634223dd5d89f4 Diff: https://git.reviewboard.kde.org/r/126196/diff/ Testing --- compiles, tests pass Thanks, Alex Richardson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 125527: Add mimeTypes() to KPluginMetaData
> On Nov. 28, 2015, 7:10 p.m., Jarosław Staniek wrote:
> > Hi, I see one regression.
> > Before this change I had "MimeType" field. And I've been using this field.
> >
> > Now I miss miss this field and have "KPlugin/MimeTypes" field. This means
> > plugin error if KF5 5.16.x is installed. I cannot fix my sources: if I do
> > users of KF5 <= 5.15.x will get equivalent error.
Hmm I hadn't thought of that. Sorry for causing a regression. I assume you are
using kcoreaddons_desktop_to_json() to generate the JSON file otherwise this
issue shouldn't appear
I see a few ways of working around this in the sources:
- Depend on KF >= 5.16 (easiest but unlikely to be possible)
- Change MimeType= into X-MyApp-MimeType= and then read that field
- You could use JSON files directly instead of .desktop files
- a) add the MimeType string to the root object
- b) use the 5.16 JSON with a string list and use
KPluginMetaData::readStringList(rawData()["KPlugin"],
QStringLiteral("MimeTypes")); for KF <= 5.16
- Alex
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125527/#review88915
-------
On Oct. 6, 2015, 12:42 p.m., Alex Richardson wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125527/
> ---
>
> (Updated Oct. 6, 2015, 12:42 p.m.)
>
>
> Review request for KDE Frameworks, David Faure and Sebastian Kügler.
>
>
> Repository: kcoreaddons
>
>
> Description
> ---
>
> When loading a .desktop file this will parse the XDG MimeType= key
>
> Contrary to KService we don't merge these with ServiceTypes but rather
> have them as a separate property.
>
> REVIEW: 125261
>
> KPluginMetaData: Warn when a list entry is not a JSON list
>
> We still convert single values to a list with one entry but now also
> output a warning.
>
> REVIEW: 125261
>
>
> Diffs
> -
>
> autotests/data/fakeplugin.desktop 30ff9a98d4587507620f70e3c271456877ab1812
> autotests/kpluginmetadatatest.cpp 3af5e1b842b0bc231a5ac001112e141f751d2ff5
> src/lib/plugin/desktopfileparser.h 98d47ddf0f877c4a25928026b3d5fe169cfc9e75
> src/lib/plugin/desktopfileparser.cpp
> 0b03eb154deb58840c91c12658780c0d492b593c
> src/lib/plugin/kpluginmetadata.h 183b0d0583259f7ed74e97858a68c5c388fd0a9a
> src/lib/plugin/kpluginmetadata.cpp b13d6dd52827cc03d9473600aa4d2bab8a95a1d4
>
> Diff: https://git.reviewboard.kde.org/r/125527/diff/
>
>
> Testing
> ---
>
> This is the same as review 125261 but for some reason I kept getting a
> internal server error when I tried to update it.
>
>
> Used this for a WIP port of Okular to new Plugin loading. Could also be used
> by KDevelop instead of the custom X-KDevelop-SupportedMimeTypes property
>
> Requires https://git.reviewboard.kde.org/r/125263/ to ensure that there are
> no regressions
>
>
> Thanks,
>
> Alex Richardson
>
>
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 126196: Fix regression caused by RR 125527
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126196/ --- Review request for KDE Frameworks and Jarosław Staniek. Repository: kcoreaddons Description --- Make sure that applications using kcoreaddons_desktop_to_json() that depend on reading the MimeType property of the root object still work See https://git.reviewboard.kde.org/r/125527/ Diffs - src/lib/plugin/desktopfileparser.cpp eae91881bafc93b047b81a2b21634223dd5d89f4 Diff: https://git.reviewboard.kde.org/r/126196/diff/ Testing --- compiles, tests pass Thanks, Alex Richardson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 125869: Convert all io slave .protocol data to json and embed it.
> On Nov. 3, 2015, 9:47 p.m., Albert Astals Cid wrote: > > How does this work without modifying > > KProtocolInfoPrivate::KProtocolInfoPrivate? > > Christoph Cullmann wrote: > You mean the JSON stuff? That was implemented in > https://git.reviewboard.kde.org/r/125830/ > For the http slave, that already works nicely, but we missed that > "ExtraNames" as used by other slaves need i18n care. > > Albert Astals Cid wrote: > i see, i did not see that there's two almost copied > KProtocolInfoPrivate::KProtocolInfoPrivate implementations > > So the json magic stuff (you can see > ./src/ioslaves/http/kcookiejar/kcookiejar.json in kio) only works for > Description and Name inside KPlugin, someone would need to update > createjsoncontext.py and filljsonfrompo.py so they also take into account > ExtraNames inside childs of "KDE-KIO-Protocols" and then make that new code > from 125830 extract the correct translation from the json. Reading the translated string can be done using `KPluginMetaData::readTranslatedString()` - Alex --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/125869/#review87966 --- On Nov. 1, 2015, 6:13 p.m., Christoph Cullmann wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/125869/ > --- > > (Updated Nov. 1, 2015, 6:13 p.m.) > > > Review request for KDE Frameworks, Albert Astals Cid, Alex Richardson, David > Faure, and Luigi Toscano. > > > Repository: kio > > > Description > --- > > Convert all io slave .protocol data to json and embed it. > Allows easier deployment of the slaves. > > > Diffs > - > > src/ioslaves/http/CMakeLists.txt 76a8e28 > src/ioslaves/help/main_ghelp.cpp 59c8558 > src/ioslaves/help/main.cpp 9939196 > src/ioslaves/help/help.protocol 1deefe5 > src/ioslaves/help/help.json PRE-CREATION > src/ioslaves/help/ghelp.protocol d2a642a > src/ioslaves/help/ghelp.json PRE-CREATION > src/ioslaves/help/CMakeLists.txt 867b59d > src/ioslaves/ftp/ftp.protocol 4c5f80c > src/ioslaves/ftp/ftp.json PRE-CREATION > src/ioslaves/ftp/ftp.cpp 382723a > src/ioslaves/ftp/CMakeLists.txt 04f5600 > src/ioslaves/file/file.protocol 523c0f5 > src/ioslaves/file/file.json PRE-CREATION > src/ioslaves/file/file.cpp 5ef1587 > src/ioslaves/file/CMakeLists.txt cb85cfb > autotests/kprotocolinfotest.cpp fa3ad38 > src/ioslaves/http/http.protocol 49e5dc5 > src/ioslaves/http/https.protocol c15d06f > src/ioslaves/http/webdav.protocol 05c977a > src/ioslaves/http/webdavs.protocol d5e4b2f > src/ioslaves/trash/CMakeLists.txt 05161cd > src/ioslaves/trash/kio_trash.cpp cb23169 > src/ioslaves/trash/trash.json PRE-CREATION > src/ioslaves/trash/trash.protocol 7430575 > > Diff: https://git.reviewboard.kde.org/r/125869/diff/ > > > Testing > --- > > Tests still work (one needed patching, as the exec line contains now the full > path). > > Correction: Somehow the ./autotests/jobtest test is unstable for me here, > sometimes it works, sometimes not :/ but even without this change. > > > Thanks, > > Christoph Cullmann > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 125797: protocoltojson application
> On Oct. 26, 2015, 5:27 p.m., Alex Richardson wrote:
> > The protocoltojson program will have to read the plugin metadata.json file
> > to insert the "KDE-KIO-Protocols" to that json file as it is not possible
> > to embed more than one JSON file into a Qt plugin.
> >
> > I am not sure whether we need a CMake macro, to me this is a conversion
> > that should be done once and then we store the JSON file in the repository
> > so that we don't increase the build time.
>
> Christoph Cullmann wrote:
> Actually ioslaves have atm no other json stored, this would be the only
> meta data added to them, I think no ioslave is at all a qt plugin and to stay
> compatible, I would only add a dummy class there with this json embedded to
> have the meta data available. But perhaps I am mistaken.
> For the one time conversion: I am not sure that the .protocol files will
> be deprecated, I thought to do it like ATM for the .desktop files for the
> plugins for which we kept the .desktop files, too.
>
> David Faure wrote:
> IIRC we kept .desktop files because there was no other way to translate
> them, back then.
>
> But that's not an issue anymore, and not an issue for .protocol files
> anyway.
>
> So when converting an ioslave to json, rather than just change its
> cmakelists, we could also just convert the .protocol to json, either with a
> one-time conversion tool (now that you wrote it...) or by hand, reading some
> documentation on the format.
> Seems simpler than more cmake magic.
>
> KIO will need to keep being able to parse .protocol files anyway, for
> compat reasons, that's unrelated.
>
> Christoph Cullmann wrote:
> Ok ;) That makes its easier, no cmake foo needed, cool.
> For the second issue: I am right, or? There is no other meta data we need
> inside the io slave?
>
> Christoph Cullmann wrote:
> My idea would be to add some:
>
> /**
> * Pseudo plugin class to embedd meta data
> */
> class KIOPluginForMetaData : public QObject
> {
> Q_OBJECT
> Q_PLUGIN_METADATA(IID "org.kde.kio.slave.http" FILE "http.json")
> };
>
> to each slave that then uses the meta data json as above.
>
> Moc will then embedd that like needed and no more stuff is needed to have
> the protocols meta data in the .so files, at least that seems to work for me.
Ah I thought there was already some JSON embedded. This makes it much simpler.
I don't think any of the default KPluginMetaData values are useful for
KIOSlaves (e.g. Icon can be different for each protocol) so only having these
JSON keys should be fine.
We can always add KPlugin { foo: "" } later if we need it.
- Alex
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125797/#review87444
---
On Oct. 26, 2015, 5:03 p.m., Christoph Cullmann wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125797/
> ---
>
> (Updated Oct. 26, 2015, 5:03 p.m.)
>
>
> Review request for KDE Frameworks, Alex Richardson and David Faure.
>
>
> Repository: kio
>
>
> Description
> ---
>
> Application to convert multiple .protocol files into on json map.
> If this is acceptable my next steps would be:
>
> 1) use this, to generate json files and embedded them in io slaves
> 2) extend the protocol factory to search in the embedded files
>
> That would make the protocol factory parts of:
> https://git.reviewboard.kde.org/r/125778/ not needed.
>
>
> Diffs
> -
>
> src/CMakeLists.txt f65ad8e
> src/protocoltojson/CMakeLists.txt PRE-CREATION
> src/protocoltojson/main.cpp PRE-CREATION
>
> Diff: https://git.reviewboard.kde.org/r/125797/diff/
>
>
> Testing
> ---
>
> Feed in http slave files:
>
> ./protocoltojson -o http.json
> /local/cullmann/kf5/src/frameworks/kio/src/ioslaves/http/*.protocol
>
> We get mapping:
>
> {
> "http": {
> "Class": ":internet",
> "Icon": "text-html",
> "X-DocPath": "kioslave5/http/index.html",
> "defaultMimetype": "application/octet-stream",
> "deleting": true,
> "determineMimetypeFromExtension":
Re: Review Request 125797: protocoltojson application
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125797/#review87527
---
Ship it!
- Alex Richardson
On Oct. 26, 2015, 5:03 p.m., Christoph Cullmann wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125797/
> ---
>
> (Updated Oct. 26, 2015, 5:03 p.m.)
>
>
> Review request for KDE Frameworks, Alex Richardson and David Faure.
>
>
> Repository: kio
>
>
> Description
> ---
>
> Application to convert multiple .protocol files into on json map.
> If this is acceptable my next steps would be:
>
> 1) use this, to generate json files and embedded them in io slaves
> 2) extend the protocol factory to search in the embedded files
>
> That would make the protocol factory parts of:
> https://git.reviewboard.kde.org/r/125778/ not needed.
>
>
> Diffs
> -
>
> src/CMakeLists.txt f65ad8e
> src/protocoltojson/CMakeLists.txt PRE-CREATION
> src/protocoltojson/main.cpp PRE-CREATION
>
> Diff: https://git.reviewboard.kde.org/r/125797/diff/
>
>
> Testing
> ---
>
> Feed in http slave files:
>
> ./protocoltojson -o http.json
> /local/cullmann/kf5/src/frameworks/kio/src/ioslaves/http/*.protocol
>
> We get mapping:
>
> {
> "http": {
> "Class": ":internet",
> "Icon": "text-html",
> "X-DocPath": "kioslave5/http/index.html",
> "defaultMimetype": "application/octet-stream",
> "deleting": true,
> "determineMimetypeFromExtension": false,
> "exec": "kf5/kio/http",
> "input": "none",
> "maxInstances": 20,
> "maxInstancesPerHost": 5,
> "output": "filesystem",
> "protocol": "http",
> "reading": true,
> "writing": true
> },
> "https": {
> "Class": ":internet",
> "Icon": "text-html",
> "X-DocPath": "kioslave5/http/index.html",
> "config": "http",
> "defaultMimetype": "application/octet-stream",
> "deleting": true,
> "determineMimetypeFromExtension": false,
> "exec": "kf5/kio/http",
> "input": "none",
> "maxInstances": 20,
> "maxInstancesPerHost": 5,
> "output": "filesystem",
> "protocol": "https",
> "reading": true,
> "writing": true
> },
> "webdav": {
> "Class": ":internet",
> "Icon": "folder-remote",
> "X-DocPath": "kioslave5/webdav/index.html",
> "defaultMimetype": "application/octet-stream",
> "deleteRecursive": true,
> "deleting": true,
> "determineMimetypeFromExtension": false,
> "exec": "kf5/kio/http",
> "input": "none",
> "listing": [
> "Name",
> "Type",
> "Size",
> "Date",
> "AccessDate",
> "Access"
> ],
> "makedir": true,
> "maxInstances": 20,
> "maxInstancesPerHost": 5,
> "moving": true,
> "output": "filesystem",
> "protocol": "webdav",
> "reading": true,
> "writing": true
> },
> "webdavs": {
> "Class": ":internet",
> "Icon": "folder-remote",
> "X-DocPath": "kioslave5/webdav/index.html",
> "config": "webdav",
> "defaultMimetype": "application/octet-stream",
> "deleteRecursive": true,
> "deleting": true,
> "determineMimetypeFromExtension": false,
> "exec": "kf5/kio/http",
> "input": "none",
> "listing": [
> "Name",
> "Type",
> "Size",
> "Date",
> "AccessDate",
> "Access"
> ],
> "makedir": true,
> "maxInstances": 20,
> "maxInstancesPerHost": 5,
> "moving": true,
> "output": "filesystem",
> "protocol": "webdavs",
> "reading": true,
> "writing": true
> }
> }
>
>
> Thanks,
>
> Christoph Cullmann
>
>
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 125830: Read protocol info from plugin metadata
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125830/#review87561
---
Looks good to me if it still works.
I don't really like all the code being duplicated with config.readEntry()
changing to value().toFoo() but I don't see an easy way to avoid the
duplication.
src/core/kprotocolinfo.cpp (line 126)
<https://git.reviewboard.kde.org/r/125830/#comment60120>
toBool() has has bool defaultValue parameter which could be used instead
src/core/kprotocolinfofactory.cpp (line 87)
<https://git.reviewboard.kde.org/r/125830/#comment60119>
Probably easier to use KPluginLoader::findPlugins() instead, it only
returns plugins with metadata:
Something like this should work (untested)
```
foreach (const KPluginMetaData& md : KPluginLoader::findPlugins("kf5/kio"))
{
const QString slavePath = md.fileName()
const QJsonObject
protocols(md.rawData().value(QStringLiteral("KDE-KIO-Protocols")).toObject());
// add all protocols, does nothing if object invalid
for (auto it = protocols.begin(); it != protocols.end(); ++it) {
// skip empty objects
const QJsonObject protocol(it.value().toObject());
if (protocol.isEmpty()) {
continue;
}
// add to cache, skip double entries
if (!m_cache.contains(it.key())) {
m_cache.insert(it.key(), new KProtocolInfoPrivate(it.key(),
slavePath, protocol));
}
}
```
- Alex Richardson
On Oct. 27, 2015, 10:31 p.m., Christoph Cullmann wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125830/
> ---
>
> (Updated Oct. 27, 2015, 10:31 p.m.)
>
>
> Review request for KDE Frameworks, Alex Richardson and David Faure.
>
>
> Repository: kio
>
>
> Description
> ---
>
> Extends the protocol factory and co. to allow reading of protocol data from
> embedded json.
> Allows to deploy io slaves without anything else than the io slave itself in
> librarypath kf5/kio.
>
> I changed to factory to ALWAYS fill its cache for any request, as now the
> determination which protocols are around is more expensive than just some
> directory traversal.
>
> If this preview is ok, I will do that for all other shipped slaves and update
> this request.
>
>
> Diffs
> -
>
> src/core/kprotocolinfo.cpp 8a02f7a
> src/core/kprotocolinfo_p.h c3dea6b
> src/core/kprotocolinfofactory.cpp 29ba8f4
> src/core/kprotocolinfofactory_p.h aa79fc5
> src/ioslaves/http/http.cpp 1727d41
> src/ioslaves/http/http.json PRE-CREATION
>
> Diff: https://git.reviewboard.kde.org/r/125830/diff/
>
>
> Testing
> ---
>
> http slave still seems to be found and work.
>
>
> Thanks,
>
> Christoph Cullmann
>
>
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 125797: protocoltojson application
> On Oct. 25, 2015, 9:32 p.m., Christoph Cullmann wrote:
> > I think one needs to add a outer scope with some "KIO-Protocol-Info" or so
> > key to be able to check if we have valid data inside a lib.
> > But I am not sure how to name that.
>
> Alex Merry wrote:
> How about "KDE-KIO-Protocols"? I think having a KDE prefix in keys for
> KDE software is good practice.
>
> I'd have thought the "exec" key is unnecessary if you're embedding it
> into the exec.
>
> I have to say that the "Class", "Icon" and "X-DocPath" entries look a
> little out of place; I don't know if we'd maybe want to take the opportunity
> to adjust those (to "class", "icon" and "docpath", I guess).
>
> Christoph Cullmann wrote:
> > How about "KDE-KIO-Protocols"? I think having a KDE prefix in keys for
> KDE software is good practice.
> Sound good too me.
>
> > I'd have thought the "exec" key is unnecessary if you're embedding it
> into the exec.
> > I have to say that the "Class", "Icon" and "X-DocPath" entries look a
> little out of place; I don't know if we'd maybe want to take the opportunity
> to adjust those (to "class", "icon" and "docpath", I guess).
> Actually I would like to keep all keys "as is" to have some easy 1:1
> mapping to the .protocol files that still will be kept like they are.
> Thought I can add that renaming, if wanted by David, too.
>
> Btw., if this approach would be OK, would you help me to create a cmake
> macro for that in kio like the one for desktoptojson?
> My CMake Foo is very limited (beside copy'n'paste and adjust) ;=)
>
> Alex Merry wrote:
> I can definitely see the advantage of keeping the keys the same, it just
> struck me that if we *do* want to change the keys, this is the perfect
> opportunity.
>
> Sure, I can help with the CMake stuff.
>
> Christoph Cullmann wrote:
> I was confused by the names during typing in the copy list, too ;)
> Let's see what David thinks about the helper, if he is OK, I will come
> back to that help offer.
Aren't the "protocol" and "exec" key redundant? exec is not needed if we embed
it in the library and the value for protocol seems to always(?) be the same as
the object key.
- Alex
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125797/#review87382
---
On Oct. 25, 2015, 9:20 p.m., Christoph Cullmann wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125797/
> ---
>
> (Updated Oct. 25, 2015, 9:20 p.m.)
>
>
> Review request for KDE Frameworks and David Faure.
>
>
> Repository: kio
>
>
> Description
> ---
>
> Application to convert multiple .protocol files into on json map.
> If this is acceptable my next steps would be:
>
> 1) use this, to generate json files and embedded them in io slaves
> 2) extend the protocol factory to search in the embedded files
>
> That would make the protocol factory parts of:
> https://git.reviewboard.kde.org/r/125778/ not needed.
>
>
> Diffs
> -
>
> src/CMakeLists.txt f65ad8e
> src/protocoltojson/CMakeLists.txt PRE-CREATION
> src/protocoltojson/main.cpp PRE-CREATION
>
> Diff: https://git.reviewboard.kde.org/r/125797/diff/
>
>
> Testing
> ---
>
> Feed in http slave files:
>
> ./protocoltojson -o http.json
> /local/cullmann/kf5/src/frameworks/kio/src/ioslaves/http/*.protocol
>
> We get mapping:
>
> {
> "http": {
> "Class": ":internet",
> "Icon": "text-html",
> "X-DocPath": "kioslave5/http/index.html",
> "defaultMimetype": "application/octet-stream",
> "deleting": true,
> "determineMimetypeFromExtension": false,
> "exec": "kf5/kio/http",
> "input": "none",
> "maxInstances": 20,
> "maxInstancesPerHost": 5,
> "output": "filesystem",
> "protocol": "http",
> "reading": true,
> "writing": true
> },
> "https": {
> "Class": ":internet",
> "Icon": "text-html",
> "X-DocPath": "kioslave5/http/index.html",
> "config": "http",
> "defaultMimetype": "application/octet-stream",
> "deleting": true,
> "determineMimetypeFromExtension": false,
> "exec": "kf5/kio/http",
> "input": "none",
> "maxInstances": 20,
> "maxInstancesPerHost": 5,
> "output": "filesystem",
> "protocol": "https",
> "reading": true,
> "writing": true
> },
> "webdav": {
> "Class": ":internet",
> "Icon": "folder-remote",
> "X-DocPath": "kioslave5/webdav/index.html",
> "defaultMimetype": "application/octet-stream",
>
Re: Review Request 125750: Reduce some allocations
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/125750/#review87265 --- Ship it! There's probably a way of avoinding the allocation and only iterating over the line once but it's a big improvement like this anyway. Iterating over a few more bytes that are already in the cache shouldn't be noticable, whereas saving one call to malloc() per .desktop file line is a big improvement. Also calling contains() it makes the code a lot easier to understand and maintain. I don't really know the details of how plasma plugins are handled but for normal shared libraries using .json instead of .desktop is the way to go. That's why there's the transitional comment in the source. If plasma plugins have metadata.desktop files, using metadata.json instead should (hopefully) just work. However, I haven't looked into the plasma code so I can't be certain. src/lib/plugin/desktopfileparser.cpp (line 97) <https://git.reviewboard.kde.org/r/125750/#comment59944> This should be '\' Looks I didn't test escaped values if the tests still pass... - Alex Richardson On Oct. 22, 2015, 2:20 p.m., Aleix Pol Gonzalez wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/125750/ > --- > > (Updated Oct. 22, 2015, 2:20 p.m.) > > > Review request for KDE Frameworks and Sebastian Kügler. > > > Repository: kcoreaddons > > > Description > --- > > I know this could be done so much better and there's a comment saying it's > transitional (heh), but this happens to be called a ton of times on every > plasma boot through [1]. > Actually, why is it transitional? Are we supposed to move from > metadata.desktop to metadata.json? Or is it because we should all be using > `kpackagetool5 --generate-index`? > > [1] KPluginMetaData::loadFromDesktopFile(QString const&, QStringList const&) > > > Diffs > - > > src/lib/plugin/desktopfileparser.cpp 7a4556a > > Diff: https://git.reviewboard.kde.org/r/125750/diff/ > > > Testing > --- > > Tests pass, plasma still boots. > > > Thanks, > > Aleix Pol Gonzalez > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 125262: Parse ServiceType files when reading .desktop files
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125262/
---
(Updated Oct. 12, 2015, 10:25 p.m.)
Status
--
This change has been marked as submitted.
Review request for KDE Frameworks, David Faure and Sebastian Kügler.
Changes
---
Submitted with commit e3b1d023dcf2c10f5b4feb4eb1973bcc0fbdb954 by Alex
Richardson to branch master.
Repository: kcoreaddons
Description
---
Parse ServiceType files when reading .desktop files
---
Remove lots of duplicated code for desktop{tojson,fileparser}.cpp
The only reason these were copy-pasted are minor differences in the
output which is now fixed by using qInstallMessageHandler and the
ability to generate JSON compatible with the first published version
of desktoptojson (which is hopefully no longer used and can be removed
soon)
---
QCommandLineParser uses -v for --version so just use --verbose
Otherwise the whole QCommandlineOption is ignored and there is no way
to enable verbose mode
---
desktopparser: Use more categorized logging
---
desktopparser: Allow passing relative paths to service type files
---
Add KPluginMetaData::fromDesktopFile()
This function allows specifying a list of service type files to be
parsed when loading the .desktop file.
---
desktopparser: Improve warning messages and add new unit test
The new test checks how the desktop parser handles service type files
with invalid property definitions
---
desktopparser: Fix parsing of double and bool values
QString::compare returns 0 on equal and make sure that we don't assign
the parsed double to an integer local variable
---
Add another unit test for desktop parsing with service types
Test that all supported types are converted correctly
---
Allow setting service types in kcoreaddons_desktop_to_json()
Diffs
-
KF5CoreAddonsMacros.cmake acfcaa3069991395d83923bcc30cd08f231c30eb
autotests/data/servicetypes/bad-groups-input.desktop PRE-CREATION
autotests/data/servicetypes/bad-groups-servicetype.desktop PRE-CREATION
autotests/data/servicetypes/example-input.desktop PRE-CREATION
autotests/data/servicetypes/example-servicetype.desktop PRE-CREATION
autotests/data/servicetypes/fake-kdevelopplugin.desktop PRE-CREATION
autotests/desktoptojsontest.cpp 63ba2bee10de77ae6c0697cb654defa67d069f65
autotests/kpluginmetadatatest.cpp 82ec06a91005ffd2c8e50d7a641706c6c7beac6b
src/desktoptojson/CMakeLists.txt 94a199d8fa44a21b15e24c2e4f42551adada8f72
src/desktoptojson/desktoptojson.h bfa38b0f5ddd0581ad176d854614bc9c80dd139a
src/desktoptojson/desktoptojson.cpp f07de309a667aaa017a22f761222f2b5f6694ccb
src/desktoptojson/main.cpp 9bac8ff55d005d1944c04f557aa9601de2b0ca15
src/lib/plugin/desktopfileparser.cpp 0f71ead0e83270d757179ad233982beadb6c9806
src/lib/plugin/desktopfileparser_p.h 767146e691a9b6c9827c5b7bcd9b73c98ff868e3
src/lib/plugin/kpluginmetadata.h 59b6a9db0811d24c9ad8a3e86212ea50b9cd95ce
src/lib/plugin/kpluginmetadata.cpp f7942b1aef3f165c0fab2a0cb4422422342e5f8d
Diff: https://git.reviewboard.kde.org/r/125262/diff/
Testing
---
Added some unit test and they pass
Thanks,
Alex Richardson
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 125527: Add mimeTypes() to KPluginMetaData
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/125527/ --- (Updated Oct. 6, 2015, 11:42 a.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks, David Faure and Sebastian Kügler. Changes --- Submitted with commit 0e73135459d3d9088702d9ec4f4b4b4ffd527024 by Alex Richardson to branch master. Repository: kcoreaddons Description --- When loading a .desktop file this will parse the XDG MimeType= key Contrary to KService we don't merge these with ServiceTypes but rather have them as a separate property. REVIEW: 125261 KPluginMetaData: Warn when a list entry is not a JSON list We still convert single values to a list with one entry but now also output a warning. REVIEW: 125261 Diffs - autotests/data/fakeplugin.desktop 30ff9a98d4587507620f70e3c271456877ab1812 autotests/kpluginmetadatatest.cpp 3af5e1b842b0bc231a5ac001112e141f751d2ff5 src/lib/plugin/desktopfileparser.h 98d47ddf0f877c4a25928026b3d5fe169cfc9e75 src/lib/plugin/desktopfileparser.cpp 0b03eb154deb58840c91c12658780c0d492b593c src/lib/plugin/kpluginmetadata.h 183b0d0583259f7ed74e97858a68c5c388fd0a9a src/lib/plugin/kpluginmetadata.cpp b13d6dd52827cc03d9473600aa4d2bab8a95a1d4 Diff: https://git.reviewboard.kde.org/r/125527/diff/ Testing --- This is the same as review 125261 but for some reason I kept getting a internal server error when I tried to update it. Used this for a WIP port of Okular to new Plugin loading. Could also be used by KDevelop instead of the custom X-KDevelop-SupportedMimeTypes property Requires https://git.reviewboard.kde.org/r/125263/ to ensure that there are no regressions Thanks, Alex Richardson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 125263: Adapt KPluginInfo to introduction of KPluginMetaData::mimeTypes()
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/125263/ --- (Updated Oct. 6, 2015, 11:43 a.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks, David Faure and Sebastian Kügler. Changes --- Submitted with commit 3cc94ba4df468433e96d4c2ce6d945db920d1246 by Alex Richardson to branch master. Repository: kservice Description --- Keep MIME types separate when converting KPluginInfo to KPluginMetaData Diffs - autotests/kplugininfotest.cpp c3a0f8f7a79fc9b063bbcf0f52a5442ae0545432 src/services/kplugininfo.cpp 93dcb8fc35930d89e4e60efea755b018d329794f Diff: https://git.reviewboard.kde.org/r/125263/diff/ Testing --- new + old unit tests pass Thanks, Alex Richardson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 125262: Parse ServiceType files when reading .desktop files
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125262/
---
(Updated Oct. 6, 2015, 3:07 p.m.)
Review request for KDE Frameworks, David Faure and Sebastian Kügler.
Changes
---
rebased
Summary (updated)
-
Parse ServiceType files when reading .desktop files
Repository: kcoreaddons
Description (updated)
---
Parse ServiceType files when reading .desktop files
---
Remove lots of duplicated code for desktop{tojson,fileparser}.cpp
The only reason these were copy-pasted are minor differences in the
output which is now fixed by using qInstallMessageHandler and the
ability to generate JSON compatible with the first published version
of desktoptojson (which is hopefully no longer used and can be removed
soon)
---
QCommandLineParser uses -v for --version so just use --verbose
Otherwise the whole QCommandlineOption is ignored and there is no way
to enable verbose mode
---
desktopparser: Use more categorized logging
---
desktopparser: Allow passing relative paths to service type files
---
Add KPluginMetaData::fromDesktopFile()
This function allows specifying a list of service type files to be
parsed when loading the .desktop file.
---
desktopparser: Improve warning messages and add new unit test
The new test checks how the desktop parser handles service type files
with invalid property definitions
---
desktopparser: Fix parsing of double and bool values
QString::compare returns 0 on equal and make sure that we don't assign
the parsed double to an integer local variable
---
Add another unit test for desktop parsing with service types
Test that all supported types are converted correctly
---
Allow setting service types in kcoreaddons_desktop_to_json()
Diffs (updated)
-
KF5CoreAddonsMacros.cmake acfcaa3069991395d83923bcc30cd08f231c30eb
autotests/data/servicetypes/bad-groups-input.desktop PRE-CREATION
autotests/data/servicetypes/bad-groups-servicetype.desktop PRE-CREATION
autotests/data/servicetypes/example-input.desktop PRE-CREATION
autotests/data/servicetypes/example-servicetype.desktop PRE-CREATION
autotests/data/servicetypes/fake-kdevelopplugin.desktop PRE-CREATION
autotests/desktoptojsontest.cpp 63ba2bee10de77ae6c0697cb654defa67d069f65
autotests/kpluginmetadatatest.cpp 82ec06a91005ffd2c8e50d7a641706c6c7beac6b
src/desktoptojson/CMakeLists.txt 94a199d8fa44a21b15e24c2e4f42551adada8f72
src/desktoptojson/desktoptojson.h bfa38b0f5ddd0581ad176d854614bc9c80dd139a
src/desktoptojson/desktoptojson.cpp f07de309a667aaa017a22f761222f2b5f6694ccb
src/desktoptojson/main.cpp 9bac8ff55d005d1944c04f557aa9601de2b0ca15
src/lib/plugin/desktopfileparser.cpp 0f71ead0e83270d757179ad233982beadb6c9806
src/lib/plugin/desktopfileparser_p.h 767146e691a9b6c9827c5b7bcd9b73c98ff868e3
src/lib/plugin/kpluginmetadata.h 59b6a9db0811d24c9ad8a3e86212ea50b9cd95ce
src/lib/plugin/kpluginmetadata.cpp f7942b1aef3f165c0fab2a0cb4422422342e5f8d
Diff: https://git.reviewboard.kde.org/r/125262/diff/
Testing
---
Added some unit test and they pass
Thanks,
Alex Richardson
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 125261: Add mimeTypes() to KPluginMetaData
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/125261/ --- (Updated Oct. 5, 2015, 11:36 a.m.) Status -- This change has been discarded. Review request for KDE Frameworks, David Faure and Sebastian Kügler. Repository: kcoreaddons Description --- When loading a .desktop file this will parse the XDG MimeType= key Contrary to KService we don't merge these with ServiceTypes but rather have them as a separate property. Diffs - autotests/data/fakeplugin.desktop 2e186869bf0cb39277ca3f9d2e8603d64d0223c6 autotests/kpluginmetadatatest.cpp 3af5e1b842b0bc231a5ac001112e141f751d2ff5 src/lib/plugin/desktopfileparser.h 98d47ddf0f877c4a25928026b3d5fe169cfc9e75 src/lib/plugin/desktopfileparser.cpp 0b03eb154deb58840c91c12658780c0d492b593c src/lib/plugin/kpluginmetadata.h 183b0d0583259f7ed74e97858a68c5c388fd0a9a src/lib/plugin/kpluginmetadata.cpp b13d6dd52827cc03d9473600aa4d2bab8a95a1d4 Diff: https://git.reviewboard.kde.org/r/125261/diff/ Testing --- Used this for a WIP port of Okular to new Plugin loading. Could also be used by KDevelop instead of the custom X-KDevelop-SupportedMimeTypes property Requires https://git.reviewboard.kde.org/r/125263/ to ensure that there are no regressions Thanks, Alex Richardson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 125527: Add mimeTypes() to KPluginMetaData
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/125527/ --- Review request for KDE Frameworks. Repository: kcoreaddons Description --- When loading a .desktop file this will parse the XDG MimeType= key Contrary to KService we don't merge these with ServiceTypes but rather have them as a separate property. REVIEW: 125261 KPluginMetaData: Warn when a list entry is not a JSON list We still convert single values to a list with one entry but now also output a warning. REVIEW: 125261 Diffs - autotests/data/fakeplugin.desktop 30ff9a98d4587507620f70e3c271456877ab1812 autotests/kpluginmetadatatest.cpp 3af5e1b842b0bc231a5ac001112e141f751d2ff5 src/lib/plugin/desktopfileparser.h 98d47ddf0f877c4a25928026b3d5fe169cfc9e75 src/lib/plugin/desktopfileparser.cpp 0b03eb154deb58840c91c12658780c0d492b593c src/lib/plugin/kpluginmetadata.h 183b0d0583259f7ed74e97858a68c5c388fd0a9a src/lib/plugin/kpluginmetadata.cpp b13d6dd52827cc03d9473600aa4d2bab8a95a1d4 Diff: https://git.reviewboard.kde.org/r/125527/diff/ Testing --- This is the same as review 125261 but for some reason I kept getting a internal server error when I tried to update it. Used this for a WIP port of Okular to new Plugin loading. Could also be used by KDevelop instead of the custom X-KDevelop-SupportedMimeTypes property Requires https://git.reviewboard.kde.org/r/125263/ to ensure that there are no regressions Thanks, Alex Richardson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 125262: Parse service type files when loading from .desktop
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125262/
---
Review request for KDE Frameworks.
Repository: kcoreaddons
Description
---
KPluginMetadata("foo.desktop") or kcoreaddons_desktop_to_json() do not always
result in the correct JSON for example when a property is
supposed to be a list. the .desktop file parser in kcoreaddons so far had no
way of know this so instead of treating the entry as a
comma-separated list it would simply return a JSON string.
This is a follow up to https://git.reviewboard.kde.org/r/121672/ with a
different solution that also handles service types that are not compiled into
the kcoreaddons library but instead parses the installed servicetype .desktop
file.
It also contains a bit of code-deduplication and porting to categorized logging
which is not directly related to this patch but it would be some effort to
split that into a separate RR.
It is a series of commits with the following messages:
desktopparser: avoid unnecessary utf8 decoding
Parse ServiceType files when reading .desktop files
Remove lots of duplicated code for desktop{tojson,fileparser}.cpp
The only reason these were copy-pasted are minor differences in the
output which is now fixed by using qInstallMessageHandler and the
ability to generate JSON compatible with the first published version
of desktoptojson (which is hopefully no longer used and can be removed
soon)
QCommandLineParser uses -v for --version so just use --verbose
Otherwise the whole QCommandlineOption is ignored and there is no way
to enable verbose mode
desktopparser: Use more categorized logging
desktopparser: Allow passing relative paths to service type files
Add KPluginMetaData::fromDesktopFile()
This function allows specifying a list of service type files to be
parsed when loading the .desktop file.
desktopparser: Improve warning messages and add new unit test
The new test checks how the desktop parser handles service type files
with invalid property definitions
desktopparser: Fix parsing of double and bool values
QString::compare returns 0 on equal and make sure that we don't assign
the parsed double to an integer local variable
Add another unit test for desktop parsing with service types
Test that all supported types are converted correctly
Allow setting service types in kcoreaddons_desktop_to_json()
Diffs
-
KF5CoreAddonsMacros.cmake acfcaa3069991395d83923bcc30cd08f231c30eb
autotests/data/servicetypes/bad-groups-input.desktop PRE-CREATION
autotests/data/servicetypes/bad-groups-servicetype.desktop PRE-CREATION
autotests/data/servicetypes/example-input.desktop PRE-CREATION
autotests/data/servicetypes/example-servicetype.desktop PRE-CREATION
autotests/data/servicetypes/fake-kdevelopplugin.desktop PRE-CREATION
autotests/desktoptojsontest.cpp 64373d5be930426dd8a1f8e455e33c411a4795fd
autotests/kpluginmetadatatest.cpp 3af5e1b842b0bc231a5ac001112e141f751d2ff5
src/desktoptojson/CMakeLists.txt 94a199d8fa44a21b15e24c2e4f42551adada8f72
src/desktoptojson/desktoptojson.h bfa38b0f5ddd0581ad176d854614bc9c80dd139a
src/desktoptojson/desktoptojson.cpp 82626b256df6b3bd106e6d4c6fad84d7d970af37
src/desktoptojson/main.cpp 9bac8ff55d005d1944c04f557aa9601de2b0ca15
src/lib/plugin/desktopfileparser.h 98d47ddf0f877c4a25928026b3d5fe169cfc9e75
src/lib/plugin/desktopfileparser.cpp 0b03eb154deb58840c91c12658780c0d492b593c
src/lib/plugin/kpluginmetadata.h 183b0d0583259f7ed74e97858a68c5c388fd0a9a
src/lib/plugin/kpluginmetadata.cpp b13d6dd52827cc03d9473600aa4d2bab8a95a1d4
Diff: https://git.reviewboard.kde.org/r/125262/diff/
Testing
---
Added some unit test and they pass
Thanks,
Alex Richardson
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 125261: Add mimeTypes() to KPluginMetaData
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/125261/ --- Review request for KDE Frameworks. Repository: kcoreaddons Description --- When loading a .desktop file this will parse the XDG MimeType= key Contrary to KService we don't merge these with ServiceTypes but rather have them as a separate property. Diffs - autotests/data/fakeplugin.desktop 2e186869bf0cb39277ca3f9d2e8603d64d0223c6 autotests/kpluginmetadatatest.cpp 3af5e1b842b0bc231a5ac001112e141f751d2ff5 src/lib/plugin/desktopfileparser.h 98d47ddf0f877c4a25928026b3d5fe169cfc9e75 src/lib/plugin/desktopfileparser.cpp 0b03eb154deb58840c91c12658780c0d492b593c src/lib/plugin/kpluginmetadata.h 183b0d0583259f7ed74e97858a68c5c388fd0a9a src/lib/plugin/kpluginmetadata.cpp b13d6dd52827cc03d9473600aa4d2bab8a95a1d4 Diff: https://git.reviewboard.kde.org/r/125261/diff/ Testing --- Thanks, Alex Richardson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 125261: Add mimeTypes() to KPluginMetaData
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/125261/ --- (Updated Sept. 16, 2015, 5:50 p.m.) Review request for KDE Frameworks. Repository: kcoreaddons Description --- When loading a .desktop file this will parse the XDG MimeType= key Contrary to KService we don't merge these with ServiceTypes but rather have them as a separate property. Diffs - autotests/data/fakeplugin.desktop 2e186869bf0cb39277ca3f9d2e8603d64d0223c6 autotests/kpluginmetadatatest.cpp 3af5e1b842b0bc231a5ac001112e141f751d2ff5 src/lib/plugin/desktopfileparser.h 98d47ddf0f877c4a25928026b3d5fe169cfc9e75 src/lib/plugin/desktopfileparser.cpp 0b03eb154deb58840c91c12658780c0d492b593c src/lib/plugin/kpluginmetadata.h 183b0d0583259f7ed74e97858a68c5c388fd0a9a src/lib/plugin/kpluginmetadata.cpp b13d6dd52827cc03d9473600aa4d2bab8a95a1d4 Diff: https://git.reviewboard.kde.org/r/125261/diff/ Testing (updated) --- Used this for a WIP port of Okular to new Plugin loading. Could also be used by KDevelop instead of the custom X-KDevelop-SupportedMimeTypes property Thanks, Alex Richardson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 125263: Adapt KPluginInfo to introduction of KPluginMetaData::mimeTypes()
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/125263/ --- Review request for KDE Frameworks, David Faure and Sebastian Kügler. Repository: kservice Description --- Keep MIME types separate when converting KPluginInfo to KPluginMetaData Diffs - autotests/kplugininfotest.cpp c3a0f8f7a79fc9b063bbcf0f52a5442ae0545432 src/services/kplugininfo.cpp 93dcb8fc35930d89e4e60efea755b018d329794f Diff: https://git.reviewboard.kde.org/r/125263/diff/ Testing --- new + old unit tests pass Thanks, Alex Richardson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 125261: Add mimeTypes() to KPluginMetaData
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/125261/ --- (Updated Sept. 16, 2015, 5:53 p.m.) Review request for KDE Frameworks, David Faure and Sebastian Kügler. Repository: kcoreaddons Description --- When loading a .desktop file this will parse the XDG MimeType= key Contrary to KService we don't merge these with ServiceTypes but rather have them as a separate property. Diffs - autotests/data/fakeplugin.desktop 2e186869bf0cb39277ca3f9d2e8603d64d0223c6 autotests/kpluginmetadatatest.cpp 3af5e1b842b0bc231a5ac001112e141f751d2ff5 src/lib/plugin/desktopfileparser.h 98d47ddf0f877c4a25928026b3d5fe169cfc9e75 src/lib/plugin/desktopfileparser.cpp 0b03eb154deb58840c91c12658780c0d492b593c src/lib/plugin/kpluginmetadata.h 183b0d0583259f7ed74e97858a68c5c388fd0a9a src/lib/plugin/kpluginmetadata.cpp b13d6dd52827cc03d9473600aa4d2bab8a95a1d4 Diff: https://git.reviewboard.kde.org/r/125261/diff/ Testing (updated) --- Used this for a WIP port of Okular to new Plugin loading. Could also be used by KDevelop instead of the custom X-KDevelop-SupportedMimeTypes property Requires https://git.reviewboard.kde.org/r/125263/ to ensure that there are no regressions Thanks, Alex Richardson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 125261: Add mimeTypes() to KPluginMetaData
> On Sept. 16, 2015, 10:44 p.m., Sebastian Kügler wrote: > > autotests/kpluginmetadatatest.cpp, line 110 > > <https://git.reviewboard.kde.org/r/125261/diff/1/?file=404287#file404287line110> > > > > Should be a list, so enclosed with [ ]. > > > > In the header docs, you also say "string", so maybe I'm overlooking > > something? You're right, it should be a list. `KPluginMetaData::readStringList()` treats a single string as a list with one element so this does not cause an error and I didn't notice. Possibly it should output a warning in that case? - Alex --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/125261/#review85519 ------- On Sept. 16, 2015, 5:53 p.m., Alex Richardson wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/125261/ > --- > > (Updated Sept. 16, 2015, 5:53 p.m.) > > > Review request for KDE Frameworks, David Faure and Sebastian Kügler. > > > Repository: kcoreaddons > > > Description > --- > > When loading a .desktop file this will parse the XDG MimeType= key > > Contrary to KService we don't merge these with ServiceTypes but rather > have them as a separate property. > > > Diffs > - > > autotests/data/fakeplugin.desktop 2e186869bf0cb39277ca3f9d2e8603d64d0223c6 > autotests/kpluginmetadatatest.cpp 3af5e1b842b0bc231a5ac001112e141f751d2ff5 > src/lib/plugin/desktopfileparser.h 98d47ddf0f877c4a25928026b3d5fe169cfc9e75 > src/lib/plugin/desktopfileparser.cpp > 0b03eb154deb58840c91c12658780c0d492b593c > src/lib/plugin/kpluginmetadata.h 183b0d0583259f7ed74e97858a68c5c388fd0a9a > src/lib/plugin/kpluginmetadata.cpp b13d6dd52827cc03d9473600aa4d2bab8a95a1d4 > > Diff: https://git.reviewboard.kde.org/r/125261/diff/ > > > Testing > --- > > Used this for a WIP port of Okular to new Plugin loading. Could also be used > by KDevelop instead of the custom X-KDevelop-SupportedMimeTypes property > > Requires https://git.reviewboard.kde.org/r/125263/ to ensure that there are > no regressions > > > Thanks, > > Alex Richardson > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 125262: Parse service type files when loading from .desktop
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125262/
---
(Updated Sept. 17, 2015, 12:37 a.m.)
Review request for KDE Frameworks.
Repository: kcoreaddons
Description
---
KPluginMetadata("foo.desktop") or kcoreaddons_desktop_to_json() do not always
result in the correct JSON for example when a property is
supposed to be a list. the .desktop file parser in kcoreaddons so far had no
way of know this so instead of treating the entry as a
comma-separated list it would simply return a JSON string.
This is a follow up to https://git.reviewboard.kde.org/r/121672/ with a
different solution that also handles service types that are not compiled into
the kcoreaddons library but instead parses the installed servicetype .desktop
file.
It also contains a bit of code-deduplication and porting to categorized logging
which is not directly related to this patch but it would be some effort to
split that into a separate RR.
It is a series of commits with the following messages:
desktopparser: avoid unnecessary utf8 decoding
Parse ServiceType files when reading .desktop files
Remove lots of duplicated code for desktop{tojson,fileparser}.cpp
The only reason these were copy-pasted are minor differences in the
output which is now fixed by using qInstallMessageHandler and the
ability to generate JSON compatible with the first published version
of desktoptojson (which is hopefully no longer used and can be removed
soon)
QCommandLineParser uses -v for --version so just use --verbose
Otherwise the whole QCommandlineOption is ignored and there is no way
to enable verbose mode
desktopparser: Use more categorized logging
desktopparser: Allow passing relative paths to service type files
Add KPluginMetaData::fromDesktopFile()
This function allows specifying a list of service type files to be
parsed when loading the .desktop file.
desktopparser: Improve warning messages and add new unit test
The new test checks how the desktop parser handles service type files
with invalid property definitions
desktopparser: Fix parsing of double and bool values
QString::compare returns 0 on equal and make sure that we don't assign
the parsed double to an integer local variable
Add another unit test for desktop parsing with service types
Test that all supported types are converted correctly
Allow setting service types in kcoreaddons_desktop_to_json()
Diffs (updated)
-
KF5CoreAddonsMacros.cmake acfcaa3069991395d83923bcc30cd08f231c30eb
autotests/data/servicetypes/bad-groups-input.desktop PRE-CREATION
autotests/data/servicetypes/bad-groups-servicetype.desktop PRE-CREATION
autotests/data/servicetypes/example-input.desktop PRE-CREATION
autotests/data/servicetypes/example-servicetype.desktop PRE-CREATION
autotests/data/servicetypes/fake-kdevelopplugin.desktop PRE-CREATION
autotests/desktoptojsontest.cpp 64373d5be930426dd8a1f8e455e33c411a4795fd
autotests/kpluginmetadatatest.cpp 3af5e1b842b0bc231a5ac001112e141f751d2ff5
src/desktoptojson/CMakeLists.txt 94a199d8fa44a21b15e24c2e4f42551adada8f72
src/desktoptojson/desktoptojson.h bfa38b0f5ddd0581ad176d854614bc9c80dd139a
src/desktoptojson/desktoptojson.cpp 82626b256df6b3bd106e6d4c6fad84d7d970af37
src/desktoptojson/main.cpp 9bac8ff55d005d1944c04f557aa9601de2b0ca15
src/lib/plugin/desktopfileparser.h 98d47ddf0f877c4a25928026b3d5fe169cfc9e75
src/lib/plugin/desktopfileparser.cpp 0b03eb154deb58840c91c12658780c0d492b593c
src/lib/plugin/kpluginmetadata.h 183b0d0583259f7ed74e97858a68c5c388fd0a9a
src/lib/plugin/kpluginmetadata.cpp b13d6dd52827cc03d9473600aa4d2bab8a95a1d4
Diff: https://git.reviewboard.kde.org/r/125262/diff/
Testing
---
Added some unit test and they pass
Thanks,
Alex Richardson
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 125262: Parse service type files when loading from .desktop
> On Sept. 16, 2015, 6:14 p.m., David Faure wrote:
> > KF5CoreAddonsMacros.cmake, line 3
> > <https://git.reviewboard.kde.org/r/125262/diff/1/?file=404292#file404292line3>
> >
> > typo: SERIVCE -> SERVICE
> >
> > Does this break SC, if DEFAULT_SERVICE_TYPE must be specified? It
> > sounds like it should be the default value instead, so that existing cmake
> > code can keep working.
It uses DEFAULT_SERVICE_TYPE if neither is set, message(DEPRECATION) will only
cause an error if CMAKE_ERROR_DEPRECATED is set.
>From the docs:
DEPRECATION = CMake Deprecation Error or Warning if variable
CMAKE_ERROR_DEPRECATED or CMAKE_WARN_DEPRECATED is enabled, respectively, else
no message.
> On Sept. 16, 2015, 6:14 p.m., David Faure wrote:
> > src/lib/plugin/desktopfileparser.h, line 47
> > <https://git.reviewboard.kde.org/r/125262/diff/1/?file=404304#file404304line47>
> >
> > hm? it *is* marked as const...
Forgot to remove this comment when I changed the way I stored data.
- Alex
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/125262/#review85502
-------
On Sept. 17, 2015, 12:37 a.m., Alex Richardson wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/125262/
> ---
>
> (Updated Sept. 17, 2015, 12:37 a.m.)
>
>
> Review request for KDE Frameworks.
>
>
> Repository: kcoreaddons
>
>
> Description
> ---
>
> KPluginMetadata("foo.desktop") or kcoreaddons_desktop_to_json() do not always
> result in the correct JSON for example when a property is
> supposed to be a list. the .desktop file parser in kcoreaddons so far had no
> way of know this so instead of treating the entry as a
> comma-separated list it would simply return a JSON string.
>
> This is a follow up to https://git.reviewboard.kde.org/r/121672/ with a
> different solution that also handles service types that are not compiled into
> the kcoreaddons library but instead parses the installed servicetype .desktop
> file.
>
> It also contains a bit of code-deduplication and porting to categorized
> logging which is not directly related to this patch but it would be some
> effort to split that into a separate RR.
>
> It is a series of commits with the following messages:
>
>
> desktopparser: avoid unnecessary utf8 decoding
>
> Parse ServiceType files when reading .desktop files
>
>
> Remove lots of duplicated code for desktop{tojson,fileparser}.cpp
>
> The only reason these were copy-pasted are minor differences in the
> output which is now fixed by using qInstallMessageHandler and the
> ability to generate JSON compatible with the first published version
> of desktoptojson (which is hopefully no longer used and can be removed
> soon)
>
> QCommandLineParser uses -v for --version so just use --verbose
>
> Otherwise the whole QCommandlineOption is ignored and there is no way
> to enable verbose mode
>
> desktopparser: Use more categorized logging
>
>
> desktopparser: Allow passing relative paths to service type files
>
>
> Add KPluginMetaData::fromDesktopFile()
>
> This function allows specifying a list of service type files to be
> parsed when loading the .desktop file.
>
> desktopparser: Improve warning messages and add new unit test
>
> The new test checks how the desktop parser handles service type files
> with invalid property definitions
>
> desktopparser: Fix parsing of double and bool values
>
> QString::compare returns 0 on equal and make sure that we don't assign
> the parsed double to an integer local variable
>
> Add another unit test for desktop parsing with service types
>
> Test that all supported types are converted correctly
>
> Allow setting service types in kcoreaddons_desktop_to_json()
>
>
> Diffs
> -
>
> KF5CoreAddonsMacros.cmake acfcaa3069991395d83923bcc30cd08f231c30eb
> autotests/data/servicetypes/bad-groups-input.desktop PRE-CREATION
> autotests/data/servicetypes/bad-groups-servicetype.desktop PRE-CREATION
> autotests/data/servicetypes/example-input.desktop PRE-CREATION
> autotests/data/servicetypes/example-servicetype.desktop PRE-CREATION
> autotests/data/servicetypes/fake-kdevelopplugin.desktop PRE-CREATION
> autotests/desktoptojsontest.cpp 64373d5be930426dd8a1f8e455e33c411a4795fd
> autotests/kpluginmetadatatest.cpp 3af5e1b842b0
Re: Review Request 125048: Use JSON files directly instead of kcoreaddons_desktop_to_json()
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/125048/ --- (Updated Sept. 8, 2015, 7:28 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks and David Faure. Changes --- Submitted with commit 6db255388532a4fd0788aa971c83798e1849d479 by Alex Richardson to branch master. Repository: kio Description --- Use JSON files directly instead of kcoreaddons_desktop_to_json() Diffs - src/ioslaves/http/kcookiejar/CMakeLists.txt 7b4778d1f67c1ad9f9edcaa4692b39ee6fe3f365 src/ioslaves/http/kcookiejar/kcookiejar.desktop 3ea56abdc134cae22b69d7b7636ce6dd415a3d9b src/ioslaves/http/kcookiejar/kcookiejar.json PRE-CREATION src/kpac/CMakeLists.txt fc5989714480ca49b5bd72e1c7b458b26bd0d9bc src/kpac/proxyscout.desktop a545f3d6ef2fd18b1a2c85ebff15c9f2513d87f1 src/kpac/proxyscout.json PRE-CREATION src/kpasswdserver/CMakeLists.txt 11c30201012fbe413ff58561b54255e88c2c55b9 src/kpasswdserver/kpasswdserver.desktop bc788e5665e3d9d43309da241c3a3f5ac3cd0fc9 src/kpasswdserver/kpasswdserver.json PRE-CREATION src/kssld/CMakeLists.txt bf6970c2741a6edd01e36b86744d643e70046889 src/kssld/kssld.desktop 86581b208ffc89fa5235f9284395d9d7ccebc472 src/kssld/kssld.json PRE-CREATION Diff: https://git.reviewboard.kde.org/r/125048/diff/ Testing --- Thanks, Alex Richardson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 125048: Use JSON files directly instead of kcoreaddons_desktop_to_json()
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/125048/ --- Review request for KDE Frameworks and David Faure. Repository: kio Description --- Use JSON files directly instead of kcoreaddons_desktop_to_json() Diffs - src/ioslaves/http/kcookiejar/CMakeLists.txt 7b4778d1f67c1ad9f9edcaa4692b39ee6fe3f365 src/ioslaves/http/kcookiejar/kcookiejar.desktop 3ea56abdc134cae22b69d7b7636ce6dd415a3d9b src/ioslaves/http/kcookiejar/kcookiejar.json PRE-CREATION src/kpac/CMakeLists.txt fc5989714480ca49b5bd72e1c7b458b26bd0d9bc src/kpac/proxyscout.desktop a545f3d6ef2fd18b1a2c85ebff15c9f2513d87f1 src/kpac/proxyscout.json PRE-CREATION src/kpasswdserver/CMakeLists.txt 11c30201012fbe413ff58561b54255e88c2c55b9 src/kpasswdserver/kpasswdserver.desktop bc788e5665e3d9d43309da241c3a3f5ac3cd0fc9 src/kpasswdserver/kpasswdserver.json PRE-CREATION src/kssld/CMakeLists.txt bf6970c2741a6edd01e36b86744d643e70046889 src/kssld/kssld.desktop 86581b208ffc89fa5235f9284395d9d7ccebc472 src/kssld/kssld.json PRE-CREATION Diff: https://git.reviewboard.kde.org/r/125048/diff/ Testing --- Thanks, Alex Richardson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 125048: Use JSON files directly instead of kcoreaddons_desktop_to_json()
> On Sept. 4, 2015, 3:56 p.m., David Faure wrote: > > Wasn't the reason for desktop files that we have infrastructure for > > translating desktop files but not json files? > > > > (I could be wrong, of course). I thought this has now been fixed and JSON files are also translated. - Alex --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/125048/#review84831 --- On Sept. 4, 2015, 3:18 p.m., Alex Richardson wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/125048/ > --- > > (Updated Sept. 4, 2015, 3:18 p.m.) > > > Review request for KDE Frameworks and David Faure. > > > Repository: kio > > > Description > --- > > Use JSON files directly instead of kcoreaddons_desktop_to_json() > > > Diffs > - > > src/ioslaves/http/kcookiejar/CMakeLists.txt > 7b4778d1f67c1ad9f9edcaa4692b39ee6fe3f365 > src/ioslaves/http/kcookiejar/kcookiejar.desktop > 3ea56abdc134cae22b69d7b7636ce6dd415a3d9b > src/ioslaves/http/kcookiejar/kcookiejar.json PRE-CREATION > src/kpac/CMakeLists.txt fc5989714480ca49b5bd72e1c7b458b26bd0d9bc > src/kpac/proxyscout.desktop a545f3d6ef2fd18b1a2c85ebff15c9f2513d87f1 > src/kpac/proxyscout.json PRE-CREATION > src/kpasswdserver/CMakeLists.txt 11c30201012fbe413ff58561b54255e88c2c55b9 > src/kpasswdserver/kpasswdserver.desktop > bc788e5665e3d9d43309da241c3a3f5ac3cd0fc9 > src/kpasswdserver/kpasswdserver.json PRE-CREATION > src/kssld/CMakeLists.txt bf6970c2741a6edd01e36b86744d643e70046889 > src/kssld/kssld.desktop 86581b208ffc89fa5235f9284395d9d7ccebc472 > src/kssld/kssld.json PRE-CREATION > > Diff: https://git.reviewboard.kde.org/r/125048/diff/ > > > Testing > --- > > > Thanks, > > Alex Richardson > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 125048: Use JSON files directly instead of kcoreaddons_desktop_to_json()
> On Sept. 4, 2015, 3:56 p.m., David Faure wrote: > > Wasn't the reason for desktop files that we have infrastructure for > > translating desktop files but not json files? > > > > (I could be wrong, of course). > > Alex Richardson wrote: > I thought this has now been fixed and JSON files are also translated. Checking the git log on various JSON files in kdevplatform show commits by scripty adding translations so I am pretty sure this has been fixed. - Alex --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/125048/#review84831 --- On Sept. 4, 2015, 3:18 p.m., Alex Richardson wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/125048/ > --- > > (Updated Sept. 4, 2015, 3:18 p.m.) > > > Review request for KDE Frameworks and David Faure. > > > Repository: kio > > > Description > --- > > Use JSON files directly instead of kcoreaddons_desktop_to_json() > > > Diffs > - > > src/ioslaves/http/kcookiejar/CMakeLists.txt > 7b4778d1f67c1ad9f9edcaa4692b39ee6fe3f365 > src/ioslaves/http/kcookiejar/kcookiejar.desktop > 3ea56abdc134cae22b69d7b7636ce6dd415a3d9b > src/ioslaves/http/kcookiejar/kcookiejar.json PRE-CREATION > src/kpac/CMakeLists.txt fc5989714480ca49b5bd72e1c7b458b26bd0d9bc > src/kpac/proxyscout.desktop a545f3d6ef2fd18b1a2c85ebff15c9f2513d87f1 > src/kpac/proxyscout.json PRE-CREATION > src/kpasswdserver/CMakeLists.txt 11c30201012fbe413ff58561b54255e88c2c55b9 > src/kpasswdserver/kpasswdserver.desktop > bc788e5665e3d9d43309da241c3a3f5ac3cd0fc9 > src/kpasswdserver/kpasswdserver.json PRE-CREATION > src/kssld/CMakeLists.txt bf6970c2741a6edd01e36b86744d643e70046889 > src/kssld/kssld.desktop 86581b208ffc89fa5235f9284395d9d7ccebc472 > src/kssld/kssld.json PRE-CREATION > > Diff: https://git.reviewboard.kde.org/r/125048/diff/ > > > Testing > --- > > > Thanks, > > Alex Richardson > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 124904: Make KDE_FORK_SLAVES work under Windows
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124904/#review84309 --- Looks good to me src/core/slave.cpp (line 88) https://git.reviewboard.kde.org/r/124904/#comment58378 Unrelated but while you're touching this code maybe change this to qEnvironmentVariableIsSet(). More efficient and easier to understand - Alex Richardson On Aug. 24, 2015, 2:11 p.m., Kevin Funk wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124904/ --- (Updated Aug. 24, 2015, 2:11 p.m.) Review request for KDE Frameworks, David Faure and Boudewijn Rempt. Repository: kio Description --- Make KDE_FORK_SLAVES work under Windows Diffs - src/core/slave.cpp 62455c1db471aa3fc1273b1bb398fbc183ddca1f Diff: https://git.reviewboard.kde.org/r/124904/diff/ Testing --- Thanks, Kevin Funk ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 124281: Remove KService and KIconThemes usage from KNotifications
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124281/#review82234 --- src/knotificationmanager.cpp (line 92) https://git.reviewboard.kde.org/r/124281/#comment56610 There is no need to iterate over the library paths: If a relative path is given for directory, all entries of QCoreApplication::libraryPaths() will be checked with directory appended as a subdirectory. If an absolute path is given only that directory will be searched. `QListQObject* plugins = KPluginLoader::instantiatePlugins(knotification/notifyplugins, nullptr, this);` - Alex Richardson On July 7, 2015, 8 p.m., Martin Klapetek wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124281/ --- (Updated July 7, 2015, 8 p.m.) Review request for KDE Frameworks and Martin Gräßlin. Repository: knotifications Description --- This patch reduces the dependencies of KNotifications framework and effectively makes it a tier 2 framework. KService is used only for locating additional notification plugins and to my knowledge, there are none such plugins currently existing, at least not in all around KDE plus the class for the plugins wasn't exported until about two months ago, so this should be safe without a legacy support. KIconThemes is used only to get KIconLoader::Small icon pixmap for notifications using KPassivePopup. After some playing around it turns out that it puts the icon into the KPassivePopup title and makes it as big as the text. So I've made the icon size to be the same as the text height. So this keeps things visually the same + still DPI aware, though I believe the KPassivePopup should receive a complete visual overhaul anyway. Additionally, KCodecs dependency has again one single usage for decoding html entities to QChars within QXmlStreamReader parser, so eventually could also be removed/replaced with QTextDocument::toPlainText() which seems to do the same job as QXmlStreamReader+KCodecs. Diffs - CMakeLists.txt 2d5437b metainfo.yaml 7fc15f7 src/CMakeLists.txt 1cebece src/knotificationmanager.cpp 8d4f953 src/knotificationplugin.cpp 7315c17 src/notifybypopup.cpp e377051 tests/kpassivepopuptest.h bc0dedc tests/kpassivepopuptest.cpp 2486fd8 Diff: https://git.reviewboard.kde.org/r/124281/diff/ Testing --- Everything still compiles + I added a test for KPassivePopup with an icon. Thanks, Martin Klapetek ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 124281: Remove KService and KIconThemes usage from KNotifications
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124281/#review82179 --- Looks good to me otherwise src/knotificationmanager.cpp (line 87) https://git.reviewboard.kde.org/r/124281/#comment56570 Use `KPluginLoader::findPlugins` or `KPluginLoader::instantiatePlugins` and then do the qobject_cast to avoid that duplicate code src/notifybypopup.cpp (line 274) https://git.reviewboard.kde.org/r/124281/#comment56571 nullptr? or Q_NULLPTR if that's not allowed. Would make it explicit that it's a pointer. - Alex Richardson On July 7, 2015, 3:42 p.m., Martin Klapetek wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124281/ --- (Updated July 7, 2015, 3:42 p.m.) Review request for KDE Frameworks and Martin Gräßlin. Repository: knotifications Description --- This patch reduces the dependencies of KNotifications framework and effectively makes it a tier 2 framework. KService is used only for locating additional notification plugins and to my knowledge, there are none such plugins currently existing, at least not in all around KDE plus the class for the plugins wasn't exported until about two months ago, so this should be safe without a legacy support. KIconThemes is used only to get KIconLoader::Small icon pixmap for notifications using KPassivePopup. After some playing around it turns out that it puts the icon into the KPassivePopup title and makes it as big as the text. So I've made the icon size to be the same as the text height. So this keeps things visually the same + still DPI aware, though I believe the KPassivePopup should receive a complete visual overhaul anyway. Additionally, KCodecs dependency has again one single usage for decoding html entities to QChars within QXmlStreamReader parser, so eventually could also be removed/replaced with QTextDocument::toPlainText() which seems to do the same job as QXmlStreamReader+KCodecs. Diffs - CMakeLists.txt 2d5437b metainfo.yaml 7fc15f7 src/CMakeLists.txt 1cebece src/knotificationmanager.cpp 8d4f953 src/notifybypopup.cpp e377051 tests/kpassivepopuptest.h bc0dedc tests/kpassivepopuptest.cpp 2486fd8 Diff: https://git.reviewboard.kde.org/r/124281/diff/ Testing --- Everything still compiles + I added a test for KPassivePopup with an icon. Thanks, Martin Klapetek ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 124118: Make it possible to use KCoreAddons's desktoptojson in cross-compiled environments
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124118/#review81544 --- autotests/CMakeLists.txt (line 49) https://git.reviewboard.kde.org/r/124118/#comment55894 I have no experience with cross compiling, but shouldn't this be `if(NOT ...)` as now the test will only be compiled when crosscompiling? - Alex Richardson On June 18, 2015, 2:39 a.m., Aleix Pol Gonzalez wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124118/ --- (Updated June 18, 2015, 2:39 a.m.) Review request for KDE Frameworks. Repository: kcoreaddons Description --- Follows the lead of kconfig patch: https://git.reviewboard.kde.org/r/124104/ Diffs - CMakeLists.txt 552851d KF5CoreAddonsConfig.cmake.in caa1c0a autotests/CMakeLists.txt 51fcfb4 src/desktoptojson/CMakeLists.txt e6041cb Diff: https://git.reviewboard.kde.org/r/124118/diff/ Testing --- Thanks, Aleix Pol Gonzalez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 124066: Recognize X-KDE-FormFactor as stringlist
On June 17, 2015, 8:24 a.m., David Faure wrote: I don't really have the overview anymore, but the kdelibs4 solution was fully extensible, a servicetype desktop file could define new keys and their type. It looks like desktopfileparser.cpp doesn't have the same flexibility, if each and every app needs to add their keys to the code :( Possibly we should let desktopfileparser read a servicetype file? Was also suggested here: https://git.reviewboard.kde.org/r/121672/ - Alex --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124066/#review81520 --- On June 11, 2015, 5:16 a.m., Sebastian Kügler wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124066/ --- (Updated June 11, 2015, 5:16 a.m.) Review request for KDE Frameworks, Alex Richardson and David Faure. Repository: kcoreaddons Description --- This patch adds X-KDE-FormFactor to the keys recognized as stringlists. We would like to see this to go in to allow us to filter plugins (KCMs for example) by form factor, so we only display UI plugins that are suitable for a given target device. The idea is that plugins indicate which form factor (for example media center, tablet, desktop, etc...) they're suitable for, and the host application filters based on these. If this approach is deemed valid, I'd be happy to add convenience API to KPluginMetaData, i.e. QStringList KPluginMetaData::formFactor(). This patch would be the minimal implementation we'd need. The naming of the key is of course open to better suggestions. Diffs - autotests/data/fakeplugin.desktop 95152f6 autotests/kpluginmetadatatest.cpp 231ac36 src/lib/plugin/desktopfileparser.cpp b19da6b Diff: https://git.reviewboard.kde.org/r/124066/diff/ Testing --- added autotest, also implemented using this in the Plasma Active settings app as proof-of-concept, works like a charm. Thanks, Sebastian Kügler ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 124032: Do not try to complete users and assert when prepend is non-empty.
On June 6, 2015, 10:07 p.m., David Faure wrote: The test doesn't check that user completion still works though? (in the case where it's expected, ~foo). I'm curious btw, why did it crash/assert? Finally I wonder why this didn't happen in kdelibs4, but ok, the code has had some changes due to the QUrl-KUrl porting, you don't *have* to answer this one ;) Milian Wolff wrote: it still works and I also wonder why this has not happened before - I blame the KUrl/QUrl differences here. Who was the expert in that area again to ask? :P Is it OK if I just add a test that expects a non-empty result set when ~ is inserted? Or what should we look out for ~root maybe? Or can I get the shell user name (or actually - home path) via some Qt API? David Faure wrote: You could compare that completion with KUser::allUsersNames() :-) Or just using the current user, KUser(KUser::currentUserId()), to check it's in the list, to avoid differences with allUsersNames() due to special users or stuff like that. Milian Wolff wrote: should I run this test only on unix platforms or is this also valid on windows e.g.? what about mac os x? David Faure wrote: KUser is portable. The completion in KIO is portable too (in fact it uses KUser::allUserNames() :-) So you can run the test on all platforms. It should work fine on Windows as well (at least it did when I tested that code on Windows 7, I assume Microsoft stays backwards compatible). However, I no longer have a Windows machine to test it. - Alex --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124032/#review81277 --- On June 6, 2015, 9:58 p.m., Milian Wolff wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124032/ --- (Updated June 6, 2015, 9:58 p.m.) Review request for KDE Frameworks and David Faure. Repository: kio Description --- BUG: 346920 REVIEW: 124032 Diffs - autotests/kurlcompletiontest.cpp 55bd38011c93ea2d35a53a911a4878f28a09c00c src/widgets/kurlcompletion.cpp d7ae787e6c3d1dba1a168e77cc2d5001b4bed7ef Diff: https://git.reviewboard.kde.org/r/124032/diff/ Testing --- added a unit test, it now passes. also kate and the manual kurlrequestertest_gui test don't crash anymore when I insert ~/. into a url requester. Thanks, Milian Wolff ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123940: Use KPluginLoader::factory() when loading KIO::DndPopupMenuPlugin
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123940/#review80959 --- Ship it! Ship It! - Alex Richardson On May 29, 2015, 11:09 p.m., Ragnar Thomsen wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123940/ --- (Updated May 29, 2015, 11:09 p.m.) Review request for Dolphin, KDE Frameworks, Emmanuel Pescosta, and Harald Hvaal. Repository: kio Description --- This patch fixes an error when loading the DndPopupMenuPlugin of Ark (frameworks branch). DndPopupMenuPlugins use the K_PLUGIN_FACTORY_WITH_JSON macro, and therefore KPluginLoader::factory() should be used instead of KPluginLoader::instance() which is used by KPluginMetaData::instantiate(). The DndPopupMenuPlugin is used by Ark to enable the Extract here menu option when dragging an archive in Dolphin. Diffs - src/widgets/dropjob.cpp 63beb0a Diff: https://git.reviewboard.kde.org/r/123940/diff/ Testing --- The Extract here option now appears when dragging an archive in Dolphin (current frameworks branch), and extraction works as expected. Thanks, Ragnar Thomsen ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123857: Fix crash after a user has launched kbuildsycoca as root.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123857/#review80677 --- src/kbuildsycoca/kbuildsycoca.cpp (line 426) https://git.reviewboard.kde.org/r/123857/#comment55309 `#if Q_OS_UNIX`? Don't think Windows has getuid(). - Alex Richardson On May 20, 2015, 7:33 p.m., David Edmundson wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123857/ --- (Updated May 20, 2015, 7:33 p.m.) Review request for KDE Frameworks. Repository: kservice Description --- 3 commits fixing; a crash, what caused the crash, and why we weren't auto recovering from it. QFile::open() will return a Read/Write error on failed access, PermissionsError is only in the result of a setPermissions call. CCBUG: 342438 If running as root, keep file ownership the same as the original file we're replacing. $HOME is often preserved as root, making us write cache files in the user's home directory. CCBUG: 342438 Guard against being unable to open stream This can happen if we are unable to open the database when a notifyDatabaseChanged signal is emitted. Other places guard also against this eventuality. BUG: 342438 Diffs - src/kbuildsycoca/kbuildsycoca.cpp d14f1f950cdb0d8c9fefbce2a4740f211d3d97a1 src/services/kservicegroupfactory.cpp 8cfc6c6670d3b87e8ed6bdfe4aeac947846afc18 Diff: https://git.reviewboard.kde.org/r/123857/diff/ Testing --- Thanks, David Edmundson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123595: Fix KUser test for Mac.
On May 6, 2015, 5:40 p.m., René J.V. Bertin wrote: I'm not sure, after reading http://pig.made-it.com/uidgid.html also: ``` id nobody uid=4294967294(nobody) gid=4294967294(nobody) groups=4294967294(nobody),12(everyone),61(localaccounts),402(com.apple.sharepoint.group.1),403(com.apple.sharepoint.group.2),100(_lpoperator) ``` and ``` id unknown uid=99(_unknown) gid=99(_unknown) groups=99(_unknown),402(com.apple.sharepoint.group.1),12(everyone),61(localaccounts),403(com.apple.sharepoint.group.2),100(_lpoperator) ``` David Faure wrote: nobody is a special user, let's put that one aside since the unittest doesn't need it. I'm not sure `id` works like the APIs KUser uses. What does the unittest say for you, in fact? (with or without my patch, doesn't matter for debugging this) René J.V. Bertin wrote: I certainly hope that KUser agrees with `id` ... Sorry I can't answer your question, I haven't been touching KF5 at all until now. KUser just uses the stanard `getpwuid(3)`/`getpwnam(3)` function which AFAIK will also be used by `id`. - Alex --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123595/#review79976 --- On May 6, 2015, 5:08 p.m., David Faure wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123595/ --- (Updated May 6, 2015, 5:08 p.m.) Review request for KDE Software on Mac OS X, KDE Frameworks and Marko Käning. Repository: kcoreaddons Description --- According to CI [1], an invalid user belongs to nogroup on Mac. Not sure if this is true on all OSX installations though? https://build.kde.org/view/Frameworks%20kf5-qt5/job/kcoreaddons%20master%20kf5-qt5/PLATFORM=OSX,compiler=clang/19/testReport/%28root%29/TestSuite/kusertest/ Diffs - autotests/kusertest.cpp d17a2d3e97d5056524281eb18766377e48a0da35 Diff: https://git.reviewboard.kde.org/r/123595/diff/ Testing --- Still passes on Linux; should fix the CI for mac, AFAICS. Thanks, David Faure ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123669: Add KPluginLoader::findPluginsById convenience API
On May 7, 2015, 8:24 a.m., Alex Richardson wrote: Seems like this is duplicated in a few places already so I agree we should add it. But won't most users of the API want only a single plugin returned? Maybe also add a function `KPluginMetaData KPluginLoader::findPluginById(const QString directory, const QString pluginId)`? Do we also need the function that returns a vector for a given ID? Sebastian Kügler wrote: At least our changes in libplasma need a QVectorKPluginMetaData. Otherwise, a list seems easy enough to check if something's found. Returning a single metadata won't be very useful for us at this point (but I see it making sense). Sebastian Kügler wrote: Ow, also, and perhaps more importantly, multiple ids are technically perfectly valid (only the plugin name and directory are important here). I think that fact should be reflected in the API. Perhaps a word on ordering would be in place in the API docs, plugin locations are cascaded properly in code using it. The most local plugin is at the end of the list, system-widely installed ones at the beginning, so code that uses plugins.first() would not allow the user to override plugins installed for example into /usr/lib with a plugin with the same id and relative path installed into ~/.local). So an extra method returning the .last() plugin found might take away this caveat from the API. We'll still need the method returning a vector for libplasma, though (and I think it's a semantically useful addition). About adding another method to return the most-local plugin, I'm on the edge. If others think it's useful and we think the additional API (with its long-term maintainance implications) is worth it, I'm happy to add it as well. (Perhaps in a separate review.) Opinions welcome. The problem is that .last() won't work either. QCoreApplication::libraryPaths() has this order (http://code.woboq.org/qt5/qtbase/src/corelib/kernel/qcoreapplication.cpp.html#_ZN16QCoreApplication12libraryPathsEv): $QT_INSTALLDIR/plugins, then the current executable directory and then QT_PLUGIN_PATH. This means that the lowest priority entry from QT_PLUGIN_PATH will be chosen in that case. - Alex --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123669/#review80013 --- On May 7, 2015, 12:21 a.m., Sebastian Kügler wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123669/ --- (Updated May 7, 2015, 12:21 a.m.) Review request for KDE Frameworks, Alex Richardson and David Faure. Repository: kcoreaddons Description --- Add findPluginsById convenience API It's a quite common case to load a plugin from an ID. This makes it easy. CHANGELOG:New KPluginLoader::findPluginById() convenience API REVIEW:123669 Diffs - autotests/kpluginloadertest.cpp 3ded0ebca2e0fd20e09bf6e4eca152d13ac11f46 src/lib/plugin/kpluginloader.h 124bfab7e207b17d3c8ab4d5a88321af476aad42 src/lib/plugin/kpluginloader.cpp 4310d6cd7792329511f12b28d7c68b0fdd013538 Diff: https://git.reviewboard.kde.org/r/123669/diff/ Testing --- Added autotests, everything passes. Thanks, Sebastian Kügler ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123669: Add KPluginLoader::findPluginsById convenience API
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123669/#review80013 --- Seems like this is duplicated in a few places already so I agree we should add it. But won't most users of the API want only a single plugin returned? Maybe also add a function `KPluginMetaData KPluginLoader::findPluginById(const QString directory, const QString pluginId)`? Do we also need the function that returns a vector for a given ID? src/lib/plugin/kpluginloader.cpp (line 278) https://git.reviewboard.kde.org/r/123669/#comment54900 QVector KPluginMetaData - QVectorKPluginMetaData - Alex Richardson On May 7, 2015, 12:21 a.m., Sebastian Kügler wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123669/ --- (Updated May 7, 2015, 12:21 a.m.) Review request for KDE Frameworks, Alex Richardson and David Faure. Repository: kcoreaddons Description --- Add findPluginsById convenience API It's a quite common case to load a plugin from an ID. This makes it easy. CHANGELOG:New KPluginLoader::findPluginById() convenience API REVIEW:123669 Diffs - autotests/kpluginloadertest.cpp 3ded0ebca2e0fd20e09bf6e4eca152d13ac11f46 src/lib/plugin/kpluginloader.h 124bfab7e207b17d3c8ab4d5a88321af476aad42 src/lib/plugin/kpluginloader.cpp 4310d6cd7792329511f12b28d7c68b0fdd013538 Diff: https://git.reviewboard.kde.org/r/123669/diff/ Testing --- Added autotests, everything passes. Thanks, Sebastian Kügler ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123556: KPackage::findPackages with a filter callback
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123556/#review79698 --- src/kpackage/packageloader.cpp (line 273) https://git.reviewboard.kde.org/r/123556/#comment54542 This will crash if there is no filter. `if (!filter || filter(plugin))` should work. - Alex Richardson On April 29, 2015, 1:12 p.m., Marco Martin wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123556/ --- (Updated April 29, 2015, 1:12 p.m.) Review request for KDE Frameworks and Plasma. Repository: kpackage Description --- Add a KPackage::findPackages function similar to KPluginLoader::findPlugins Diffs - src/kpackage/packageloader.h 2761d98 src/kpackage/packageloader.cpp 00defbd Diff: https://git.reviewboard.kde.org/r/123556/diff/ Testing --- Thanks, Marco Martin ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123542: [runtime] Move platform specific code into plugins
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/123542/#review79637
---
src/runtime/globalshortcutsregistry.cpp (line 40)
https://git.reviewboard.kde.org/r/123542/#comment54448
You could use
`KPluginLoader::forEachPlugin(org.kde.kglobalaccel5.platforms, [](const
QString lib) { ret lib; });` here.
src/runtime/globalshortcutsregistry.cpp (line 58)
https://git.reviewboard.kde.org/r/123542/#comment54449
Actually, since all plugins have JSON metadata this can actually be further
simplified by using something like this:
```
auto candidates =
KPluginLoader::findPlugins(org.kde.kglobalaccel5.platforms);
foreach (const auto candidate, candidates) {
const auto platforms = candidate.rawData().value(platforms).toArray();
foreach (const auto platform, platforms) {
if (QString::compare(QGuiApplication::platformName(),
platform.toString(), Qt::CaseInsensitive) == 0) {
KGlobalAccelInterface *interface = qobject_cast
KGlobalAccelInterface* (candidate.instatiate());
if (interface) {
qCDebug(KGLOBALACCELD) Loaded plugin candidate
for platform QGuiApplication::platformName();
interface-setRegistry(parent);
return interface;
}
}
}
}
```
- Alex Richardson
On April 28, 2015, 3:13 p.m., Martin Gräßlin wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/123542/
---
(Updated April 28, 2015, 3:13 p.m.)
Review request for KDE Frameworks, kdewin and Martin Klapetek.
Repository: kglobalaccel
Description
---
The current architecture of the runtime component was to have compile
time destinctions between various platforms. This approach does not
allow to include Wayland support on Linux as both xcb and Wayland are
possible platforms which are selected at runtime and not at compile
time. Thus a change of the architecture is required.
The solution taken in this patch is to move the platform specific code
into plugins and to load the plugin specific to the currently used
platform. Therefore a new interface class is introduced which the
plugins have to implement. In addition most of the runtime is turned
into a library, so that the plugin can link it.
To properly support Wayland more changes will be required. The security
provided by the Wayland windowing system does not work well with the
architecture of kglobalacceld. Kglobalacceld is a kind of glorified
key logger. Wayland makes it impossible to be a key logger. Thus the
support for global shortcuts needs to be in the compositor. In the case
of KDE Plasma that is KWin. On that architecture we can make kglobalaccel
to still work by allowing KWin to link the new library and provide it's
own plugin. In order to support this the new private library must be
cleaned up and prepared for at least allowing a somewhat stable API/ABI.
Please note that this change also disables build for all platforms
except xcb (OSX and Windows) due to lack of a build environment.
Diffs
-
src/runtime/CMakeLists.txt 8c7c7610843040fa60ab095114e7b74707f75c30
src/runtime/component.cpp 663d0ade5ffe03254cc7886b76c7850d4b4317ab
src/runtime/globalshortcutsregistry.h
ca12db09f4b56b0a0f0b6304da1cd1292ef65042
src/runtime/globalshortcutsregistry.cpp
446e766deb96ae3a83baabaca726d5460b597b88
src/runtime/kglobalaccel_interface.h PRE-CREATION
src/runtime/kglobalaccel_interface.cpp PRE-CREATION
src/runtime/kglobalaccel_mac.h
src/runtime/kglobalaccel_mac.cpp
src/runtime/kglobalaccel_win.h
src/runtime/kglobalaccel_win.cpp
src/runtime/kglobalaccel_x11.h b398e1cfcf9496b58f6b05893a002c112c7cf986
src/runtime/kglobalaccel_x11.cpp 2600220c255641304d4f67aad74582b01b8f799c
src/runtime/kglobalacceld.h b2fc27223ea1d11ca5a75f2ad58a8d745fb17191
src/runtime/plugins/CMakeLists.txt PRE-CREATION
src/runtime/plugins/xcb/CMakeLists.txt PRE-CREATION
src/runtime/plugins/xcb/xcb.json PRE-CREATION
Diff: https://git.reviewboard.kde.org/r/123542/diff/
Testing
---
kglobalaccel5 still working on platform xcb (running here right now)
Thanks,
Martin Gräßlin
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123479: Adapt to Qt 5.6 changes and prevent nullbytes in QStrings.
On April 24, 2015, 12:33 a.m., Alex Richardson wrote:
src/lib/io/kdirwatch.cpp, line 303
https://git.reviewboard.kde.org/r/123479/diff/2/?file=362725#file362725line303
Why this manual loop instead of strlen()? Does that mean that null
characters in the middle are valid? Or, more likely, this reverse loop is
an optimization? If so I would add a comment since it wasn't obvious to me
straight away.
Maybe this here is easier to read?
if (cpath.endsWith('\0')) {
cpath.resize(cpath.lastIndexOf('\0'));
}
- Alex
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/123479/#review79415
---
On April 23, 2015, 6:19 p.m., Milian Wolff wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/123479/
---
(Updated April 23, 2015, 6:19 p.m.)
Review request for KDE Frameworks, David Faure, Marc Mutz, and Volker Krause.
Repository: kcoreaddons
Description
---
The len in inotify_event includes nulls of the name. To prevent
them from being included in the QString/QByteArray we must filter
them manually with a recent Qt 5 dev build now. See also:
https://codereview.qt-project.org/#/c/106473/
REVIEW: 123479
Diffs
-
autotests/kdirwatch_unittest.cpp 83b5b410e987fb45a08f8251ec65496c8074b077
src/lib/io/kdirwatch.cpp a75fae012c59021996b46845db2e97abb5526930
Diff: https://git.reviewboard.kde.org/r/123479/diff/
Testing
---
I used the test and looked at the output and also ran it against a patched
qtbase with this:
https://paste.kde.org/pmoue241d
Thanks,
Milian Wolff
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123479: Adapt to Qt 5.6 changes and prevent nullbytes in QStrings.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123479/#review79415 --- src/lib/io/kdirwatch.cpp (line 303) https://git.reviewboard.kde.org/r/123479/#comment54265 Why this manual loop instead of strlen()? Does that mean that null characters in the middle are valid? Or, more likely, this reverse loop is an optimization? If so I would add a comment since it wasn't obvious to me straight away. - Alex Richardson On April 23, 2015, 6:19 p.m., Milian Wolff wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123479/ --- (Updated April 23, 2015, 6:19 p.m.) Review request for KDE Frameworks, David Faure, Marc Mutz, and Volker Krause. Repository: kcoreaddons Description --- The len in inotify_event includes nulls of the name. To prevent them from being included in the QString/QByteArray we must filter them manually with a recent Qt 5 dev build now. See also: https://codereview.qt-project.org/#/c/106473/ REVIEW: 123479 Diffs - autotests/kdirwatch_unittest.cpp 83b5b410e987fb45a08f8251ec65496c8074b077 src/lib/io/kdirwatch.cpp a75fae012c59021996b46845db2e97abb5526930 Diff: https://git.reviewboard.kde.org/r/123479/diff/ Testing --- I used the test and looked at the output and also ran it against a patched qtbase with this: https://paste.kde.org/pmoue241d Thanks, Milian Wolff ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123397: Generalize the creation of KPluginLoader-based plugins
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123397/#review79260 --- Ship it! Ship It! - Alex Richardson On April 17, 2015, 3:53 p.m., Aleix Pol Gonzalez wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123397/ --- (Updated April 17, 2015, 3:53 p.m.) Review request for KDE Frameworks and Alex Richardson. Repository: kcoreaddons Description --- I was about to fork this macro (originally developed for KDevPlatform) for the 2nd time, it was time to put it somewhere where it can be shared. Basically puts together the different parts of the process so that it's comfortable to develop later on. Especially useful the fact that it makes the JSON file a dependency of the cpp file, so changes are adopted automatically. Diffs - KF5CoreAddonsMacros.cmake 7ea72af Diff: https://git.reviewboard.kde.org/r/123397/diff/ Testing --- It's being used already in KDevelop and KDE Connect. Thanks, Aleix Pol Gonzalez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123397: Generalize the creation of KPluginLoader-based plugins
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123397/#review79131 --- KF5CoreAddonsMacros.cmake (line 108) https://git.reviewboard.kde.org/r/123397/#comment54078 I would also grep for Q_PLUGIN_METADATA(IID ... FILE foo.json). Also maybe make the macro name customizable. For example okular uses OKULAR_EXPORT_PLUGIN for the plugins (which could probably be replaced by K_PLUGIN_FACTORY_WITH_JSON, but I haven't looked into that yet). - Alex Richardson On April 17, 2015, 3:53 p.m., Aleix Pol Gonzalez wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123397/ --- (Updated April 17, 2015, 3:53 p.m.) Review request for KDE Frameworks and Alex Richardson. Repository: kcoreaddons Description --- I was about to fork this macro (originally developed for KDevPlatform) for the 2nd time, it was time to put it somewhere where it can be shared. Basically puts together the different parts of the process so that it's comfortable to develop later on. Especially useful the fact that it makes the JSON file a dependency of the cpp file, so changes are adopted automatically. Diffs - KF5CoreAddonsMacros.cmake 7ea72af Diff: https://git.reviewboard.kde.org/r/123397/diff/ Testing --- It's being used already in KDevelop and KDE Connect. Thanks, Aleix Pol Gonzalez ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123298: Add option to show metadata of an .so file to desktoptojson
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123298/#review78683 --- AFAIK `qtplugininfo` should have all this functionality, so I don't think we need to add it here. Not sure when it was introduced, but Qt 5.5 definitively has it - Alex Richardson On April 8, 2015, 4:17 p.m., Sebastian Kügler wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123298/ --- (Updated April 8, 2015, 4:17 p.m.) Review request for KDE Frameworks, Alex Richardson, David Faure, and Marco Martin. Repository: kcoreaddons Description --- desktoptojson -s libraryfile.so dumps the metadata contained in the file to the commandline. This is useful for debugging metadata querying and plugin loading. Diffs - src/desktoptojson/desktoptojson.h bfa38b0f5ddd0581ad176d854614bc9c80dd139a src/desktoptojson/desktoptojson.cpp 82626b256df6b3bd106e6d4c6fad84d7d970af37 src/desktoptojson/main.cpp 9bac8ff55d005d1944c04f557aa9601de2b0ca15 Diff: https://git.reviewboard.kde.org/r/123298/diff/ Testing --- Tested the tool on various libraries, all show the expected data Thanks, Sebastian Kügler ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123131: Port kio_man away from kdelibs4support
On March 25, 2015, 9:54 p.m., Lukáš Tinkl wrote: man/kio_man.cpp, line 242 https://git.reviewboard.kde.org/r/123131/diff/1/?file=356605#file356605line242 Not correct, this returns only user configured list of languages, whereas it previously returned the full list. Nick Shaforostoff wrote: please see https://projects.kde.org/projects/kde/kdesdk/lokalize/repository/revisions/master/entry/src/common/languagelistmodel.cpp for the ways to get a list of all languages (there is a KDE-way and Qt-way) Alex Richardson wrote: So if I parse this correctly the KDE way is: `KConfig(QLatin1String(locale/kf5_all_languages), KConfig::NoGlobals, QStandardPaths::GenericDataLocation).groupList()`? Should there be something in KI18n or is this sufficiently rare that we can just add @deprecated use `KConfig(QLatin1String(locale/kf5_all_languages), KConfig::NoGlobals, QStandardPaths::GenericDataLocation).groupList()` instead to KLocale::languageList()? David Faure wrote: Sounds to me like it should be a method in ki18n, it's a bit too arcane to be maintainable. Seems like it's not the right replacement: No special env vars set: ``` KLocale::global()-languageList() (de, en_US) KLocalizedString::availableApplicationTranslations() = (pt_BR, nds, kk, ga, mr, km, ko, en_US, ca, gl, is, it, el, nb, tr, ro, pl, es, et, en_GB, eu, ja, ru, nl, cs, nn, pt, ar, ug, hi, lt, da, lv, uk, zh_CN, zh_TW, fi, de, sk, hr, sl, hu, bg, fr, ca@valencia, sv) QLocale().uiLanguages() = (de-DE) KConfig(kf5_all_languages).groupsList() = (ca@valencia, ln, de, tw, lo, ty, lt, lv, ug, uk, pt_BR, dz, mg, mh, mi, ur, mk, ml, mn, mo, uz, mr, ms, mt, el, en, crh, eo, my, be@latin, es, et, eu, vi, na, nb, nd, ne, vo, ng, fa, nl, nn, csb, nr, fi, fj, sr@ijekavian, nv, wa, fo, ny, fr, oc, fy, wo, ga, sr@latin, hne, om, gd, or, os, gl, uz@cyrillic, gn, gu, xh, gv, zh_HK, pa, nso, pi, ha, pl, he, hi, mai, ps, pt, ho, hr, hu, yi, hy, hz, yo, ia, id, ie, aa, ik, ab, qu, ae, za, io, af, nds, is, ven, it, iu, zh, am, ar, as, ja, sr@ijekavianlatin, x-test, ay, zu, az, rn, ro, en_US, ba, ru, be, rw, bg, bh, bi, jv, bn, sa, bo, sc, sd, br, se, bs, sg, si, ka, sk, sl, zh_CN, sm, ast, sn, so, sq, ki, sr, bn_IN , ca, ss, kk, st, kl, su, km, sv, kn, ce, sw, ko, ch, ks, rom, ku, kv, kw, ta, co, ky, zh_TW, te, cs, tg, cu, th, cv, ti, hsb, la, lb, tk, cy, tn, to, en_GB, li, tr, da, ts, tt, dsb) ``` With LANGUAGE=de_DE ``` KLocale::global()-languageList() (de, en_US) KLocalizedString::availableApplicationTranslations() = (sv, pt_BR, ca@valencia, kk, ga, mr, km, ko, en_US, nds, ca, gl, is, it, el, nb, tr, ro, en_GB, pl, es, et, eu, ja, ru, nl, cs, nn, pt, ar, ug, hi, lt, da, zh_CN, zh_TW, lv, uk, fi, de, sk, hr, sl, hu, bg, fr) QLocale().uiLanguages() = (de-DE) KConfig(kf5_all_languages).groupsList() = (...) ``` With LANGUAGE=en_GB:de_DE ``` KLocale::global()-languageList() (en_GB, de, en_US) KLocalizedString::availableApplicationTranslations() = (ga, nds, kk, mr, km, ko, ca, gl, pt_BR, is, it, el, nb, tr, ro, en_US, pl, es, et, eu, ja, ru, nl, cs, nn, pt, ar, en_GB, ug, hi, lt, da, lv, uk, fi, de, sk, hr, sl, hu, bg, fr, ca@valencia, zh_CN, zh_TW, sv) QLocale().uiLanguages() = (en-GB, de-DE) ``` With LANGUAGE=de_DE:en_GB ``` KLocale::global()-languageList() (en_GB, de, en_US) KLocalizedString::availableApplicationTranslations() = (mr, km, ko, nds, en_GB, ca, gl, is, it, el, nb, tr, ro, pl, es, et, eu, ja, ru, nl, cs, nn, zh_CN, zh_TW, pt, ar, ca@valencia, ug, hi, lt, da, lv, uk, fi, de, sk, hr, sl, hu, bg, pt_BR, fr, sv, en_US, ga, kk) QLocale().uiLanguages() = (de-DE, en-GB) KConfig(kf5_all_languages).groupsList() = (...) ``` This one is very confusing, why is en_GB still the highest priority according to KLocale::global()-languageList()? Maybe it is reading some config file where I set that? With LANGUAGE=fr_FR ``` KLocale::global()-languageList() (de, en_US) KLocalizedString::availableApplicationTranslations() = (et, eu, ja, ru, nl, cs, nn, pt, en_GB, ar, ca@valencia, ug, hi, lt, da, lv, uk, fi, de, sk, hr, sl, zh_CN, zh_TW, hu, bg, fr, sv, nds, ga, kk, mr, km, ko, pt_BR, ca, gl, en_US, is, it, el, nb, tr, ro, pl, es) QLocale().uiLanguages() = (fr-FR) ``` Even more confusion, LANGUAGE now has no effect on KLocale::global()-languageList() With LANG=fr_FR ``` KLocale::global()-languageList() (fr, en_US) KLocalizedString::availableApplicationTranslations() = (sk, hr, sl, hu, bg, pt_BR, fr, sv, en_US, kk, ga, km, mr, ko, en_GB, ca, gl, is, it, el, nb, tr, ro, pl, es, et, eu, ja, ru, nl, cs, nn, zh_CN, zh_TW, pt, ar, ca@valencia, nds, ug, hi, lt, da, lv, uk, fi, de) QLocale().uiLanguages() = (fr-FR) ``` LANG seems to work where LANGUAGE doesn't. I have no idea what's going on here, I need some feedback from someone with more I18N experience. However, QLocale().uiLanguages() seems to be almost the right replacement if we append en_US and convert it from
Review Request 123131: Port kio_man away from kdelibs4support
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123131/ --- Review request for KDE Frameworks and Plasma. Repository: kio-extras Description --- Port kio_man away from kdelibs4support Diffs - CMakeLists.txt 02ae89f2524324f758450ad368f64849e39b2f7d man/CMakeLists.txt cb4585a289d3f69b8a16429ce87bfce115d1cc27 man/kio_man.cpp e8cf2d70c50c4c20adbb5a81a6213175d397b76e man/kmanpart.cpp 3af7fc182806e8b89297e8de884ce611c827e881 man/man2html.cpp 3c493ba334bce890b450d7b11ab00c6e783708d4 Diff: https://git.reviewboard.kde.org/r/123131/diff/ Testing --- man view in kdevelop5 works Not sure about the `KLocale::global()-languageList();` - `QLocale().uiLanguages();` change though so I would like some feedback. Thanks, Alex Richardson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 123131: Port kio_man away from kdelibs4support
On März 25, 2015, 9:54 nachm., Lukáš Tinkl wrote: man/kio_man.cpp, line 242 https://git.reviewboard.kde.org/r/123131/diff/1/?file=356605#file356605line242 Not correct, this returns only user configured list of languages, whereas it previously returned the full list. Nick Shaforostoff wrote: please see https://projects.kde.org/projects/kde/kdesdk/lokalize/repository/revisions/master/entry/src/common/languagelistmodel.cpp for the ways to get a list of all languages (there is a KDE-way and Qt-way) So if I parse this correctly the KDE way is: `KConfig(QLatin1String(locale/kf5_all_languages), KConfig::NoGlobals, QStandardPaths::GenericDataLocation).groupList()`? Should there be something in KI18n or is this sufficiently rare that we can just add @deprecated use `KConfig(QLatin1String(locale/kf5_all_languages), KConfig::NoGlobals, QStandardPaths::GenericDataLocation).groupList()` instead to KLocale::languageList()? - Alex --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123131/#review78048 --- On März 25, 2015, 9:17 nachm., Alex Richardson wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/123131/ --- (Updated März 25, 2015, 9:17 nachm.) Review request for KDE Frameworks, Plasma and Martin Koller. Repository: kio-extras Description --- Port kio_man away from kdelibs4support Diffs - CMakeLists.txt 02ae89f2524324f758450ad368f64849e39b2f7d man/CMakeLists.txt cb4585a289d3f69b8a16429ce87bfce115d1cc27 man/kio_man.cpp e8cf2d70c50c4c20adbb5a81a6213175d397b76e man/kmanpart.cpp 3af7fc182806e8b89297e8de884ce611c827e881 man/man2html.cpp 3c493ba334bce890b450d7b11ab00c6e783708d4 Diff: https://git.reviewboard.kde.org/r/123131/diff/ Testing --- man view in kdevelop5 works Not sure about the `KLocale::global()-languageList();` - `QLocale().uiLanguages();` change though so I would like some feedback. Thanks, Alex Richardson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 122733: Fix path traversal checks in KPackage
On March 4, 2015, 7:23 p.m., Hrvoje Senjan wrote: this has broken wallpaper loading here... there's loads of Attempting to read file from invalid package! file type: metadata file name: package path: /usr/share/wallpapers/Aghi/ ... warnings... Marco Martin wrote: right, now an autotest fails :/ I'll look into this. The Attempting to read file from invalid package should probably only be printed if d-fallbackFilePath() returns an empty string. But that only prints a message and doesn't change the behaviour so it can't be the reason. Are there any Path traversal attempt detected: messages? - Alex --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122733/#review77011 --- On March 3, 2015, 5:53 p.m., Alex Richardson wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122733/ --- (Updated March 3, 2015, 5:53 p.m.) Review request for KDE Frameworks, Plasma and Marco Martin. Repository: kpackage Description --- They did not canonicalize the package base directory path so it would always fail when the package base path contained symlinks Diffs - src/kpackage/package.cpp eb4a09b987970e89f28587426b21d63731634087 src/kpackage/private/package_p.h e451412fa02c88113aa4c7bbca2dcda3432b2b02 Diff: https://git.reviewboard.kde.org/r/122733/diff/ Testing --- Files inside the package are now found although the install location contains a symlink Thanks, Alex Richardson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 122796: Print a warning message when loading an invalid KDeclarative::QmlObject
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/122796/ --- Review request for KDE Frameworks. Repository: kdeclarative Description --- Print a warning message when loading an invalid KDeclarative::QmlObject Diffs - src/kdeclarative/qmlobject.cpp 00478b469159dfdee3a05a9aa833bfe615e861e6 Diff: https://git.reviewboard.kde.org/r/122796/diff/ Testing --- Thanks, Alex Richardson ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
