[Differential] [Updated] D2075: Fix bug in kfiledialog.cpp that causes crashing when native widgets are used.
kfunk added a reviewer: Frameworks. REVISION DETAIL https://phabricator.kde.org/D2075 EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To: jonathans, #frameworks
[Differential] [Planned Changes To] D3393: Make compile against WinXP SDK
kfunk planned changes to this revision. kfunk added inline comments. INLINE COMMENTS > dhaumann wrote in kioglobal_p_win.cpp:87 > Or should it just return false in this case? Thinking about it, yep, probably better that way. Will update the patch. REVISION DETAIL https://phabricator.kde.org/D3393 EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To: kfunk, arichardson, #frameworks Cc: dhaumann
[Differential] [Commented On] D2854: New: ECMGenerateApiDox, for generating qch & tag files
kfunk added a comment. Heya Friedrich. You asked me to review this. From a super brief scan of the code I don't see anything blatantly wrong. I trust you that you've tested this good enough. Love the extensive documentation. What's left here from your side? REVISION DETAIL https://phabricator.kde.org/D2854 EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To: kossebau, staniek Cc: kfunk, staniek, winterz, ochurlaud, #kdevelop, #frameworks
[Differential] [Commented On] D3393: Make compile against WinXP SDK
kfunk added a comment. In https://phabricator.kde.org/D3393#63437, @dhaumann wrote: > In general ok (but why is this a review request for KSyntaxHighlighting?) Because the reviewers field auto-completion sucks :) REVISION DETAIL https://phabricator.kde.org/D3393 EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To: kfunk, arichardson, #frameworks Cc: dhaumann
[Differential] [Updated] D3393: Make compile against WinXP SDK
kfunk edited reviewers, added: Frameworks; removed: Framework: Syntax Hightlighting. REVISION DETAIL https://phabricator.kde.org/D3393 EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To: kfunk, arichardson, #frameworks Cc: dhaumann
[Differential] [Commented On] D3392: winutils_p.h: Restore compatibility with WinXP SDK
kfunk added a comment. And I meant this line: https://code.woboq.org/qt5/qtbase/src/corelib/io/qfsfileengine_win.cpp.html#547 (Woboq bug..., can't link the Windows version of `QFSFileEngine::drivers()`...) BRANCH master REVISION DETAIL https://phabricator.kde.org/D3392 EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To: kfunk, #frameworks, brauch
[Differential] [Commented On] D3392: winutils_p.h: Restore compatibility with WinXP SDK
kfunk added a comment. In https://phabricator.kde.org/D3392#63341, @brauch wrote: > Hm, there is at least one race condition here: thread A does old = ::SetErrorMode(...), then if before it resets it thread B does the same, then the old mode might never be restored. We can put a mutex but of course there might be another piece of code which does the same. So *shrug* not sure what to do, I guess it's just bad API and that's why MS replaced it. I really just wouldn't care about it. We likely never ever run in that scenario. Keep in mind, that Qt uses `Get/SetErrorMode` like this since ages: https://code.woboq.org/qt5/qtbase/src/corelib/io/qfsfileengine_unix.cpp.html#_ZN13QFSFileEngine6drivesEv (here and in other places). It's essentially the same code; not protected by a mutex or anything. > Maybe on XP we should just set the flag once on startup? At least that's not racy. REVISION DETAIL https://phabricator.kde.org/D3392 EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To: kfunk, #frameworks, brauch
[Differential] [Updated] D3392: winutils_p.h: Restore compatibility with WinXP SDK
kfunk added reviewers: brauch, Frameworks. REVISION DETAIL https://phabricator.kde.org/D3392 EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To: kfunk, brauch, #frameworks
[Differential] [Changed Subscribers] D3226: Don't be fatal on File field not being properly parsed
kfunk added inline comments.
INLINE COMMENTS
> KF5ConfigMacros.cmake:71
> file(READ ${_tmp_FILE} _contents)
> - string(REGEX MATCH "File=([^\n]+\\.kcfg)\n" "" "${_contents}")
> + string(REGEX MATCH "File=([^\n]+\\.kcfg)c?\n" "" "${_contents}")
> set(_kcfg_FILENAME "${CMAKE_MATCH_1}")
I suggest to add some commentary here.
REVISION DETAIL
https://phabricator.kde.org/D3226
EMAIL PREFERENCES
https://phabricator.kde.org/settings/panel/emailpreferences/
To: apol, #frameworks
Cc: kfunk
Re: Review Request 129260: Add find module for QtPlatformSupport
> On Oct. 25, 2016, 11:37 a.m., Aleix Pol Gonzalez wrote: > > Shouldn't this be in Qt? What am I missing? > > Martin Gräßlin wrote: > Yes it should, but it isn't. No idea why not. > > Hrvoje Senjan wrote: > The module is internal, so it intentionally doesn't install any cmake > files. Correct, that's why. See: https://bugreports.qt.io/browse/QTBUG-50073 - Kevin --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/129260/#review100257 --- On Oct. 25, 2016, 11:21 a.m., Kai Uwe Broulik wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/129260/ > --- > > (Updated Oct. 25, 2016, 11:21 a.m.) > > > Review request for KDE Frameworks, Alex Merry and Martin Gräßlin. > > > Repository: extra-cmake-modules > > > Description > --- > > Comes from KWin and will eventually be used in Plasma Integration, hence > moving it to extra-cmake-modules. > > > Diffs > - > > find-modules/FindQt5PlatformSupport.cmake PRE-CREATION > > Diff: https://git.reviewboard.kde.org/r/129260/diff/ > > > Testing > --- > > Removed it from KWin, built KWin, worked. > > Built plasma-integration with QDBusMenu private stuff, worked, although > includes there sometimes omit the QtPlatformSupport/ prefix but this is an > upstream issue since it works with the files Kwin includes. > > > Thanks, > > Kai Uwe Broulik > >
[Differential] [Closed] D3091: Windows: Don't display error dialogs
kfunk closed this revision. kfunk added a comment. Pushed: commit f544380db86301f4833b2149dd46dca47acc8042 Author: Kevin Funk <kf...@kde.org> Date: Mon Oct 17 22:02:26 2016 +0200 Windows: Don't display error dialogs REVISION DETAIL https://phabricator.kde.org/D3091 EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To: kfunk, #frameworks, vonreth, brauch Cc: vonreth, nalvarez, broulik
[Differential] [Updated, 71 lines] D3091: Windows: Don't display error dialogs
kfunk updated this revision to Diff 7495. kfunk added a comment. Add Q_DISABLE_COPY CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D3091?vs=7476=7495 BRANCH master REVISION DETAIL https://phabricator.kde.org/D3091 AFFECTED FILES src/solid/devices/backends/win/winstoragevolume.cpp src/solid/devices/backends/win/winutils_p.h EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To: kfunk, #frameworks, brauch Cc: vonreth, nalvarez, broulik
[Differential] [Commented On] D3091: Windows: Don't display error dialogs
kfunk added a comment. In https://phabricator.kde.org/D3091#57279, @nalvarez wrote: > Meanwhile I'm skeptical of the need to "set it back". Is there any case where such error dialog boxes would be desirable? It seems like their existence is an ancient-app-compatibility thing. The documentation for `SetErrorMode` recommends setting `SEM_FAILCRITICALERRORS` at app startup and leaving it that way. I'm using a similar approach as Qt does. If some application decides to use a different "global" error mode, we should not interfere with it just because Solid wants to. This is a general purpose library after all. REVISION DETAIL https://phabricator.kde.org/D3091 EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To: kfunk, #frameworks, brauch Cc: nalvarez, broulik
[Differential] [Commented On] D3091: Windows: Don't display error dialogs
kfunk added a comment. In https://phabricator.kde.org/D3091#57277, @broulik wrote: > Can't really comment on this, though. > While I'm a huge fan of RAII you don't seem to be returning early from that function RAII is not only useful for cases where you return early. > ..., can't you just set the value and set it back at the end without this new class? I'm fairly sure, we'll need to guard other WinAPI calls as well. QStorageInfo from qtbase uses `SetErrorMode/GetErrorMode` a lot REVISION DETAIL https://phabricator.kde.org/D3091 EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To: kfunk, #frameworks, brauch Cc: nalvarez, broulik
[Differential] [Updated, 70 lines] D3091: Windows: Don't display error dialogs
kfunk updated this revision to Diff 7476. kfunk added a comment. SetErrorMode -> SetThreadErrorMode (thread-safe) CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D3091?vs=7475=7476 BRANCH master REVISION DETAIL https://phabricator.kde.org/D3091 AFFECTED FILES src/solid/devices/backends/win/winstoragevolume.cpp src/solid/devices/backends/win/winutils_p.h EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To: kfunk, #frameworks, brauch Cc: nalvarez, broulik
[Differential] [Updated] D3091: Windows: Don't display error dialogs
kfunk added reviewers: Frameworks, brauch. REVISION DETAIL https://phabricator.kde.org/D3091 EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To: kfunk, #frameworks, brauch
Re: Scrap Baloo Thread Feedback
On Friday, 7 October 2016 17:24:26 CEST Vishesh Handa wrote: > Hey guys > > I was told there is a thread about scrapping Baloo. All Baloo > discussion used to happen on kde-devel and that's where the review > requests go. It's the only reason I am still subscribed to kde-devel. Heya, Baloo is a framework nowadays, therefore it totally makes sense to have the discussion on kde-framework-devel. There's been tons of discussion around Baloo on kde-framework-devel already. kde-frameworks-devel is also where the CI messages for the baloo repo go to. It likely makes sense for you to subscribe, no? Cheers, Kevin > (snip) -- Kevin Funk | kf...@kde.org | http://kfunk.org signature.asc Description: This is a digitally signed message part.
Re: Scrap baloo?
aphor - file objects can be opened with an application, > >>> people objects can be sent mails, and so on. > >>> > >>> Hope this makes sense. > >> > >> I still not see how that should work out, atm, IMHO facts are: > >> > >> 1) baloo is not maintained > > > > It will, now. > > > >> 2) lmdb will e.g. never work for us on NFS homes and the code needs major > >> overhaul > >> to handle errors (which you confirm) > > > > LMDB goes away, either way. > > > >> 3) you said you have "some time" left to maintain it, but you now propose > >> in addition to maintain > >> Baloo to write a DB system from scratch, I don't really see that working > > > > I have a personal interest, an academic interest, and now a > > KDE-related interest in the KVDB. It *will* work, because I'm the kind > > of guy who puts a lot of time and effort into things (maybe even > > disproportionately so) into things that genuinely interest me. My > > challenge will be to make the codebase so that after I'm done with > > this (say in about 5 years or so) it'll be comprehensible to the next > > maintainer. > > As stated above, I don't doubt that your are capable and earnest and hard > working. But I don't see that we should prototype & develop a database, > alone the work on top of that (what baloo does atm) will take months to get > right. > >> 4) tracker on the other side is maintained and in use and we can share > >> the index data with GNOME and others > >> > >> I really doubt that doing the work to remove lmdb, replace it with an > >> "own one" and then starting > >> to fix all other issues (like indexer running amok, broken file > >> extractors, ...) will work out if > >> we don't clone some more people. > >> > >> But that is only my opinion. > > > > *Sigh* > > > > I don't want to take the easy way out here. Half the fun in KDE is > > doing crazy things and seeing your baby work. That's the entire > > motivation for being here. > > > > And right now I'm volunteering to do this. Just chiming in here since I got a little worried when reading there are some foggy plans to 'roll our own' KVDB... > I appreciate that, I only would like to avoid to have once more a > indexer/search that starts from scratch and is left unmaintained. > We had strigi based stuff, we had nepomuk and now we have baloo, all more or > less from scratch and all ended up unmaintained and underdocumented (strigi > actually had more docs I remember ;=). Yes. Please, pretty please, don't reinvent the wheel here again, please don't consider an academic research project as production-ready replacement for a database backend. This is (sad) history repeating indeed. There are alternatives for the DB at least, which work, are maintained (by more than one person) and where using them won't put another burden on us KDE developers who are lacking manpower in all different areas already. > Actually, I don't insist on a tracker based solution, but I would like to > have some that doesn't end up in "KDE reinvents the wheel" once more if > there are perhaps alternatives available. As Christoph I don't care about a specific solution, but going the NIH route sounds, by far, like the worst option. I'm not questioning Boudhayan's credibility to work out a great "draft" implementation of a KVDB for academic research... But, the *major* selling points of database implementations is a track record of being rock-stable in different environments for a continuous amount of time. There's no way you can guarantee this for a one-man academic research project. Please reconsider the options. Just 2c of a worried user, fearing even more Plasma workspace instabilities... Kevin > Beside the speed argument, I got nothing that e.g. is a contra again using > tracker in this mail. > > (David had some good points in his mail, e.g. if sqlite doesn't have nfs > probs, yes, if locking is broken, or how to migrate the xattr data, which > is not solved yet) > > Greetings > Christoph -- Kevin Funk | kf...@kde.org | http://kfunk.org signature.asc Description: This is a digitally signed message part.
Re: Review Request 129020: Fix find invocation in validate_svg.sh on FreeBSD.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/129020/#review99522 --- Ship it! Ship It! - Kevin Funk On Sept. 26, 2016, 9:45 a.m., Gleb Popov wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/129020/ > --- > > (Updated Sept. 26, 2016, 9:45 a.m.) > > > Review request for KDE Frameworks. > > > Repository: breeze-icons > > > Description > --- > > Without this patch the build was failing with > > ``` > find: illegal option -- n > usage: find [-H | -L | -P] [-EXdsx] [-f path] path ... [expression] >find [-H | -L | -P] [-EXdsx] -f path [path ...] [expression] > ``` > > > Diffs > - > > validate_svg.sh bbc3076 > > Diff: https://git.reviewboard.kde.org/r/129020/diff/ > > > Testing > --- > > > Thanks, > > Gleb Popov > >
Re: C++11 & friends
On Monday, 12 September 2016 09:10:46 CEST Jaroslaw Staniek wrote: > On 12 September 2016 at 08:04, Kevin Funk <kf...@kde.org> wrote: > > On Sunday, 11 September 2016 03:21:21 CEST Dominik Haumann wrote: > > > Hi all, > > > > > > I just saw a commit by Volker turning nullptr into Q_NULLPTR with the > > > comment that Visual Studio 2012 does not support nullptr. > > > > > > While this change is trivial for obvious reasons, do we really need to > > > do > > > that? > > > > I don't think so. > > > > nullptr is actually supported since VS2010 [1]. > > > > Some other remark: I think noone's using anything below VS2015 for testing > > KDE > > on Windows anyway. I'm expecting compilation to be broken on earlier > > versions > > of VS. > > I am not only testing but also using/depend on 2013, which has quite some > C++11 features already. Alright, thanks for letting us know. > The builds of KF5 just work (this is not a CI, this > is building from time to time). Sometimes patching is needed here and there > because of completely unnecessary - for my context - dependency of > kdoctools. Things improved here in 2016 anyway. > > If not for other reasons, please note that people may depend on < 2015 > because some other component is not supported on 2015 or because od the > boss'/customer's request. > > I think fixing on a 'beeding edge' compiler may be not the best strategy, > yet I see no reason to be much less flexible in this regard than Qt is. VS2015 is no longer 'bleeding edge', it's at Update 3 already and in my experience it's a highly reliable version of VS to this date. That said, depending on VS2013 sounds fine to me of course. Cheers, Kevin > > Cheers, > > Kevin > > > > [1] http://en.cppreference.com/w/cpp/compiler_support for evidence > > > > > I am raising this question especially since KTextEditor seems to use > > > 'nullptr' in several locations now for several releases - and noone > > > complained. > > > > > > Are we supposed to turn nullptr in KTextEditor also into nullptr, or > > > can we take the liberty and ditch Q_NULLPTR completely for all > > > frameworks? > > > > > > Same also applies to 'override'. > > > > > > Best, > > > Dominik > > > > -- > > Kevin Funk | kf...@kde.org | http://kfunk.org -- Kevin Funk | kf...@kde.org | http://kfunk.org signature.asc Description: This is a digitally signed message part.
Re: C++11 & friends
On Sunday, 11 September 2016 03:21:21 CEST Dominik Haumann wrote: > Hi all, > > I just saw a commit by Volker turning nullptr into Q_NULLPTR with the > comment that Visual Studio 2012 does not support nullptr. > > While this change is trivial for obvious reasons, do we really need to do > that? I don't think so. nullptr is actually supported since VS2010 [1]. Some other remark: I think noone's using anything below VS2015 for testing KDE on Windows anyway. I'm expecting compilation to be broken on earlier versions of VS. Cheers, Kevin [1] http://en.cppreference.com/w/cpp/compiler_support for evidence > I am raising this question especially since KTextEditor seems to use > 'nullptr' in several locations now for several releases - and noone > complained. > > Are we supposed to turn nullptr in KTextEditor also into nullptr, or > can we take the liberty and ditch Q_NULLPTR completely for all > frameworks? > > Same also applies to 'override'. > > Best, > Dominik -- Kevin Funk | kf...@kde.org | http://kfunk.org signature.asc Description: This is a digitally signed message part.
Re: Review Request 128855: Fix build on Windows
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/128855/#review98993 --- Ship it! Ship It! - Kevin Funk On Sept. 7, 2016, 3:23 p.m., Gleb Popov wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/128855/ > --- > > (Updated Sept. 7, 2016, 3:23 p.m.) > > > Review request for KDE Frameworks. > > > Repository: kio > > > Description > --- > > Windows doesn't have `S_IRUSR` & co defines, so use what it has and stub out > remainings. > > > Diffs > - > > src/core/slavebase.cpp 4b8f460 > > Diff: https://git.reviewboard.kde.org/r/128855/diff/ > > > Testing > --- > > > Thanks, > > Gleb Popov > >
Re: Review Request 128855: Fix build on Windows
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/128855/#review98981 --- `kioglobal_p.h` defines those. Just include that one? - Kevin Funk On Sept. 7, 2016, 3:23 p.m., Gleb Popov wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/128855/ > --- > > (Updated Sept. 7, 2016, 3:23 p.m.) > > > Review request for KDE Frameworks. > > > Repository: kio > > > Description > --- > > Windows doesn't have `S_IRUSR` & co defines, so use what it has and stub out > remainings. > > > Diffs > - > > src/core/slavebase.cpp 4b8f460 > > Diff: https://git.reviewboard.kde.org/r/128855/diff/ > > > Testing > --- > > > Thanks, > > Gleb Popov > >
[Differential] [Commented On] D2545: Cleanup KSharedUiServerProxy before qApp exists
kfunk added a comment. Here's the backtrace (note: using Qt 5.6 branch): ntdll.dll!NtWaitForSingleObject () Unknown KernelBase.dll!7ffbcb2eaadf() Unknown > Qt5Core.dll!QWaitCondition::wait(QMutex * mutex=0x01c24fd16520, unsigned long time=4294967295) Line 178 C++ Qt5Core.dll!QSemaphore::acquire(int n=1) Line 136 C++ Qt5Core.dll!QMetaObject::activate(QObject * sender=0x01c24a6eb500, int signalOffset, int local_signal_index, void * * argv=0x0081e131f648) Line 3699C++ Qt5Core.dll!QObject::~QObject() Line 913C++ Qt5DBus.dll!QDBusServiceWatcher::`vector deleting destructor'(unsigned int) C++ Qt5Core.dll!QObjectPrivate::deleteChildren() Line 1960 C++ Qt5Core.dll!QObject::~QObject() Line 1034 C++ KF5JobWidgets.dll!KSharedUiServerProxy::~KSharedUiServerProxy() Line 325C++ KF5JobWidgets.dll!``anonymous namespace'::Q_QGS_serverProxy::innerFunction'::`2'::`dynamic atexit destructor for 'holder''()C++ ucrtbase.dll!_time64 () Unknown ucrtbase.dll!__crt_seh_guarded_call::operator(),class &,class >(class &&,class &,class &&) Unknown ucrtbase.dll!_execute_onexit_table () Unknown KF5JobWidgets.dll!dllmain_crt_process_detach(const bool is_terminating=true) Line 109 C++ KF5JobWidgets.dll!dllmain_dispatch(HINSTANCE__ * const instance=0x7ffbb8e1, const unsigned long reason=0, void * const reserved=0x0001) Line 207C++ ntdll.dll!LdrpCallInitRoutine() Unknown ntdll.dll!LdrShutdownProcess() Unknown ntdll.dll!RtlExitUserProcess() Unknown kernel32.dll!ExitProcessImplementation () Unknown ucrtbase.dll!swprintf ()Unknown ucrtbase.dll!swprintf ()Unknown kdevelop.exe!__scrt_common_main_seh() Line 262 C++ kernel32.dll!BaseThreadInitThunk () Unknown ntdll.dll!RtlUserThreadStart() Unknown BRANCH master REVISION DETAIL https://phabricator.kde.org/D2545 EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To: kfunk, vonreth, dfaure Cc: arrowdodger, #frameworks
[Differential] [Commented On] D2545: Cleanup KSharedUiServerProxy before qApp exists
kfunk added a comment. Bump? REVISION DETAIL https://phabricator.kde.org/D2545 EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To: kfunk, dfaure, vonreth Cc: #frameworks
[Differential] [Commented On] D2546: Cleanup DBus-related resources before qApp exits
kfunk added a comment. Bump? REVISION DETAIL https://phabricator.kde.org/D2546 EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To: kfunk, dfaure, vonreth Cc: #frameworks
[Differential] [Updated, 19 lines] D2545: Cleanup KSharedUiServerProxy before qApp exists
kfunk updated this revision to Diff 6175. kfunk added a comment. Fix typo in commit msg CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D2545?vs=6173=6175 BRANCH master REVISION DETAIL https://phabricator.kde.org/D2545 AFFECTED FILES src/kuiserverjobtracker.cpp src/kuiserverjobtracker_p.h EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To: kfunk, dfaure, vonreth Cc: #frameworks
[Differential] [Updated] D2546: Cleanup DBus-related resources before qApp exits
kfunk updated the test plan for this revision. kfunk added reviewers: dfaure, vonreth. kfunk added a subscriber: Frameworks. REVISION DETAIL https://phabricator.kde.org/D2546 EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To: kfunk, dfaure, vonreth Cc: #frameworks
[Differential] [Updated] D2545: Cleanup KSharedUiServerProxy before qApp exists
kfunk added reviewers: dfaure, vonreth. kfunk added a subscriber: Frameworks. REVISION DETAIL https://phabricator.kde.org/D2545 EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To: kfunk, dfaure, vonreth Cc: #frameworks
Re: Review Request 128661: [KTreeWidgetsSearchLineWidget] Use placeholderText instead of separate label
> On Aug. 14, 2016, 7:17 p.m., Thomas Pfeiffer wrote: > > Actually, this is in violation of the current search HIG ( > > https://community.kde.org/KDE_Visual_Design_Group/HIG/SearchPattern ), but > > since the de-facto standard in our software and elsewhere is an inline > > hint, I suppose we should update the HIG. > > > > So yes, +1, good change! One drawback: The search line is no longer acccessible via keyboard mnemonics if it's just an inline hint. Is this something you're aware of in the VDG team? Any solutions/work-arounds? - Kevin --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/128661/#review98384 --- On Aug. 12, 2016, 8:22 a.m., Kai Uwe Broulik wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/128661/ > --- > > (Updated Aug. 12, 2016, 8:22 a.m.) > > > Review request for KDE Frameworks and KDE Usability. > > > Repository: kitemviews > > > Description > --- > > This makes the search look identical to virtually any other search box used > throughout the workspace. > > > Diffs > - > > src/ktreewidgetsearchline.cpp e40a61e > src/ktreewidgetsearchlinewidget.cpp 9af4c2e > > Diff: https://git.reviewboard.kde.org/r/128661/diff/ > > > Testing > --- > > Tests still pass. > > Looks good. I couldn't figure out how to just create the QLineEdit without > the intermediate QHBoxLayout. > > > File Attachments > > > Separate "Search:" label gone > > https://git.reviewboard.kde.org/media/uploaded/files/2016/08/12/c1e2e92d-2113-4d9f-9594-a2e1db0545c2__Screenshot_20160812_102022.png > > > Thanks, > > Kai Uwe Broulik > >
Re: Review Request 127023: [KFileMetadata] Support Origin Email subject/sender/id
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127023/#review98302 --- I'm late to the party, but ...: This won't fly on Windows (at least under MSVC). I'm not usually compiling kfilemetadata.git on Windows, so I wasn't affected by this change thus didn't notice. Can we make the dependency on xattr optional so we can restore the build with MSVC? - Kevin Funk On Feb. 26, 2016, 7:56 p.m., Kai Uwe Broulik wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/127023/ > --- > > (Updated Feb. 26, 2016, 7:56 p.m.) > > > Review request for KDE Frameworks, KDEPIM, Daniel Vrátil, Sebastian Kügler, > and Vishesh Handa. > > > Repository: kfilemetadata > > > Description > --- > > This adds support for the user.xdg.origin.email.subject, > user.xdg.origin.email.sender, user.xdg.origin.email.message-id xattrs [1] to > KFileMetadata. > > This can (should :P) be populated by KMail when you save an attachment. > > Not too happy about the "displayName" I chose. Also we'll need to figure out > what to index and how we can relate this information and present it to the > user in a meaningful way. But at least let's collect the information first > and then we can think about ways to handle this. > > [1] https://wiki.freedesktop.org/www/CommonExtendedAttributes/ > > > Diffs > - > > src/properties.h 6ceaca5 > src/propertyinfo.cpp 4d1fac4 > src/usermetadata.h 9e10d2a > src/usermetadata.cpp 5485d0e > > Diff: https://git.reviewboard.kde.org/r/127023/diff/ > > > Testing > --- > > Not really. Tests pass, though. > > > Thanks, > > Kai Uwe Broulik > >
Re: Review Request 128650: [KBookmarks] Fix crash on app shutdown due to QRegExp(QStringLiteral())
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/128650/#review98301 --- Rather port to `QRegularExpression` right away? - Kevin Funk On Aug. 10, 2016, 3:21 p.m., Daniel Vrátil wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/128650/ > --- > > (Updated Aug. 10, 2016, 3:21 p.m.) > > > Review request for KDE Frameworks. > > > Repository: kbookmarks > > > Description > --- > > Kleopatra was crashing on shutdown during cleanup of global statics with > QRegExps. Based on [0] I was able to trace the pointer back to .rodata in > libKF5KBookmarks.so. The crash is gone when QRegExp(QLatin1String()) is used. > > > [0] https://blogs.kde.org/2015/11/05/qregexp-qstringliteral-crash-exit > > > Diffs > - > > src/kbookmarkimporter_ie.cpp 7dcbfb3 > src/kbookmarkmanager.cpp dd7a98b > > Diff: https://git.reviewboard.kde.org/r/128650/diff/ > > > Testing > --- > > > Thanks, > > Daniel Vrátil > >
[Differential] [Closed] D2142: KGlobalAccel: Fix deadlock on exit under Windows
kfunk closed this revision. kfunk added a comment. Pushed. commit 2bdb684a872aff26c01f5e1fc175fbaf663ad8ba Author: Kevin Funk <kf...@kde.org> Date: Tue Jul 12 10:18:40 2016 +0200 REVISION DETAIL https://phabricator.kde.org/D2142 EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To: kfunk, vonreth, graesslin, dfaure Cc: graesslin, #frameworks ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
[Differential] [Commented On] D2142: KGlobalAccel: Fix deadlock on exit under Windows
kfunk added a comment. FYI: There are more issues in other frameworks, the next deadlock is in kiconthemes... BRANCH master REVISION DETAIL https://phabricator.kde.org/D2142 EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To: kfunk, dfaure, vonreth, graesslin Cc: graesslin, #frameworks ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
[Differential] [Commented On] D2142: KGlobalAccel: Fix deadlock on exit under Windows
kfunk added a comment. Waiting for a +1 from David. There's one possible point of regression, namely if `KGlobalAccel::self()` is being used after qApp ran all the post routines (and `KGlobalAccelPrivate::cleanup()` has been invoked already). Not sure if this is a problem, though. BRANCH master REVISION DETAIL https://phabricator.kde.org/D2142 EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To: kfunk, dfaure, vonreth, graesslin Cc: graesslin, #frameworks ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
[Differential] [Updated] D2142: KGlobalAccel: Fix deadlock on exit under Windows
kfunk added reviewers: dfaure, vonreth. kfunk added a subscriber: Frameworks. REVISION DETAIL https://phabricator.kde.org/D2142 EMAIL PREFERENCES https://phabricator.kde.org/settings/panel/emailpreferences/ To: kfunk, dfaure, vonreth Cc: #frameworks ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 124905: Win: Hide console window for binaries in LIBEXEC
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124905/ --- (Updated July 11, 2016, 4:15 p.m.) Status -- This change has been discarded. Review request for KDE Frameworks, Alex Merry and David Faure. Repository: kio Description --- Win: Hide console window for binaries in LIBEXEC Diffs - src/ioslaves/http/CMakeLists.txt 76a8e2800b84c312431cc1996ac81d1ef6fb5cfc src/ioslaves/http/kcookiejar/CMakeLists.txt 7b4778d1f67c1ad9f9edcaa4692b39ee6fe3f365 src/kioexec/CMakeLists.txt 91284a3a61b86770b4d1939da52d256840803608 src/kioslave/CMakeLists.txt e02febd380b268c596e8ecc3b745b6f50993ab4e src/kpac/CMakeLists.txt fc5989714480ca49b5bd72e1c7b458b26bd0d9bc Diff: https://git.reviewboard.kde.org/r/124905/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 124905: Win: Hide console window for binaries in LIBEXEC
> On April 18, 2016, 11:05 a.m., Gleb Popov wrote:
> > Let's do something about this. I see two solutions for this:
> >
> > - Add another macro. If we go this route, can we put it in the same .cmake
> > file?
> > - Add a keyword parameter to the existing macro. Because macro uses
> > `${ARGN}` it shouldn't break anything.
>
> Kevin Funk wrote:
> @dfaure: Do you think this issue should be fixed in Qt itself?
>
> I've just found
> http://stackoverflow.com/questions/33874243/qprocessstartdetached-but-hide-console-window
> which supports Patrick's remarks about "how we did it in KDE4 times".
>
> * We use `QProcess:startDetached` in KF5, in combination with
> "KDE_FORK_SLAVES" (=> causes console win)
> * We've been leaking QProcess and used `QProcess::start` in KDE4 times
> for 'kdeinit4.exe' (=> no console win).
>
> I haven't looked at the Windows implementation of QProcess yet, but maybe
> it's feasible to extend `QProcess::startDetached`? Ideas? Opinions?
>
> /me wonders why noone bothered fixing this in Qt itself yet (lots of
> discussions about the issues in forums)
>
> David Faure wrote:
> Yes this sounds like a Qt bug in startDetached() then.
Handled upstream: https://bugreports.qt.io/browse/QTBUG-53833 (Qt was already
patched, fix will be in 5.8.0)
- Kevin
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124905/#review94684
---
On Dec. 3, 2015, 8 a.m., Kevin Funk wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/124905/
> ---
>
> (Updated Dec. 3, 2015, 8 a.m.)
>
>
> Review request for KDE Frameworks, Alex Merry and David Faure.
>
>
> Repository: kio
>
>
> Description
> ---
>
> Win: Hide console window for binaries in LIBEXEC
>
>
> Diffs
> -
>
> src/ioslaves/http/CMakeLists.txt 76a8e2800b84c312431cc1996ac81d1ef6fb5cfc
> src/ioslaves/http/kcookiejar/CMakeLists.txt
> 7b4778d1f67c1ad9f9edcaa4692b39ee6fe3f365
> src/kioexec/CMakeLists.txt 91284a3a61b86770b4d1939da52d256840803608
> src/kioslave/CMakeLists.txt e02febd380b268c596e8ecc3b745b6f50993ab4e
> src/kpac/CMakeLists.txt fc5989714480ca49b5bd72e1c7b458b26bd0d9bc
>
> Diff: https://git.reviewboard.kde.org/r/124905/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 128249: Add switch to disable KParts' handling of window titles
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/128249/#review96850 --- Cool, thanks Andreas! - Kevin Funk On June 24, 2016, 8:17 p.m., Andreas Cord-Landwehr wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/128249/ > --- > > (Updated June 24, 2016, 8:17 p.m.) > > > Review request for KDE Frameworks, David Faure and Kevin Funk. > > > Repository: kparts > > > Description > --- > > Applications that use several KParts, like KDevelop, want to do the > window title settings themselves. To enable that without causing a > flickering of the KParts' title and the application's title, an > explicit switch is required, since simply removing the setWindowTitle > connection after executing a KParts::MainWindow::createGUI(...) call still > causes an initial flickering, when the GUIActivateEvent is sent in > the createGUI method. Sending such an event should stay in the createGUI > method, though. > > > Diffs > - > > src/mainwindow.h 61a92e3 > src/mainwindow.cpp be0b7dd > src/part.h 29791d5 > > Diff: https://git.reviewboard.kde.org/r/128249/diff/ > > > Testing > --- > > > 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 128249: Add switch to disable KParts' handling of window titles
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/128249/#review96772
---
Rest LGTM
src/mainwindow.cpp (line 81)
<https://git.reviewboard.kde.org/r/128249/#comment65362>
Turn boolean parameter into enum maybe?
E.g.:
```
enum CreationFlag
{
ManageWindowTitle = 0 << 1
}
Q_DECLARE_FLAGS(CreationFlag, CreationFlags)
void createGUI(Part*, CreationFlags flags);
```
Makes the user code a bit easier to read.
- Kevin Funk
On June 19, 2016, 12:35 p.m., Andreas Cord-Landwehr wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128249/
> ---
>
> (Updated June 19, 2016, 12:35 p.m.)
>
>
> Review request for KDE Frameworks, David Faure and Kevin Funk.
>
>
> Repository: kparts
>
>
> Description
> ---
>
> Applications that use several KParts, like KDevelop, want to do the
> window title settings themselves. To enable that without causing a
> flickering of the KParts' title and the application's title, an
> explicit switch is required, since simply removing the setWindowTitle
> connection after executing a KParts::MainWindow::createGUI(...) call still
> causes an initial flickering, when the GUIActivateEvent is sent in
> the createGUI method. Sending such an event should stay in the createGUI
> method, though.
>
>
> Diffs
> -
>
> src/mainwindow.h 61a92e32e638c187253ba9e2cf0d8a410e9966af
> src/mainwindow.cpp be0b7ddc637d509f4c8191134096e365616ab38a
>
> Diff: https://git.reviewboard.kde.org/r/128249/diff/
>
>
> Testing
> ---
>
>
> 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 128194: Mark helper exe as non-gui app
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/128194/#review96525 --- Ship it! Ship It! - Kevin Funk On June 15, 2016, 3:43 p.m., Christoph Cullmann wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/128194/ > --- > > (Updated June 15, 2016, 3:43 p.m.) > > > Review request for KDE Frameworks and Kevin Funk. > > > Repository: sonnet > > > Description > --- > > Mark parsetrigram helper as non-gui app, no app bundle for that on mac. > Guess not wanted on Windows, too, should be command line application. > > > Diffs > - > > data/CMakeLists.txt c3e643a > > Diff: https://git.reviewboard.kde.org/r/128194/diff/ > > > Testing > --- > > make && Co. works > > > 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 128170: Allow nsspellcheck to be compiled on mac per default
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/128170/#review96430 --- src/plugins/nsspellchecker/CMakeLists.txt (line 12) <https://git.reviewboard.kde.org/r/128170/#comment65135> Nitpick: Excessive whitespace - Kevin Funk On June 13, 2016, 7:43 a.m., Christoph Cullmann wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/128170/ > --- > > (Updated June 13, 2016, 7:43 a.m.) > > > Review request for KDE Frameworks. > > > Repository: sonnet > > > Description > --- > > Allow nsspellcheck to be compiled on mac per default > > > Diffs > - > > src/plugins/CMakeLists.txt f275f63 > src/plugins/nsspellchecker/CMakeLists.txt PRE-CREATION > > Diff: https://git.reviewboard.kde.org/r/128170/diff/ > > > Testing > --- > > make && make install work on a mac. > > > 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 128170: Allow nsspellcheck to be compiled on mac per default
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/128170/#review96431 --- Ship it! Rest LGTM - Kevin Funk On June 13, 2016, 7:43 a.m., Christoph Cullmann wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/128170/ > --- > > (Updated June 13, 2016, 7:43 a.m.) > > > Review request for KDE Frameworks. > > > Repository: sonnet > > > Description > --- > > Allow nsspellcheck to be compiled on mac per default > > > Diffs > - > > src/plugins/CMakeLists.txt f275f63 > src/plugins/nsspellchecker/CMakeLists.txt PRE-CREATION > > Diff: https://git.reviewboard.kde.org/r/128170/diff/ > > > Testing > --- > > make && make install work on a mac. > > > Thanks, > > Christoph Cullmann > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: KApiDox Maintainship
On Dienstag, 7. Juni 2016 18:09:19 CEST Aleix Pol wrote: > On Tue, Jun 7, 2016 at 7:28 AM, Olivier Churlaud <oliv...@churlaud.com> wrote: > > Hi, > > > > I did a lot of work on KApiDox the last weeks, and I have still a lot in > > > > mind to improve it. I spoke with Alex about the maintainship of KApidox as > > he doesn't have currently time to spend on it. He told me he was ok to > > pass > > me the maintainship. > > > > I don't know how this work, so I would like some explanations about what > > > > being the maintainer means, what are the responsibilities and constrains, > > and of course, after I'll know that, if it's ok that I change the Alex's > > name with mine for Kapidox maintainer. > > +1 +10. Thanks Olivier! Go ahead and make our apidocs rock again! Cheers, Kevin > Thanks Olivier! KApiDox does need that love! > And thanks Alex for the work so far! :) > > Aleix > ___ > Kde-frameworks-devel mailing list > Kde-frameworks-devel@kde.org > https://mail.kde.org/mailman/listinfo/kde-frameworks-devel -- Kevin Funk | kf...@kde.org | http://kfunk.org signature.asc Description: This is a digitally signed message part. ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 127911: Add a CMake option to build binary Qt resource out of icons dir.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127911/#review95522 --- Ship it! Ship It! - Kevin Funk On May 17, 2016, 5:55 a.m., Gleb Popov wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/127911/ > --- > > (Updated May 17, 2016, 5:55 a.m.) > > > Review request for KDE Frameworks. > > > Repository: breeze-icons > > > Description > --- > > I copied icons into the binary dir, because i haven't found a way to generate > rcc without polluting source dir. > > Not sure if installation dir is right, too. > > > Diffs > - > > CMakeLists.txt 2147705 > icons-dark/CMakeLists.txt 36d37f1 > icons/CMakeLists.txt 5ded49c > > Diff: https://git.reviewboard.kde.org/r/127911/diff/ > > > Testing > --- > > > Thanks, > > Gleb Popov > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 124905: Win: Hide console window for binaries in LIBEXEC
> On April 18, 2016, 11:05 a.m., Gleb Popov wrote:
> > Let's do something about this. I see two solutions for this:
> >
> > - Add another macro. If we go this route, can we put it in the same .cmake
> > file?
> > - Add a keyword parameter to the existing macro. Because macro uses
> > `${ARGN}` it shouldn't break anything.
@dfaure: Do you think this issue should be fixed in Qt itself?
I've just found
http://stackoverflow.com/questions/33874243/qprocessstartdetached-but-hide-console-window
which supports Patrick's remarks about "how we did it in KDE4 times".
* We use `QProcess:startDetached` in KF5, in combination with "KDE_FORK_SLAVES"
(=> causes console win)
* We've been leaking QProcess and used `QProcess::start` in KDE4 times for
'kdeinit4.exe' (=> no console win).
I haven't looked at the Windows implementation of QProcess yet, but maybe it's
feasible to extend `QProcess::startDetached`? Ideas? Opinions?
/me wonders why noone bothered fixing this in Qt itself yet (lots of
discussions about the issues in forums)
- Kevin
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124905/#review94684
-------
On Dec. 3, 2015, 8 a.m., Kevin Funk wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/124905/
> ---
>
> (Updated Dec. 3, 2015, 8 a.m.)
>
>
> Review request for KDE Frameworks, Alex Merry and David Faure.
>
>
> Repository: kio
>
>
> Description
> ---
>
> Win: Hide console window for binaries in LIBEXEC
>
>
> Diffs
> -
>
> src/ioslaves/http/CMakeLists.txt 76a8e2800b84c312431cc1996ac81d1ef6fb5cfc
> src/ioslaves/http/kcookiejar/CMakeLists.txt
> 7b4778d1f67c1ad9f9edcaa4692b39ee6fe3f365
> src/kioexec/CMakeLists.txt 91284a3a61b86770b4d1939da52d256840803608
> src/kioslave/CMakeLists.txt e02febd380b268c596e8ecc3b745b6f50993ab4e
> src/kpac/CMakeLists.txt fc5989714480ca49b5bd72e1c7b458b26bd0d9bc
>
> Diff: https://git.reviewboard.kde.org/r/124905/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 127380: More conservative approach to saving some disk accesses
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127380/#review93636 --- Since you asked for review: I don't know the code enough to judge. Generally I'd say "yes". Does `iconPath(..., KIconLoader::MatchBest)` return results you'd have received with `iconPath(..., KIconLoader::MatchExact)`? - Kevin Funk On March 15, 2016, 1:52 a.m., Aleix Pol Gonzalez wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/127380/ > --- > > (Updated March 15, 2016, 1:52 a.m.) > > > Review request for KDE Frameworks and Christoph Feck. > > > Repository: kiconthemes > > > Description > --- > > My previous approach to KIconThemes felt like a dead end, I decided I'll take > a more conservative approach. Here's a first step. > > > Diffs > - > > autotests/CMakeLists.txt 61e81f6 > autotests/kiconloader_benchmark.cpp PRE-CREATION > src/kiconloader.cpp 75ab482 > src/kicontheme.cpp 0996054 > > Diff: https://git.reviewboard.kde.org/r/127380/diff/ > > > Testing > --- > > Attempts to call `KIconThemeDir::iconPath` on the benchmark: 4318 after/5040 > before= 17% less QFile::exists > > > 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 127406: Fix encoding for gap.xml (use UTF-8)
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127406/#review93635 --- Ship it! Ship It! - Kevin Funk On March 17, 2016, 12:36 a.m., Luigi Toscano wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/127406/ > --- > > (Updated March 17, 2016, 12:36 a.m.) > > > Review request for Kate, KDE Frameworks and Daniel Vrátil. > > > Repository: ktexteditor > > > Description > --- > > The file is really ASCII-only (7 bit), so both ISO-8859-15 and UTF-8 works > but no need to use a deprecated encoding. > (this could help systems were support for those encodings has been removed). > > See also: https://mail.kde.org/pipermail/release-team/2016-March/009327.html > > > Diffs > - > > src/syntax/data/gap.xml 1af543d > > Diff: https://git.reviewboard.kde.org/r/127406/diff/ > > > Testing > --- > > > Thanks, > > Luigi Toscano > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Qt 5.6/QtWebkit
On Wednesday, March 16, 2016 5:59:30 PM CET René J. V. Bertin wrote: > Aleix Pol wrote: > > https://code.qt.io/cgit/qt/qtwebkit.git/ > > Right, thanks, I'd seen that one too (there's also a clone on github but it > isn't clear how recent/unmodified that one is). > > What I didn't yet see is if there's a way to do tarball fetches from > code.qt.io that correspond to a specific commit, tag or release (purely for > convenience in a packaging script)? I think qt-developm...@qt-project.org is a better fit for this question. Cheers, Kevin > R. > > ___ > Kde-frameworks-devel mailing list > Kde-frameworks-devel@kde.org > https://mail.kde.org/mailman/listinfo/kde-frameworks-devel -- Kevin Funk | kf...@kde.org | http://kfunk.org signature.asc Description: This is a digitally signed message part. ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 127236: Leverage QDir to know what's in a KIconThemeDir
> On March 1, 2016, 4:37 p.m., Aleix Pol Gonzalez wrote:
> > Eh... I just realized it's not 100% correct. We have a test
> > (testUnknownIconNotCached) that fails, unsure how I didn't see it yesterday.
> >
> > The problem with this one is that we are not reacting when icons are
> > introduced at runtime. I tried adding a sneaky QFileSystemWatcher/KDirWatch
> > on KIconTheme but it didn't signal.
> >
> > Should I just discard the patch?
>
> David Edmundson wrote:
> Could you compare modified time of mBaseDirThemeDir and reload if needed?
> it'd be one extra stat on the directory each time, but still a saving
> from before
>
> Aleix Pol Gonzalez wrote:
> That doesn't work, for some reason...
> ```
> // Install the existing icon by copying.
> +qDebug() << "1" <<
> QFileInfo(actionIconsDir).lastModified();
> QVERIFY(QFile::copy(QStringLiteral(":/test-22x22.png"),
> newIconPath));
> +qDebug() << "2" <<
> QFileInfo(actionIconsDir).lastModified();
> ```
>
> Both return the same value.
>
> Nick Shaforostoff wrote:
> there is a difference between changing a file from inside the program and
> changing it from outside.
>
> did you make the change externally when testing QFileSystemWatcher-based
> solution?
>
> Nick Shaforostoff wrote:
> also for the QFileInfo::lastModified() there may be just not enough time
> between the tests, so try adding a 2-minute sleep there
>
> David Faure wrote:
> QFileInfo::lastModified() doesn't include milliseconds yet. There is a
> pending patch for Qt to implement that (as part of a much bigger task which
> also adds QFile::setFileTime()), but yeah, that's the problem. Right now you
> need a 1s sleep whenever unittesting changes to lastModified().
>
> QTest::qWait(1000); // remove this once lastModified includes ms
>
> Or at least "up to 1s, until the end of the current second", I have code
> for that in the kdirwatch unittest for instance.
>
> Another alternative, hacking the modification time on files (using utime,
> like I do in ksycocatest.cpp)
>
> (all this to avoid tests that take 15 seconds in total, I like the policy
> that a test should run in under 5s -- otherwise people stop running them)
>
> Nick Shaforostoff wrote:
> so, any news on this one?
>
> Nick Shaforostoff wrote:
> even simple solution of discarding the cache after certain point of time
> (e.g. 10 seconds) and going uncached code path would give us the benefit
> without introducing regression, right?
/me likes that idea. You could make the cache lifetime configurable via
environment variables, and set that to the minimum for the
`testUnknownIconNotCached` test.
@Apol: Awesome idea indeed, this reduces a lot of strain on the file system!
- Kevin
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127236/#review93023
---
On March 1, 2016, 3:25 a.m., Aleix Pol Gonzalez wrote:
>
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/127236/
> ---
>
> (Updated March 1, 2016, 3:25 a.m.)
>
>
> Review request for KDE Frameworks and Christoph Feck.
>
>
> Repository: kiconthemes
>
>
> Description
> ---
>
> At the moment we're playing Battleship to see if an icon is present in a
> subdirectory. This means that we are checking on every directory if there's
> an icon that matches the size with a said name on every request.
>
> This can be seen easily with strace:
> ```
> $ strace kwrite |& grep ENOENT | wc -l
> 6212
> ```
> After the patch:
> ```
> $ strace kwrite |& grep ENOENT | wc -l
> 1993
> ```
> We reduce these accesses to let QDir keep the list of files inside the
> directory (that was already being generated at some point, it just was being
> discarded).
>
>
> Diffs
> -
>
> src/kicontheme.cpp 0996054
>
> Diff: https://git.reviewboard.kde.org/r/127236/diff/
>
>
> Testing
> ---
>
> Builds, tests still pass, applications start noticeably faster.
>
>
> 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 127334: Fix build with -DBUILD_TEST=TRUE
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127334/#review93396 --- Ship it! Ship It! - Kevin Funk On March 10, 2016, 7:04 p.m., Heiko Becker wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/127334/ > --- > > (Updated March 10, 2016, 7:04 p.m.) > > > Review request for KDE Frameworks, Dario Freddi and Martin Gräßlin. > > > Repository: polkit-qt-1 > > > Description > --- > > Otherwise it fails with "test/test.cpp:49:14: error: 'qVariantValue' > was not declared in this scope". qVariantValue is a obsolete > workaround for MSVC 6, which was released 1998 and thus seems ancient > enough to change this. > > > Diffs > - > > test/test.cpp afb7f64333e1122d86d2692f9cbaa94a08bec800 > > Diff: https://git.reviewboard.kde.org/r/127334/diff/ > > > Testing > --- > > Builds at least. The test doesn't run successfully though (Totals: 6 passed, > 3 failed, 0 skipped, 0 blacklisted). I doubt it did before. There's also no > target to run it. > > > Thanks, > > Heiko Becker > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 127211: Properly use static QMaps
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127211/#review92877 --- Ship it! Ship It! - Kevin Funk On Feb. 28, 2016, 9:52 p.m., Aleix Pol Gonzalez wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/127211/ > --- > > (Updated Feb. 28, 2016, 9:52 p.m.) > > > Review request for Kate and KDE Frameworks. > > > Repository: ktexteditor > > > Description > --- > > Only initialize them once. > Since they're not const, use ::value rather than ::operator[], because the > latter will provide a default-constructed object and add it to the map. > > > Diffs > - > > src/vimode/cmds.cpp 7804af4 > > Diff: https://git.reviewboard.kde.org/r/127211/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 127177: Compilation fixes for MSVC (2013)
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127177/#review92805 --- Fix it, then Ship it! src/kcatalog.cpp (line 46) <https://git.reviewboard.kde.org/r/127177/#comment63232> Remove indentation. Same below. - Kevin Funk On Feb. 25, 2016, 5:16 p.m., Thomas Friedrichsmeier wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/127177/ > --- > > (Updated Feb. 25, 2016, 5:16 p.m.) > > > Review request for KDE Frameworks and kdewin. > > > Repository: ki18n > > > Description > --- > > Fix compilation on Windows with MSVC. > > a) MSVC accepts declspec at global scope, only. > b) QStringLiteral does not work with concatenated literals in MSVC (see > http://blog.qt.io/blog/2014/06/13/qt-weekly-13-qstringliteral/). Since the > occurences seem to be mostly for debugging, going through > QString::fromLatin1() in these cases should not hurt much. > > > Diffs > - > > src/kcatalog.cpp 083443d > src/ktranscript.cpp 72c3755 > > Diff: https://git.reviewboard.kde.org/r/127177/diff/ > > > Testing > --- > > Now builds with MSVC 2013. > > No other compilers and no other platforms tested for this patch. > > > Thanks, > > Thomas Friedrichsmeier > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 127031: Add function to get runtime frameworks version information
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/127031/#review92564 --- src/lib/kcoreaddons.h (line 38) <https://git.reviewboard.kde.org/r/127031/#comment63107> Maybe also add a hint / some code which shows how to use that uint? I.e. point to Qt's QT_VERSION_CHECK documentation or copy/paste it. - Kevin Funk On Feb. 19, 2016, 12:46 p.m., David Edmundson wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/127031/ > --- > > (Updated Feb. 19, 2016, 12:46 p.m.) > > > Review request for KDE Frameworks. > > > Repository: kcoreaddons > > > Description > --- > > Adds a method similar to qVersion() that returns a string of the > frameworks version being run. This differs from the header file which > can only provide the version this app is compiled against. > > The intended usage is in drkonqi system information. > > > Diffs > - > > src/lib/CMakeLists.txt a36eed26a281baf9ef1064dfb9aed3c394c52604 > src/lib/kcoreaddons.h PRE-CREATION > src/lib/kcoreaddons.cpp PRE-CREATION > > Diff: https://git.reviewboard.kde.org/r/127031/diff/ > > > Testing > --- > > See https://git.reviewboard.kde.org/r/127032/ > > > Thanks, > > David Edmundson > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 126969: KRecursiveFilterProxyModel::match: Fix crash
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126969/ --- Review request for KDE Frameworks. Repository: kitemmodels Description --- As seen in GammaRay Diffs - src/krecursivefilterproxymodel.cpp dbb6eba421c0e680fffe43582f210ea3e42e6e7f Diff: https://git.reviewboard.kde.org/r/126969/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 126969: KRecursiveFilterProxyModel::match: Fix crash
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126969/ --- (Updated Feb. 2, 2016, 8:41 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks and Volker Krause. Changes --- Submitted with commit 11055706bca380b75da48b8f5d4c27c53d86372d by Kevin Funk to branch master. Repository: kitemmodels Description --- As seen in GammaRay Diffs - src/krecursivefilterproxymodel.cpp dbb6eba421c0e680fffe43582f210ea3e42e6e7f Diff: https://git.reviewboard.kde.org/r/126969/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 126745: Fix karchive autotests on Windows
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126745/ --- (Updated Jan. 20, 2016, 8:59 a.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks and Patrick Spendrin. Changes --- Submitted with commit 123235fc5f036e42afce90bce7d1c06742679080 by Kevin Funk to branch master. Repository: karchive Description --- Still not perfect (no proper dependencies set), but better: * Moves from CMake time to post-build time of kcompressiondevicetest * Moves the test data to the actual output dir of kcompressiondevicetest Still doesn't regenerate the data in case examples/ changes. Diffs - autotests/CMakeLists.txt 39881f90c614200a6c840b67d14e6c518f8e57e9 Diff: https://git.reviewboard.kde.org/r/126745/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 126745: Fix karchive autotests on Windows
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126745/ --- (Updated Jan. 18, 2016, 4:58 p.m.) Review request for KDE Frameworks and Patrick Spendrin. Repository: karchive Description --- Still not perfect (no proper dependencies set), but better: * Moves from CMake time to post-build time of kcompressiondevicetest * Moves the test data to the actual output dir of kcompressiondevicetest Still doesn't regenerate the data in case examples/ changes. Diffs (updated) - autotests/CMakeLists.txt 39881f90c614200a6c840b67d14e6c518f8e57e9 Diff: https://git.reviewboard.kde.org/r/126745/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 126764: Workaround for leading slash returned by QUrl::toDisplayString(QUrl::PreferLocalFile) untill fixed in Qt
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126764/#review91214 --- I have a fix: https://codereview.qt-project.org/#/c/145963/ Feel free to patch your self-built Qt on Windows for now... :) - Kevin Funk On Jan. 15, 2016, 10:30 p.m., Kåre Särs wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/126764/ > --- > > (Updated Jan. 15, 2016, 10:30 p.m.) > > > Review request for KDE Frameworks. > > > Repository: kio > > > Description > --- > > The KUrlRequester has an extra leading slash when a local url is set. This is > due to strange behaviour of QUrl::toDisplayString(QUrl::PreferLocalFile) on > Windows. There is a bug report for the problem on > https://bugreports.qt.io/browse/QTBUG-41729 but somebody has to do the actual > fixing ;) Untill it is fixed this is a workaround for this place. > > > Diffs > - > > src/widgets/kurlrequester.cpp 06e9ddb > > Diff: https://git.reviewboard.kde.org/r/126764/diff/ > > > Testing > --- > > Kate on windows does not now have a leading slash in the Search in files > KUrlRequester > > > Thanks, > > Kåre Särs > > ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 126745: Fix karchive autotests on Windows
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126745/ --- Review request for KDE Frameworks. Repository: karchive Description --- Still not perfect (no proper dependencies set), but better: * Moves from CMake time to post-build time of kcompressiondevicetest * Moves the test data to the actual output dir of kcompressiondevicetest Still doesn't regenerate the data in case examples/ changes. Diffs - autotests/CMakeLists.txt 39881f90c614200a6c840b67d14e6c518f8e57e9 Diff: https://git.reviewboard.kde.org/r/126745/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 126695: Fix clang warning about implicit copy ctor and explicit operator=.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126695/#review90838 --- Ship it! Ship It! - Kevin Funk On Jan. 10, 2016, 11:03 a.m., David Faure wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/126695/ > --- > > (Updated Jan. 10, 2016, 11:03 a.m.) > > > Review request for KDE Frameworks. > > > Repository: kio > > > Description > --- > > QList calls both, so both are needed, but indeed operator= means the > members can't be const. Let's just remove the const for the members > rather than use const_cast. > > Change-Id: I9e749a448dc69439ba34ceed31fd7f0032f68c3b > > > Diffs > - > > src/core/ksslcertificatemanager_p.h > 0b9a85d6f05b93dbe8e80962401dff5ca73617f8 > > Diff: https://git.reviewboard.kde.org/r/126695/diff/ > > > Testing > --- > > It compiles. > > > 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 126615: (knewstuff) Fix compilation: QStringLiteral and multistring parameter do not mix on Windows
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126615/#review90492 --- Ship it! Ship It! - Kevin Funk On Jan. 3, 2016, 12:51 p.m., Kåre Särs wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/126615/ > --- > > (Updated Jan. 3, 2016, 12:51 p.m.) > > > Review request for KDE Frameworks. > > > Repository: knewstuff > > > Description > --- > > Strings that are separated into multiple parts don't work on Windows > together with QStringLiteral as the first string is interpreted as a > wide (16bit) string, and the second one as a narrow (8bit) string. (Copy > pasted from Patrick Spendrin's RR) > > Replacing QStringLitearl() with QLatin1String() > > > Diffs > - > > src/ui/itemsviewdelegate.cpp 8a891ba > > Diff: https://git.reviewboard.kde.org/r/126615/diff/ > > > Testing > --- > > 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 126616: (ktextwidgets) Fix compilation: QStringLiteral and multistring parameter do not mix on Windows
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126616/#review90491 --- Ship it! Ship It! - Kevin Funk On Jan. 3, 2016, 12:53 p.m., Kåre Särs wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/126616/ > --- > > (Updated Jan. 3, 2016, 12:53 p.m.) > > > Review request for KDE Frameworks. > > > Repository: ktextwidgets > > > Description > --- > > Strings that are separated into multiple parts don't work on Windows > together with QStringLiteral as the first string is interpreted as a > wide (16bit) string, and the second one as a narrow (8bit) string. (Copy > pasted from Patrick Spendrin's RR) > > Replacing QStringLitearl() with QLatin1String() > > > Diffs > - > > src/widgets/krichtextedit.cpp 805fbe3 > > Diff: https://git.reviewboard.kde.org/r/126616/diff/ > > > Testing > --- > > 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 126614: (Ki18n) Fix compilation: QStringLiteral and multistring parameter do not mix on Windows
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126614/#review90493 --- Ship it! Ship It! - Kevin Funk On Jan. 3, 2016, 12:45 p.m., Kåre Särs wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/126614/ > --- > > (Updated Jan. 3, 2016, 12:45 p.m.) > > > Review request for KDE Frameworks. > > > Repository: ki18n > > > Description > --- > > Strings that are separated into multiple parts don't work on Windows > together with QStringLiteral as the first string is interpreted as a > wide (16bit) string, and the second one as a narrow (8bit) string. (Copy > pasted from Patrick Spendrin's RR) > > Joining the strings here. > > > Diffs > - > > src/kuitmarkup.cpp 9831b34 > > Diff: https://git.reviewboard.kde.org/r/126614/diff/ > > > Testing > --- > > 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 126617: (kxmlgui) Fix compilation: QStringLiteral and multistring parameter do not mix on Windows
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126617/#review90490 --- Ship it! Ship It! - Kevin Funk On Jan. 3, 2016, 12:57 p.m., Kåre Särs wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/126617/ > --- > > (Updated Jan. 3, 2016, 12:57 p.m.) > > > Review request for KDE Frameworks. > > > Repository: kxmlgui > > > Description > --- > > Strings that are separated into multiple parts don't work on Windows > together with QStringLiteral as the first string is interpreted as a > wide (16bit) string, and the second one as a narrow (8bit) string. (Copy > pasted from Patrick Spendrin's RR) > > Joining the strings here. > > > Diffs > - > > src/kbugreport.cpp b43f3c4 > > Diff: https://git.reviewboard.kde.org/r/126617/diff/ > > > Testing > --- > > 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 126453: Fix library order
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126453/ --- (Updated Jan. 1, 2016, 11 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks, Heiko Becker and Jeremy Whiting. Changes --- Submitted with commit 38685ad63b08a6c1c4de2c2ecd525188aecd3e94 by Kevin Funk to branch master. Repository: knewstuff Description --- Fixes issues leading to creation of QTBUG-47240 Diffs - src/CMakeLists.txt cc606444e48b0e519551183c022ccecdac0aa62f Diff: https://git.reviewboard.kde.org/r/126453/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 126453: Fix library order
> On Dec. 21, 2015, 4:33 p.m., Hrvoje Senjan wrote: > > A public header here (DownloadWidget) includes QWidget Qt5::Widgets is still in PUBLIC scope - Kevin --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126453/#review89837 --- On Dec. 21, 2015, 4:06 p.m., Kevin Funk wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/126453/ > --- > > (Updated Dec. 21, 2015, 4:06 p.m.) > > > Review request for KDE Frameworks, Heiko Becker and Jeremy Whiting. > > > Repository: knewstuff > > > Description > --- > > Fixes issues leading to creation of QTBUG-47240 > > > Diffs > - > > src/CMakeLists.txt cc606444e48b0e519551183c022ccecdac0aa62f > > Diff: https://git.reviewboard.kde.org/r/126453/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 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/#review89815 --- Ship it! My nitpicks have been resolved, a weak +1 from my side (other reviewers welcome). - Kevin Funk On Dec. 21, 2015, 9:45 a.m., Alex Richardson wrote: > > --- > 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 > - > > 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
Review Request 126453: Fix library order
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/126453/ --- Review request for KDE Frameworks, Heiko Becker and Jeremy Whiting. Repository: knewstuff Description --- Fixes issues leading to creation of QTBUG-47240 Diffs - src/CMakeLists.txt cc606444e48b0e519551183c022ccecdac0aa62f Diff: https://git.reviewboard.kde.org/r/126453/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 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/#review89480 --- Just a few nitpicks. Rest looks good to me. src/lib/kaboutdata.h (line 979) <https://git.reviewboard.kde.org/r/126343/#comment61241> Can't you move that into `KAboutData::Private`? What am I missing? Polluting the public header otherwise. src/lib/plugin/kpluginmetadata.h (line 72) <https://git.reviewboard.kde.org/r/126343/#comment61240> Duplicated documentation. Just `\sa KAboutPerson::fromJSON`? - Kevin Funk On Dec. 14, 2015, 11:45 a.m., Alex Richardson wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/126343/ > --- > > (Updated Dec. 14, 2015, 11: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 > - > > 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 124905: Win: Hide console window for binaries in LIBEXEC
> On Aug. 27, 2015, 9:57 p.m., Patrick Spendrin wrote: > > Wait, Wait, Wait. > > 1) The difference between console apps and GUI apps is that console apps > > actually have stdin, stdout and stderr streams. So using or not using a > > console app actually depends on if you need to have these streams! > > 2) In kde4 (as in standard cmake) the default when you use add_executable > > is a console application, and if you add WIN32 you'll get a GUI > > application. In kf5 it was decided that it makes much more sense to mark > > non-gui applications explicitly, similar to how it is done in qmake (CONFIG > > += CONSOLE); this is done by the set_target_properties or the ecm macro. > > About your problem of console windows: there is a different solution for > > that (it depends on the way the executables are run in QProcess), but I > > would need to look into the KDE4 code which starts kdeinit4.exe. kdeinit4 > > *is* a console app. @Patrick: So what's the proper solution? I'd really like to get this fixed. It's a big show-stopper on Windows. - Kevin --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124905/#review84496 ------- On Aug. 27, 2015, 7:31 a.m., Kevin Funk wrote: > > --- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/124905/ > --- > > (Updated Aug. 27, 2015, 7:31 a.m.) > > > Review request for KDE Frameworks, Alex Merry, David Faure, and Boudewijn > Rempt. > > > Repository: kio > > > Description > --- > > Win: Hide console window for binaries in LIBEXEC > > > Diffs > - > > src/ioslaves/http/CMakeLists.txt 76a8e2800b84c312431cc1996ac81d1ef6fb5cfc > src/ioslaves/http/kcookiejar/CMakeLists.txt > 7b4778d1f67c1ad9f9edcaa4692b39ee6fe3f365 > src/kioexec/CMakeLists.txt 91284a3a61b86770b4d1939da52d256840803608 > src/kioslave/CMakeLists.txt e02febd380b268c596e8ecc3b745b6f50993ab4e > src/kpac/CMakeLists.txt fc5989714480ca49b5bd72e1c7b458b26bd0d9bc > > Diff: https://git.reviewboard.kde.org/r/124905/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 124905: Win: Hide console window for binaries in LIBEXEC
On Aug. 26, 2015, 7:24 a.m., David Faure wrote: It sounds to me like ecm_mark_non_gui_executable doesn't do the right thing then, and should be fixed, instead? These are non gui executables, so from an API point of view, using this function is correct. Would we ever want a console window when running an app on windows? I guess not? So maybe we should do whatever WIN32 does from within that function, if it's doable outside the add_executable call? Kevin Funk wrote: It's rather that `ecm_mark_non_gui_executable` is has a misleading meaning: Marks an executable target as not being a GUI application Well, in that case setting WIN32_EXECUTABLE to FALSE is *correct*, *but* in fact marking an executable as console application on Windows implies the executable showing a console window. I'm not sure if we should just switch the description/behavior of `ecm_mark_non_gui_executable`, or just not use it (as I did). Honestly I think we'd be better off just not using it (it doesn't really increase convenience, in fact my patch just removes code and still fixes things). Documentation of WIN32_EXECUTABLE: WIN32_EXECUTABLE Build an executable with a WinMain entry point on windows. When this property is set to true the executable when linked on Windows will be created with a WinMain() entry point instead of just main(). This makes it a GUI executable instead of a console application. See the CMAKE_MFC_FLAG variable documentation to configure use of MFC for WinMain executables. This property is initialized by the value of the variable CMAKE_WIN32_EXECUTABLE if it is set when a target is created. http://www.cmake.org/cmake/help/v3.0/prop_tgt/WIN32_EXECUTABLE.html#prop_tgt:WIN32_EXECUTABLE David Faure wrote: I think what you're missing is that ecm_mark_non_gui_executable aims at doing the right thing on all platforms, which includes having no .app bundle on OSX. Your patch goes back to unwanted app bundles on OSX, I presume. This is why we should fix ecm_mark_non_gui_executable rather than not use it. Also for a linux developer it's easier to think in terms of mark as non gui than do I need that weird WIN32 thing in there?. But I'm starting to understand something. Applications that are meant to be used on the command line like kioclient, should be console apps on windows i.e. use main() instead of WinMain(), otherwise we can't see any output, is that right? So we don't have two cases (gui and non gui), we have three? * GUI application (user visible) * helper binary (libexec style). Not user visible. Should be WIN32 to avoid a console window. * command line tool (console app, user visible in the console). Should not be WIN32. In that case we should have a different ecm macro for the second item in that list, that's what we're missing, isn't it? On OSX, AFAIU, the last two would be without app bundle. I know about OSX bundles. MACOSX_BUNDLE is FALSE by default. However, I just noticed we have this defined somewhere in ECM (thus MACOSX_BUNDLE is indeed TRUE by default b/c of CMAKE_MACOSX_BUNDLE being ON...) ``` # By default, create 'GUI' executables. This can be reverted on a per-target basis # using ECMMarkNonGuiExecutable # Since CMake 2.8.8 set(CMAKE_WIN32_EXECUTABLE ON) set(CMAKE_MACOSX_BUNDLE ON) ``` Didn't realize until now... I totally agree with your remark; there are three cases. Let's solve them by: * (1) - do nothing * (2) - ecm_mark_helper_executable() (to be done, sets WIN32 to TRUE, MACOSX_BUNDLE to FALSE) * (3) - ecm_mark_nongui_executable() Makes sense? - Kevin --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124905/#review84389 --- On Aug. 27, 2015, 7:31 a.m., Kevin Funk wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124905/ --- (Updated Aug. 27, 2015, 7:31 a.m.) Review request for KDE Frameworks, Alex Merry, David Faure, and Boudewijn Rempt. Repository: kio Description --- Win: Hide console window for binaries in LIBEXEC Diffs - src/ioslaves/http/CMakeLists.txt 76a8e2800b84c312431cc1996ac81d1ef6fb5cfc src/ioslaves/http/kcookiejar/CMakeLists.txt 7b4778d1f67c1ad9f9edcaa4692b39ee6fe3f365 src/kioexec/CMakeLists.txt 91284a3a61b86770b4d1939da52d256840803608 src/kioslave/CMakeLists.txt e02febd380b268c596e8ecc3b745b6f50993ab4e src/kpac/CMakeLists.txt fc5989714480ca49b5bd72e1c7b458b26bd0d9bc Diff: https://git.reviewboard.kde.org/r/124905/diff/ Testing --- Thanks
Re: Paths to internal executables (c.f. libexec) hard-coded
On Wednesday 26 August 2015 13:40:20 Volker Krause wrote:
On Wednesday 26 August 2015 13:15:15 Kevin Funk wrote:
Heya,
This is problem on Windows because the *final* installation location is
not
known at compile-time (obviously software is installed via installers,
users can pick up the installation prefix). Of course this also limits
the relocateability on other platforms, so it's not just a Windows issue
per se.
I've found this, which describes the issue in detail:
https://www.mail-archive.com/kde-frameworks-devel@kde.org/msg01833.html
Consider this code (kio.git):
const QLatin1String libExecDir(CMAKE_INSTALL_FULL_LIBEXECDIR_KF5);
const QString kioslaveExecutable =
QStandardPaths::findExecutable(kioslave, {libExecDir});
Similar code exists in other frameworks.
Problem:
- Hard-coding the install path is a no-go (for sure on Windows)
How can we solve this?
Proposals:
- Generic solution:
- Add new API such as KSomeFittingClass::libexecPaths()
which returns a list of candidates based on the platform
- Then just do:
`QSP::findExecutable(myexe, KSomeFittingClass::libexecPaths())`
- Windows solution only (also less pretty):
- In every place where CMAKE_INSTALL_FULL_LIBEXECDIR_KF5 is used:
- Replace install prefix by QCA::applicationDirPath() via #ifdef
Opinions?
You might find useful building blocks for this in GammaRay. Instead of hard-
coding the absolute libexec dir there we compile in the relative path from
bin to libexec (as that depends on the exact install layout you configure
in CMake), and then combine that with QCoreApp::applicationDirPath() at
runtime. Makes the whole installation relocatable, as long as you don't
move things around within the install prefix itself.
Yep, thanks for this pointer. Didn't even know we had such code in GammaRay.
It's a bit more complex in KDE land because we have multiple levels of library
dependencies, each can be installed into different locations.
Assume on Linux
- KF5 installed into /usr
- KDevelop into $HOME/opt
When invoking `kdevelop` we can't just use
QCA::applicationDirPath()/../lib/libexec, b/c then we might not find
executables installed by KF5 in /usr/lib/libexec.
If we want to support this case, then we'd need to create a
KSomething::libexecPaths() method returning the following search path (in
order):
- Windows:
- Just QCA::applicationDirPath() (we control the install layout anyway)
- Unix:
- a list based on {DY,}LD_LIBRARY_PATH, each suffixed by libexec/
- then : As fallback: the hardcoded install prefix of KF5 as before
(otherwise we might not check /usr/lib/libexec at all, if installed there)
This is somewhat in line what David F. suggested in his initial mail, I think.
If this is not feasible in near future, I'd also be fine just fixing the
Windows-case by placing a few #ifdefs in KF5 code... (In fact, our shiny
KDevelop Windows installer already contains such patches)
regards,
Volker
Cheers
--
Kevin Funk | kf...@kde.org | http://kfunk.org
signature.asc
Description: This is a digitally signed message part.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 124905: Win: Hide console window for binaries in LIBEXEC
On Aug. 26, 2015, 7:24 a.m., David Faure wrote: It sounds to me like ecm_mark_non_gui_executable doesn't do the right thing then, and should be fixed, instead? These are non gui executables, so from an API point of view, using this function is correct. Would we ever want a console window when running an app on windows? I guess not? So maybe we should do whatever WIN32 does from within that function, if it's doable outside the add_executable call? It's rather that `ecm_mark_non_gui_executable` is has a misleading meaning: Marks an executable target as not being a GUI application Well, in that case setting WIN32_EXECUTABLE to FALSE is *correct*, *but* in fact marking an executable as console application on Windows implies the executable showing a console window. I'm not sure if we should just switch the description/behavior of `ecm_mark_non_gui_executable`, or just not use it (as I did). Honestly I think we'd be better off just not using it (it doesn't really increase convenience, in fact my patch just removes code and still fixes things). Documentation of WIN32_EXECUTABLE: WIN32_EXECUTABLE Build an executable with a WinMain entry point on windows. When this property is set to true the executable when linked on Windows will be created with a WinMain() entry point instead of just main(). This makes it a GUI executable instead of a console application. See the CMAKE_MFC_FLAG variable documentation to configure use of MFC for WinMain executables. This property is initialized by the value of the variable CMAKE_WIN32_EXECUTABLE if it is set when a target is created. http://www.cmake.org/cmake/help/v3.0/prop_tgt/WIN32_EXECUTABLE.html#prop_tgt:WIN32_EXECUTABLE - Kevin --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124905/#review84389 --- On Aug. 24, 2015, 1:36 p.m., Kevin Funk wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124905/ --- (Updated Aug. 24, 2015, 1:36 p.m.) Review request for KDE Frameworks, David Faure and Boudewijn Rempt. Repository: kio Description --- Win: Hide console window for binaries in LIBEXEC Diffs - src/ioslaves/http/CMakeLists.txt 76a8e2800b84c312431cc1996ac81d1ef6fb5cfc src/ioslaves/http/kcookiejar/CMakeLists.txt 7b4778d1f67c1ad9f9edcaa4692b39ee6fe3f365 src/kioexec/CMakeLists.txt 91284a3a61b86770b4d1939da52d256840803608 src/kioslave/CMakeLists.txt e02febd380b268c596e8ecc3b745b6f50993ab4e src/kpac/CMakeLists.txt fc5989714480ca49b5bd72e1c7b458b26bd0d9bc Diff: https://git.reviewboard.kde.org/r/124905/diff/ Testing --- Thanks, Kevin Funk ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Paths to internal executables (c.f. libexec) hard-coded
Heya,
This is problem on Windows because the *final* installation location is not
known at compile-time (obviously software is installed via installers, users
can pick up the installation prefix). Of course this also limits the
relocateability on other platforms, so it's not just a Windows issue per se.
I've found this, which describes the issue in detail:
https://www.mail-archive.com/kde-frameworks-devel@kde.org/msg01833.html
Consider this code (kio.git):
const QLatin1String libExecDir(CMAKE_INSTALL_FULL_LIBEXECDIR_KF5);
const QString kioslaveExecutable =
QStandardPaths::findExecutable(kioslave, {libExecDir});
Similar code exists in other frameworks.
Problem:
- Hard-coding the install path is a no-go (for sure on Windows)
How can we solve this?
Proposals:
- Generic solution:
- Add new API such as KSomeFittingClass::libexecPaths()
which returns a list of candidates based on the platform
- Then just do:
`QSP::findExecutable(myexe, KSomeFittingClass::libexecPaths())`
- Windows solution only (also less pretty):
- In every place where CMAKE_INSTALL_FULL_LIBEXECDIR_KF5 is used:
- Replace install prefix by QCA::applicationDirPath() via #ifdef
Opinions?
--
Kevin Funk | kf...@kde.org | http://kfunk.org
signature.asc
Description: This is a digitally signed message part.
___
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
On Aug. 24, 2015, 11:18 p.m., Alex Richardson wrote: src/core/slave.cpp, line 89 https://git.reviewboard.kde.org/r/124904/diff/2/?file=397847#file397847line89 Unrelated but while you're touching this code maybe change this to qEnvironmentVariableIsSet(). More efficient and easier to understand Done, see: commit 140a82d6796cb6ba82a9b173813968594113cb74 Author: Kevin Funk kf...@kde.org Date: Tue Aug 25 09:24:24 2015 +0200 Prefer qEnvironmentVariableIsSet - Kevin --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124904/#review84309 --- On Aug. 25, 2015, 7:10 a.m., Kevin Funk wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124904/ --- (Updated Aug. 25, 2015, 7:10 a.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 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/ --- (Updated Aug. 25, 2015, 7:10 a.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks, David Faure and Boudewijn Rempt. Changes --- Submitted with commit a04683c12300c6b388fad2cfcd8e173d06ca5693 by Kevin Funk to branch master. 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 124905: Win: Hide console window for binaries in LIBEXEC
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124905/#review84357 --- I accidentally pushed this, sorry. Could you review? - Kevin Funk On Aug. 24, 2015, 1:36 p.m., Kevin Funk wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124905/ --- (Updated Aug. 24, 2015, 1:36 p.m.) Review request for KDE Frameworks, David Faure and Boudewijn Rempt. Repository: kio Description --- Win: Hide console window for binaries in LIBEXEC Diffs - src/ioslaves/http/CMakeLists.txt 76a8e2800b84c312431cc1996ac81d1ef6fb5cfc src/ioslaves/http/kcookiejar/CMakeLists.txt 7b4778d1f67c1ad9f9edcaa4692b39ee6fe3f365 src/kioexec/CMakeLists.txt 91284a3a61b86770b4d1939da52d256840803608 src/kioslave/CMakeLists.txt e02febd380b268c596e8ecc3b745b6f50993ab4e src/kpac/CMakeLists.txt fc5989714480ca49b5bd72e1c7b458b26bd0d9bc Diff: https://git.reviewboard.kde.org/r/124905/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 124896: Fix bad behavior / running OOM on Windows
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124896/ --- (Updated Aug. 24, 2015, 12:37 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks. Changes --- Submitted with commit 05167e5bf563f3b9b2465e9da2ecf1e4a86a0d4d by Kevin Funk to branch master. Bugs: 345860 https://bugs.kde.org/show_bug.cgi?id=345860 Repository: kcompletion Description --- Fix bad behavior / running OOM on Windows When testing KDevelop on Windows we sometimes experienced excessive use of memory ( 2 gigs of RAM allocated). This patch appears to fix the issue. Compiler warnings didn't sound promising either: Z:\kderoot\download\git\kcompletion\src\kzoneallocator.cpp(129): warning C4319: '~': zero extending 'unsigned long' to 'quintptr' of greater size Z:\kderoot\download\git\kcompletion\src\kzoneallocator.cpp(206): warning C4319: '~': zero extending 'unsigned long' to 'quintptr' of greater size Z:\kderoot\download\git\kcompletion\src\kzoneallocator.cpp(256): warning C4267: '+=': conversion from 'size_t' to 'unsigned long', possible loss of data BUG: 345860 Diffs - src/kzoneallocator.cpp 2ab9bb021ff09445f291f69ecc3f6facde7eed22 Diff: https://git.reviewboard.kde.org/r/124896/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 124876: KSycoca: check timestamps and run kbuildsycoca if needed. No kded needed anymore.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124876/#review84272 --- More context: ``` [5748] klauncher not running... launching kdeinit [6344] kf5.kinit.klauncher: LAUNCHER_OK [2892] Could not find drkonqi at Z:/kderoot/bin/drkonqi [2892] QObject::connect: No such signal UrlHandler::destroyed(QObject*) [6344] kf5.kinit.klauncher: Z:/kderoot/bin/kbuildsycoca5.exe (pid 1780) up and running. [1780] Could not find drkonqi at Z:/kderoot/bin/drkonqi [1780] kf5.kservice.sycoca: Trying to open ksycoca from C:/Users/kfunk/AppData/Local/cache/ksycoca5 [1780] checking file timestamps [1780] timestamps check ok [1780] Emitting notifyDatabaseChanged () [6344] kf5.kinit.klauncher: kbuildsycoca5 running... [6344] [6344] kf5.kinit.klauncher: process finished exitcode= 0 exitStatus= 0 [2892] kf5.kservice.sycoca: Trying to open ksycoca from C:/Users/kfunk/AppData/Local/cache/ksycoca5 [2892] kf5.kservice.sycoca: checking file timestamps [2892] kf5.kservice.sycoca: timestamp changed: Z:/kderoot/bin/../share/kservicetypes5 [6100] Could not find drkonqi at Z:/kderoot/bin/drkonqi [6100] kf5.kservice.sycoca: Trying to open ksycoca from C:/Users/kfunk/AppData/Local/cache/ksycoca5 [6100] Reusing existing ksycoca [6100] Recreating ksycoca file (C:/Users/kfunk/AppData/Local/cache/ksycoca5, version 302) [6344] kf5.kinit.klauncher: KLauncher: launching new slave Z:/kderoot/bin/kioslave with protocol= file args= (Z:/kderoot/lib/plugins/kf5/kio/file.dll, file, tcp://127.0.0.1:56480, tcp://127.0.0.1:56477) [6344] kf5.kinit.klauncher: Z:/kderoot/bin/kioslave (pid 4568) up and running. [4568] trying to load Z:/kderoot/lib/plugins/kf5/kio/file.dll from Z:/kderoot/lib/plugins/kf5/kio/file.dll ``` - Kevin Funk On Aug. 22, 2015, 10:45 a.m., David Faure wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124876/ --- (Updated Aug. 22, 2015, 10:45 a.m.) Review request for KDE Frameworks, Boudewijn Rempt and Vishesh Handa. Repository: kservice Description --- If 1.5s have passed since the last time we checked, we compare the mtime of the directories (12 dirs on my system) with the timestamp stored in the ksycoca database (which indicates that all changes prior to that time are in the DB). Note that we only check the mtime of dirs, not files. Therefore manually editing an installed .desktop file will require touching the dir, or running kbuildsycoca5. Diffs - src/sycoca/ksycoca.cpp b7f7abc88db90d784851d91036069e0647fdcbf6 src/sycoca/ksycoca_p.h 9f403d6f4be2b406f4985f668176cfa56a5898ea Diff: https://git.reviewboard.kde.org/r/124876/diff/ Testing --- the kservice unittests still pass 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 124902: Delay starting kglobalaccel5 till it's needed
On Aug. 24, 2015, 11:52 a.m., Kevin Funk wrote:
src/kglobalaccel.cpp, line 97
https://git.reviewboard.kde.org/r/124902/diff/1/?file=397839#file397839line97
Indeed. Just move into iface()?
Martin Gräßlin wrote:
it's also used for setting up the QDBusServiceWatcher in the
KGlobalAccelPrivate ctor. So not possible to move it.
Ah, didn't notice.
This is usually solved by introducing a free function in anonymous namespace.
I.e. `namespace { QString serviceName() { return QStringLiteral(...); } }`.
(Saves the member + better for optimization)
- Kevin
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124902/#review84267
---
On Aug. 24, 2015, 12:13 p.m., Martin Gräßlin wrote:
---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/124902/
---
(Updated Aug. 24, 2015, 12:13 p.m.)
Review request for KDE Frameworks and Kevin Funk.
Repository: kglobalaccel
Description
---
There are usages of KGlobalAccel in e.g. KActionCollection of xmlgui
framework which do not require a running kglobalaccel5. But due to the
way KGlobalAccel inits, it always started the runtime part.
This change ensures that kglobalaccel5 is only started once it's actually
needed, by e.g. registering a global shortcut. If an application does
not use global shortcuts the API should no longer trigger a start of the
runtime part.
Diffs
-
src/kglobalaccel.cpp 3508c870c70a21e76263b7574c20b408cc09a837
src/kglobalaccel_p.h a84f4038dac650ddb0fc1828e67e0851153b4456
Diff: https://git.reviewboard.kde.org/r/124902/diff/
Testing
---
export $(dbus-launch)
qdbusviewer
kate
- kglobalaccel5 gets started
with the change:
kglobalaccel5 no longer gets started, when starting an application using
global shortcuts, it still gets started
Thanks,
Martin Gräßlin
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
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/ --- 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 124905: Win: Hide console window for binaries in LIBEXEC
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124905/ --- (Updated Aug. 24, 2015, 1:36 p.m.) Review request for KDE Frameworks, David Faure and Boudewijn Rempt. Changes --- Now we're talking... Summary (updated) - Win: Hide console window for binaries in LIBEXEC Repository: kio Description (updated) --- Win: Hide console window for binaries in LIBEXEC Diffs (updated) - src/ioslaves/http/CMakeLists.txt 76a8e2800b84c312431cc1996ac81d1ef6fb5cfc src/ioslaves/http/kcookiejar/CMakeLists.txt 7b4778d1f67c1ad9f9edcaa4692b39ee6fe3f365 src/kioexec/CMakeLists.txt 91284a3a61b86770b4d1939da52d256840803608 src/kioslave/CMakeLists.txt e02febd380b268c596e8ecc3b745b6f50993ab4e src/kpac/CMakeLists.txt fc5989714480ca49b5bd72e1c7b458b26bd0d9bc Diff: https://git.reviewboard.kde.org/r/124905/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 124876: KSycoca: check timestamps and run kbuildsycoca if needed. No kded needed anymore.
On Aug. 24, 2015, 12:15 p.m., Kevin Funk wrote: More context: ``` [5748] klauncher not running... launching kdeinit [6344] kf5.kinit.klauncher: LAUNCHER_OK [2892] Could not find drkonqi at Z:/kderoot/bin/drkonqi [2892] QObject::connect: No such signal UrlHandler::destroyed(QObject*) [6344] kf5.kinit.klauncher: Z:/kderoot/bin/kbuildsycoca5.exe (pid 1780) up and running. [1780] Could not find drkonqi at Z:/kderoot/bin/drkonqi [1780] kf5.kservice.sycoca: Trying to open ksycoca from C:/Users/kfunk/AppData/Local/cache/ksycoca5 [1780] checking file timestamps [1780] timestamps check ok [1780] Emitting notifyDatabaseChanged () [6344] kf5.kinit.klauncher: kbuildsycoca5 running... [6344] [6344] kf5.kinit.klauncher: process finished exitcode= 0 exitStatus= 0 [2892] kf5.kservice.sycoca: Trying to open ksycoca from C:/Users/kfunk/AppData/Local/cache/ksycoca5 [2892] kf5.kservice.sycoca: checking file timestamps [2892] kf5.kservice.sycoca: timestamp changed: Z:/kderoot/bin/../share/kservicetypes5 [6100] Could not find drkonqi at Z:/kderoot/bin/drkonqi [6100] kf5.kservice.sycoca: Trying to open ksycoca from C:/Users/kfunk/AppData/Local/cache/ksycoca5 [6100] Reusing existing ksycoca [6100] Recreating ksycoca file (C:/Users/kfunk/AppData/Local/cache/ksycoca5, version 302) [6344] kf5.kinit.klauncher: KLauncher: launching new slave Z:/kderoot/bin/kioslave with protocol= file args= (Z:/kderoot/lib/plugins/kf5/kio/file.dll, file, tcp://127.0.0.1:56480, tcp://127.0.0.1:56477) [6344] kf5.kinit.klauncher: Z:/kderoot/bin/kioslave (pid 4568) up and running. [4568] trying to load Z:/kderoot/lib/plugins/kf5/kio/file.dll from Z:/kderoot/lib/plugins/kf5/kio/file.dll ``` Oh, and I just noticed KDE_FORK_SLAVES=1 doesn't really help on Windows. That whole feature is wrapped with a big fat `#ifdef Q_OS_UNIX`... - Kevin --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124876/#review84272 --- On Aug. 22, 2015, 10:45 a.m., David Faure wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124876/ --- (Updated Aug. 22, 2015, 10:45 a.m.) Review request for KDE Frameworks, Boudewijn Rempt and Vishesh Handa. Repository: kservice Description --- If 1.5s have passed since the last time we checked, we compare the mtime of the directories (12 dirs on my system) with the timestamp stored in the ksycoca database (which indicates that all changes prior to that time are in the DB). Note that we only check the mtime of dirs, not files. Therefore manually editing an installed .desktop file will require touching the dir, or running kbuildsycoca5. Diffs - src/sycoca/ksycoca.cpp b7f7abc88db90d784851d91036069e0647fdcbf6 src/sycoca/ksycoca_p.h 9f403d6f4be2b406f4985f668176cfa56a5898ea Diff: https://git.reviewboard.kde.org/r/124876/diff/ Testing --- the kservice unittests still pass 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 124876: KSycoca: check timestamps and run kbuildsycoca if needed. No kded needed anymore.
On Aug. 24, 2015, 12:07 p.m., Kevin Funk wrote: Still runs kbuildsycoca5 for me(?) ``` [3796] kf5.kservice.sycoca: Trying to open ksycoca from C:/Users/kfunk/AppData/Local/cache/ksycoca5 [3796] checking file timestamps [3796] timestamps check ok [3796] Emitting notifyDatabaseChanged () [4824] kf5.kinit.klauncher: kbuildsycoca5 running... [4824] [4824] kf5.kinit.klauncher: process finished exitcode= 0 exitStatus= 0 [5668] kf5.kservice.sycoca: Trying to open ksycoca from C:/Users/kfunk/AppData/Local/cache/ksycoca5 ``` ^ Check says ok but then still ... runs kbuildsycoca5? - Kevin --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124876/#review84269 --- On Aug. 22, 2015, 10:45 a.m., David Faure wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124876/ --- (Updated Aug. 22, 2015, 10:45 a.m.) Review request for KDE Frameworks, Boudewijn Rempt and Vishesh Handa. Repository: kservice Description --- If 1.5s have passed since the last time we checked, we compare the mtime of the directories (12 dirs on my system) with the timestamp stored in the ksycoca database (which indicates that all changes prior to that time are in the DB). Note that we only check the mtime of dirs, not files. Therefore manually editing an installed .desktop file will require touching the dir, or running kbuildsycoca5. Diffs - src/sycoca/ksycoca.cpp b7f7abc88db90d784851d91036069e0647fdcbf6 src/sycoca/ksycoca_p.h 9f403d6f4be2b406f4985f668176cfa56a5898ea Diff: https://git.reviewboard.kde.org/r/124876/diff/ Testing --- the kservice unittests still pass Thanks, David Faure ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 124905: Make KDE_FORK_SLAVES work under Windows
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124905/ --- Review request for KDE Frameworks, David Faure and Boudewijn Rempt. Repository: kio Description --- Win: Hide console window for binaries in LIBEXEC Diffs - src/core/slave.cpp 62455c1db471aa3fc1273b1bb398fbc183ddca1f src/ioslaves/http/CMakeLists.txt 76a8e2800b84c312431cc1996ac81d1ef6fb5cfc src/ioslaves/http/kcookiejar/CMakeLists.txt 7b4778d1f67c1ad9f9edcaa4692b39ee6fe3f365 src/kioexec/CMakeLists.txt 91284a3a61b86770b4d1939da52d256840803608 src/kioslave/CMakeLists.txt e02febd380b268c596e8ecc3b745b6f50993ab4e src/kpac/CMakeLists.txt fc5989714480ca49b5bd72e1c7b458b26bd0d9bc Diff: https://git.reviewboard.kde.org/r/124905/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 124876: KSycoca: check timestamps and run kbuildsycoca if needed. No kded needed anymore.
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124876/#review84269 --- Still runs kbuildsycoca5 for me(?) ``` [3796] kf5.kservice.sycoca: Trying to open ksycoca from C:/Users/kfunk/AppData/Local/cache/ksycoca5 [3796] checking file timestamps [3796] timestamps check ok [3796] Emitting notifyDatabaseChanged () [4824] kf5.kinit.klauncher: kbuildsycoca5 running... [4824] [4824] kf5.kinit.klauncher: process finished exitcode= 0 exitStatus= 0 [5668] kf5.kservice.sycoca: Trying to open ksycoca from C:/Users/kfunk/AppData/Local/cache/ksycoca5 ``` ^ Check says ok but then still - Kevin Funk On Aug. 22, 2015, 10:45 a.m., David Faure wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124876/ --- (Updated Aug. 22, 2015, 10:45 a.m.) Review request for KDE Frameworks, Boudewijn Rempt and Vishesh Handa. Repository: kservice Description --- If 1.5s have passed since the last time we checked, we compare the mtime of the directories (12 dirs on my system) with the timestamp stored in the ksycoca database (which indicates that all changes prior to that time are in the DB). Note that we only check the mtime of dirs, not files. Therefore manually editing an installed .desktop file will require touching the dir, or running kbuildsycoca5. Diffs - src/sycoca/ksycoca.cpp b7f7abc88db90d784851d91036069e0647fdcbf6 src/sycoca/ksycoca_p.h 9f403d6f4be2b406f4985f668176cfa56a5898ea Diff: https://git.reviewboard.kde.org/r/124876/diff/ Testing --- the kservice unittests still pass 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 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/ --- (Updated Aug. 24, 2015, 1:11 p.m.) Review request for KDE Frameworks, David Faure and Boudewijn Rempt. Changes --- Simplified patch Repository: kio Description --- Make KDE_FORK_SLAVES work under Windows Diffs (updated) - 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 124905: Make KDE_FORK_SLAVES work under Windows
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124905/ --- (Updated Aug. 24, 2015, 1:35 p.m.) Review request for KDE Frameworks, David Faure and Boudewijn Rempt. Changes --- Man, I really, really, hate rbt... Repository: kio Description (updated) --- Make KDE_FORK_SLAVES work under Windows Diffs (updated) - src/core/slave.cpp 62455c1db471aa3fc1273b1bb398fbc183ddca1f Diff: https://git.reviewboard.kde.org/r/124905/diff/ Testing --- Thanks, Kevin Funk ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 124896: Fix bad behavior / running OOM on Windows
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124896/ --- Review request for KDE Frameworks. Repository: kcompletion Description --- Fix bad behavior / running OOM on Windows When testing KDevelop on Windows we sometimes experienced excessive use of memory ( 2 gigs of RAM allocated). This patch appears to fix the issue. Compiler warnings didn't sound promising either: Z:\kderoot\download\git\kcompletion\src\kzoneallocator.cpp(129): warning C4319: '~': zero extending 'unsigned long' to 'quintptr' of greater size Z:\kderoot\download\git\kcompletion\src\kzoneallocator.cpp(206): warning C4319: '~': zero extending 'unsigned long' to 'quintptr' of greater size Z:\kderoot\download\git\kcompletion\src\kzoneallocator.cpp(256): warning C4267: '+=': conversion from 'size_t' to 'unsigned long', possible loss of data BUG: 345860 Diffs - src/kzoneallocator.cpp 2ab9bb021ff09445f291f69ecc3f6facde7eed22 Diff: https://git.reviewboard.kde.org/r/124896/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 124896: Fix bad behavior / running OOM on Windows
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124896/ --- (Updated Aug. 24, 2015, 8:27 a.m.) Review request for KDE Frameworks. Bugs: 345860 https://bugs.kde.org/show_bug.cgi?id=345860 Repository: kcompletion Description --- Fix bad behavior / running OOM on Windows When testing KDevelop on Windows we sometimes experienced excessive use of memory ( 2 gigs of RAM allocated). This patch appears to fix the issue. Compiler warnings didn't sound promising either: Z:\kderoot\download\git\kcompletion\src\kzoneallocator.cpp(129): warning C4319: '~': zero extending 'unsigned long' to 'quintptr' of greater size Z:\kderoot\download\git\kcompletion\src\kzoneallocator.cpp(206): warning C4319: '~': zero extending 'unsigned long' to 'quintptr' of greater size Z:\kderoot\download\git\kcompletion\src\kzoneallocator.cpp(256): warning C4267: '+=': conversion from 'size_t' to 'unsigned long', possible loss of data BUG: 345860 Diffs - src/kzoneallocator.cpp 2ab9bb021ff09445f291f69ecc3f6facde7eed22 Diff: https://git.reviewboard.kde.org/r/124896/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: ksycoca improvements (Re: assert in kservicetypefactory.cpp)
On Saturday 22 August 2015 11:34:38 David Faure wrote: On Wednesday 19 August 2015 09:08:35 Boudewijn Rempt wrote: A windows or osx application for the masses must: * not have any pre or post install procedures * not start any process except the application itself * avoid ipc as much as possible * find all resources and plugins relative to itself * be portable (moving it around should not be a problem) OK, I understand. I am currently fixing a first issue, ksycoca usage (e.g. KServiceTypeTrader) starting kded for watching files. It will instead compare timestamps itself. This also means, no post install procedure ever, because the app will be able to realize it needs to rebuild the cache (kbuildsycoca5) all by itself, reliably. Hopefully it will also fix the bug you're seeing (which Milian experienced too, since), although I'm still a bit puzzled by what's happening there (one could try going back to one commit before ddd57bccdf, to check if it introduced the issue?). Another thing that could be done about this BTW would be to port all PreviewJob plugins to JSon (and ensure they are all in the same plugins subdir) and use KPluginLoader to load them - no trader query needed anymore. Is anyone interested in doing that? Meanwhile I'm working on ksycoca because we need it anyway for application desktop files, even if all plugins move to json. A second step that could be done to avoid external processes would be to move the kbuildsycoca code into the kservice library itself, i.e. any app looking up something might first recreate the cache itself. That code already uses QSaveFile so two apps wouldn't be able to overwrite each other, but I suppose we still don't want 10 apps noticing at the same time that they need to rebuild the cache and doing it all in parallel. So I would use a QLockFile around that code. This of course increases the library size by merging another 3075 lines of codes into it, which is why it was never done before, but if this is wanted for Windows/OSX and nobody has a strong objection against this, I can do it. Disclaimer: I'm streamlining KDevelop on Windows atm. I'd also be interested in a solution that doesn't require me to have several processes running alongside of kdevelop.exe. As Boud indicated, this is indeed a no-go on Windows. This leaves a bigger issue though: KIO uses separate processes, the kioslaves. I'm not offering to change that right now. Is there really no way to make that work seemlessly on Windows? At least they don't pop up a black window like kbuildsycoca, right? :-) You mention daemons stick around after you quit the app. That is certainly true of kdeinit and kded, but kioslaves exit after being idle for 3 minutes. You want KDE_FORK_SLAVES=1 for the one-app-on-Windows use case, btw. I *think* they even exit when the app closes, but that has to be checked and possibly fixed. Right now whenever I start kdevelop I get the following daemons up and running on Windows: - dbus-daemon - kded5 - kglobalaccel5 - kioslave - klauncher For the one-app use-case: Which ones do I need? How to stop them from being started? (Sorry, but I never really looked into that matter either, so any help welcome here). Given that KIO slaves can be invoked via fork, none of them (besides dbus-daemon for IPC) should be required, no? Didn't know about KDE_FORK_SLAVES yet, thanks! Anyhow - don't move away from KF5, let's work together :-) -- Kevin Funk | kf...@kde.org | http://kfunk.org signature.asc Description: This is a digitally signed message part. ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: ksycoca improvements (Re: assert in kservicetypefactory.cpp)
On Monday 24 August 2015 10:00:43 Martin Gräßlin wrote: Am 2015-08-24 01:03, schrieb Kevin Funk: Right now whenever I start kdevelop I get the following daemons up and running on Windows: - dbus-daemon - kded5 - kglobalaccel5 - kioslave - klauncher KGlobalAccel should only be started if there is something trying to interact with KGlobalAccel (it's dbus activated). A git grep in kdevelop doesn't show it to be used. So I assume it might be a kdevelop-plugin or maybe a kded module? In general I think there shouldn't be a reason for kglobalaccel to be started in the case of kdevelop. That certainly sounds wrong. I think that kglobalaccel5 is invoked as soon as KXmlGui gets involved (which KDevelop makes use of). Code in kxmlgui.git somewhat agrees: There are references to KGlobalAccel::self() in, which will initialize KGlobalAccel which in turn starts the kglobalaccel5 daemon (right?). Looks like that is only being done if *global* shortcuts are involved, but I'm not sure whether I'm free of global shortcuts... Cheers Martin -- Kevin Funk | kf...@kde.org | http://kfunk.org signature.asc Description: This is a digitally signed message part. ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 124902: Delay starting kglobalaccel5 till it's needed
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124902/#review84266 --- Can confirm. With that patch kglobalaccel5 is no longer started using KDevelop on Windows. Awesome! Thanks for this. - Kevin Funk On Aug. 24, 2015, 11:35 a.m., Martin Gräßlin wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124902/ --- (Updated Aug. 24, 2015, 11:35 a.m.) Review request for KDE Frameworks and Kevin Funk. Repository: kglobalaccel Description --- There are usages of KGlobalAccel in e.g. KActionCollection of xmlgui framework which do not require a running kglobalaccel5. But due to the way KGlobalAccel inits, it always started the runtime part. This change ensures that kglobalaccel5 is only started once it's actually needed, by e.g. registering a global shortcut. If an application does not use global shortcuts the API should no longer trigger a start of the runtime part. Diffs - src/kglobalaccel_p.h a84f4038dac650ddb0fc1828e67e0851153b4456 src/kglobalaccel.cpp 3508c870c70a21e76263b7574c20b408cc09a837 Diff: https://git.reviewboard.kde.org/r/124902/diff/ Testing --- export $(dbus-launch) qdbusviewer kate - kglobalaccel5 gets started with the change: kglobalaccel5 no longer gets started, when starting an application using global shortcuts, it still gets started 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 124879: Optimization readEntryGui
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124879/ --- (Updated Aug. 22, 2015, 5:11 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks and Matthew Dawson. Changes --- Submitted with commit a21e85d97f809870de94c84d2ca767260477f988 by Kevin Funk to branch master. Repository: kconfig Description --- Don't attempt to build error string in any case Diffs - src/gui/kconfiggroupgui.cpp e2eb950fb93d40abec0988c9af58c1ae14cb2011 Diff: https://git.reviewboard.kde.org/r/124879/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 124879: Optimization readEntryGui
On Aug. 22, 2015, 5:05 p.m., Albert Astals Cid wrote: While you're at it move formatError down to the only place it is used? Done - Kevin --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124879/#review84189 --- On Aug. 22, 2015, 5:11 p.m., Kevin Funk wrote: --- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124879/ --- (Updated Aug. 22, 2015, 5:11 p.m.) Review request for KDE Frameworks and Matthew Dawson. Repository: kconfig Description --- Don't attempt to build error string in any case Diffs - src/gui/kconfiggroupgui.cpp e2eb950fb93d40abec0988c9af58c1ae14cb2011 Diff: https://git.reviewboard.kde.org/r/124879/diff/ Testing --- Thanks, Kevin Funk ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Review Request 124879: Optimization readEntryGui
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124879/ --- Review request for KDE Frameworks and Matthew Dawson. Repository: kconfig Description --- Don't attempt to build error string in any case Diffs - src/gui/kconfiggroupgui.cpp e2eb950fb93d40abec0988c9af58c1ae14cb2011 Diff: https://git.reviewboard.kde.org/r/124879/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: Minimum supported version of MSVC for frameworks
On Sunday 16 August 2015 11:19:39 Alex Merry wrote:
On 2015-08-13 09:57, Kai Uwe Broulik wrote:
Hi,
My experience with MSVC 2013 was that you even need at least Update 4
for initializer lists to work properly, which was released quite
recently iirc.
And even then, initializer lists don't work in all circumstances
(specifically in member initialization like
struct foo {
int bar[3] = {1, 2, 3};
};
).
Actually, if you do use MSVC 2013, always make sure you install Update
4, otherwise the compiler's really quite broken (even generating double
destructor calls in some circumstances). I don't think it's unreasonable
to refuse to support MSVC 2013 without update 4.
Yep, I know.
So can we come to an agreement we would be better off raising the required
version to MSVC2013? I feel like people don't really want to revert the C++11
portions of code they've written until now anyway...
Cheers,
Kevin
Alex
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
--
Kevin Funk | kf...@kde.org | http://kfunk.org
signature.asc
Description: This is a digitally signed message part.
___
Kde-frameworks-devel mailing list
Kde-frameworks-devel@kde.org
https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Minimum supported version of MSVC for frameworks
Heya, I'm writing this to inform you that KF5 is not compilable under MSVC2012 as-is right now. This is the minimum required version as of [1]. Biggest issues are initializer-lists and non-static data member initialization, default + deleted functions (example patch against kdecoration here: [2]). My question: - Do we even want to hold the requirement on MSVC2012? - People want to use new features from C++11, and they do it - Patches like [2] feel like going one step back again - To me, it would make more sense to just lift our req. to MSVC2013 (with much better (despite still broken) C++11 support anyway) Opinions? If you want, I can make sure KF5 builds on MSVC2013, at least, in irregular intervals. Cheers [1] https://community.kde.org/Frameworks/Policies#Frameworks_compiler_requirements_and_C.2B.2B11 [2] https://paste.kde.org/pspfos96w -- Kevin Funk | kf...@kde.org | http://kfunk.org signature.asc Description: This is a digitally signed message part. ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
Re: Review Request 124696: Fix (worrying) MSVC warning
--- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124696/ --- (Updated Aug. 11, 2015, 1:22 p.m.) Status -- This change has been marked as submitted. Review request for KDE Frameworks and Mirko Boehm. Changes --- Submitted with commit ea52fb68bad35c91ed2cb39dc8cd0aeeea2f4f48 by Kevin Funk to branch master. Repository: threadweaver Description --- Warning: Z:\kderoot\download\git\threadweaver\src\iddecorator.cpp(196): warning C4312: 'r einterpret_cast': conversion from 'const int' to 'ThreadWeaver::IdDecorator::Pri vate2 *' of greater size Diffs - src/iddecorator.cpp 5bf6d002eb2671a02f330cd3022e0692a0343fe4 Diff: https://git.reviewboard.kde.org/r/124696/diff/ Testing --- Thanks, Kevin Funk ___ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
