[Differential] [Commented On] D3987: Use nullptr in all Frameworks (just diff in KIO shown here)

[kfunk (Kevin Funk)]

kfunk added inline comments.

INLINE COMMENTS

> graesslin wrote in job.h:50
> Question: is this change API and ABI stable?

Definitely ABI stable, since default arguments are not part of the function 
signature.

I'm not aware this could break API compat either (well, only in case the 
compiler consuming this code is not C++11-compliant)...

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D3987

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: kfunk, #frameworks
Cc: graesslin

[Differential] [Updated] D3987: Use nullptr in all Frameworks (just diff in KIO shown here)

[kfunk (Kevin Funk)]

kfunk updated the summary for this revision.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D3987

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: kfunk, #frameworks

[Differential] [Request, 2,027 lines] D3987: Use nullptr in all Frameworks (just diff in KIO shown here)

[kfunk (Kevin Funk)]

kfunk created this revision.
kfunk added a reviewer: Frameworks.
kfunk set the repository for this revision to R241 KIO.
Restricted Application added a project: Frameworks.

REVISION SUMMARY
  The full patch (all Frameworks ported to using nullptr instead of null 
literals) changes around 9000 lines in total:
  
  This is what I use locally to keep track of my changes:
  
% kde-frameworks-list.sh  | xargs -n1 -I% sh -c "(cd %; 
git-difflinesonly.sh)" | head
-Code39Barcode::Code39Barcode() : AbstractBarcode(), d(0){
+Code39Barcode::Code39Barcode() : AbstractBarcode(), d(nullptr){
-Code93Barcode::Code93Barcode() : AbstractBarcode(), d(0){
+Code93Barcode::Code93Barcode() : AbstractBarcode(), d(nullptr){
-DataMatrixBarcode::DataMatrixBarcode() : d(0) {
+DataMatrixBarcode::DataMatrixBarcode() : d(nullptr) {
-QRCodeBarcode::QRCodeBarcode() : AbstractBarcode(), d(0){
+QRCodeBarcode::QRCodeBarcode() : AbstractBarcode(), d(nullptr){
-BarcodeExampleWidget(Prison::AbstractBarcode* barcode, QWidget* 
parent=0);
+BarcodeExampleWidget(Prison::AbstractBarcode* barcode, QWidget* 
parent=nullptr);


% kde-frameworks-list.sh  | xargs -n1 -I% sh -c "(cd %; 
git-difflinesonly.sh)" | wc -l
18592
  
  
  
  > ~9000 lines changed.
  ==
  
  This change affects *all files*. Not just headers.
  
  There are more options to limit  the number of changes:
  
  - Less changes: Just change headers (.h files) -- easy
  - Even less changes: Just change public headers -- slightly more difficult 
for me to figure out *what* is public from a scripting POV
  
  If you think we should limit our changes, please speak up. I wouldn't 
recommend it though. Let's move forward instead.
  
  My plan was to push this after the next KF5 release.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D3987

AFFECTED FILES
  autotests/clipboardupdatertest.cpp
  autotests/deletejobtest.cpp
  autotests/dropjobtest.cpp
  autotests/fileundomanagertest.cpp
  autotests/http/httpauthenticationtest.cpp
  autotests/http_jobtest.cpp
  autotests/httpserver_p.h
  autotests/jobguitest.cpp
  autotests/jobremotetest.cpp
  autotests/jobtest.cpp
  autotests/kcookiejar/kcookiejartest.cpp
  autotests/kdirlistertest.cpp
  autotests/kdirmodeltest.cpp
  autotests/kfilecopytomenutest.cpp
  autotests/kfilewidgettest.cpp
  autotests/klocalsockettest.cpp
  autotests/knewfilemenutest.cpp
  autotests/krununittest.cpp
  autotests/ktcpsockettest.cpp
  autotests/kurifiltersearchprovideractionstest.cpp
  autotests/kurifiltertest.cpp
  autotests/kurlcompletiontest.cpp
  autotests/kurlnavigatortest.cpp
  autotests/listdirtest.cpp
  autotests/mkpathjobtest.cpp
  autotests/pastetest.cpp
  autotests/threadtest.cpp
  src/core/authinfo.cpp
  src/core/chmodjob.cpp
  src/core/connection.cpp
  src/core/connection_p.h
  src/core/connectionbackend.cpp
  src/core/connectionbackend_p.h
  src/core/connectionserver.cpp
  src/core/connectionserver.h
  src/core/copyjob.cpp
  src/core/dataprotocol.cpp
  src/core/deletejob.cpp
  src/core/filecopyjob.cpp
  src/core/forwardingslavebase.cpp
  src/core/hostinfo.cpp
  src/core/job.cpp
  src/core/job.h
  src/core/job_base.h
  src/core/job_error.cpp
  src/core/job_p.h
  src/core/jobtracker.cpp
  src/core/jobuidelegateextension.cpp
  src/core/jobuidelegatefactory.cpp
  src/core/kacl.cpp
  src/core/kcoredirlister.cpp
  src/core/kcoredirlister.h
  src/core/kcoredirlister_p.h
  src/core/kdirnotify.h
  src/core/kfileitem.cpp
  src/core/kfileitem.h
  src/core/klocalsocket.cpp
  src/core/klocalsocket.h
  src/core/klocalsocket_unix.cpp
  src/core/kmountpoint.cpp
  src/core/kprotocolmanager.cpp
  src/core/krecentdocument.cpp
  src/core/kremoteencoding.cpp
  src/core/kremoteencoding.h
  src/core/ksambashare.cpp
  src/core/ksslcertificatemanager.cpp
  src/core/kssld_interface.h
  src/core/ktcpsocket.h
  src/core/scheduler.cpp
  src/core/scheduler_p.h
  src/core/simplejob.cpp
  src/core/slave.cpp
  src/core/slave.h
  src/core/slavebase.cpp
  src/core/slavebase.h
  src/core/slaveconfig.cpp
  src/core/slaveinterface.h
  src/core/slaveinterface_p.h
  src/core/storedtransferjob.cpp
  src/core/tcpslavebase.h
  src/core/transferjob.cpp
  src/core/usernotificationhandler.cpp
  src/core/usernotificationhandler_p.h
  src/filewidgets/kdiroperator.cpp
  src/filewidgets/kdiroperator.h
  src/filewidgets/kdiroperatordetailview_p.h
  src/filewidgets/kdirsortfilterproxymodel.h
  src/filewidgets/kencodingfiledialog.h
  src/filewidgets/kfilecopytomenu.cpp
  src/filewidgets/kfilefiltercombo.h
  src/filewidgets/kfilemetapreview.cpp
  src/filewidgets/kfileplaceeditdialog.cpp
  src/filewidgets/kfileplaceeditdialog.h
  src/filewidgets/kfileplacesitem.cpp
  src/filewidgets/kfileplacesitem_p.h
  src/filewidgets/kfileplacesmodel.cpp
  src/filewidgets/kfileplacesmodel.h
  src/filewidgets/kfileplacesview.cpp
  src/filewidgets/kfileplacesview.h
  src/filewidgets/kfileplacesview_p.h
  

[Differential] [Updated] D3987: Use nullptr in all Frameworks (just diff in KIO shown here)

[kfunk (Kevin Funk)]

kfunk updated the summary for this revision.

REPOSITORY
  R241 KIO

REVISION DETAIL
  https://phabricator.kde.org/D3987

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: kfunk, #frameworks

[Differential] [Accepted] D3977: Fix memleak in KDynamicJobTracker, KWidgetJobTracker needs QApplication

[kfunk (Kevin Funk)]

kfunk accepted this revision.
kfunk added a reviewer: kfunk.
kfunk added a comment.
This revision is now accepted and ready to land.


  Rest LGTM, but let's wait for another review

INLINE COMMENTS

> kdynamicjobtrackernowidgetstest.cpp:34
> +public:
> +virtual void start() { QTimer::singleShot(testJobRunningTime, this, 
> ::doEmit); }
> +

`Q_DECL_OVERRIDE`

> kdynamicjobtracker.cpp:99
> +} else {
> +trackers.widgetTracker = 0;
>  }

Here & below: Use `nullptr`?

REPOSITORY
  R241 KIO

BRANCH
  fixKDynamicJobTracker

REVISION DETAIL
  https://phabricator.kde.org/D3977

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: kossebau, #frameworks, kfunk
Cc: kfunk

[Differential] [Commented On] D3850: Pass -fno-operator-names when supported

[kfunk (Kevin Funk)]

kfunk added a comment.


  Note: I'll push this after the imminent  KF5 release if no-one objects.

REPOSITORY
  R240 Extra CMake Modules

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D3850

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: kfunk, #frameworks, #buildsystem, ivan
Cc: rakuco, elvisangelaccio

[Differential] [Commented On] D3850: Pass -fno-operator-names when supported

[kfunk (Kevin Funk)]

kfunk added a comment.


  In https://phabricator.kde.org/D3850#72308, @rakuco wrote:
  
  > Isn't it better to use `check_cxx_compiler_flag` to see if the flag is 
supported and enable it in case it is?
  
  
  -fno-operator-names is an ancient compiler flag, supported by GCC since at 
least 2000 [1]. I've also tested Clang, it's supported from at *least* Clang 
3.5 there. Intel compiler also supports it since at least 2005 [2]. I don't 
think we need a `check_cxx_compiler_flag` here.
  
  [1] https://gcc.gnu.org/ml/gcc-patches/2000-04/msg00333.html
  [2] https://software.intel.com/en-us/forums/intel-c-compiler/topic/308561

REPOSITORY
  R240 Extra CMake Modules

REVISION DETAIL
  https://phabricator.kde.org/D3850

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: kfunk, #frameworks, ivan, #buildsystem
Cc: rakuco, elvisangelaccio

[Differential] [Commented On] D3850: Pass -fno-operator-names when supported

[kfunk (Kevin Funk)]

kfunk added a comment.


  In https://phabricator.kde.org/D3850#72077, @elvisangelaccio wrote:
  
  > What about adding a way (cmake variable?) to opt-in if one wants to use the 
alternative operators? Personally I like and use them whenever I start 
something from scratch...
  
  
  Hmm... You could overwrite the setting by just doing this after 
`include(KDECompilerSettings)`:
  
set(CMAKE_CXX_FLAGS "${CMAKE_CXX_FLAGS} -foperator-names")
  
  But I just learned that clang++ (Clang 3.9) does not support 
-foperator-names. GCC does support it...
  
  I'd like to hear some more opinions (from Ivan maybe) first. Making the use 
of alternative tokens opt-in is an option of course.

REPOSITORY
  R240 Extra CMake Modules

REVISION DETAIL
  https://phabricator.kde.org/D3850

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: kfunk, #frameworks, ivan
Cc: elvisangelaccio

[Differential] [Updated] D3850: Pass -fno-operator-names when supported

[kfunk (Kevin Funk)]

kfunk added a reviewer: ivan.

REPOSITORY
  R240 Extra CMake Modules

REVISION DETAIL
  https://phabricator.kde.org/D3850

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: kfunk, #frameworks, ivan

[Differential] [Commented On] D3850: Pass -fno-operator-names when supported

[kfunk (Kevin Funk)]

kfunk added a comment.


  Note: Just refreshing my complete KF5 build to test the change.
  
  kactivities fails:
  
/home/kfunk/devel/src/kf5/kactivities/autotests/common/test.h:143:25: 
error: token is not a valid binary operator in a preprocessor subexpression
#if defined(Q_NO_DEBUG) or (not defined(Q_OS_LINUX))
~~~ ^
1 error generated.

REPOSITORY
  R240 Extra CMake Modules

REVISION DETAIL
  https://phabricator.kde.org/D3850

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: kfunk, #frameworks

[Differential] [Requested Changes To] D3733: Force colored warnings in ninja's output

[kfunk (Kevin Funk)]

kfunk requested changes to this revision.
kfunk added a reviewer: kfunk.
kfunk added a comment.
This revision now requires changes to proceed.


  This flag is only needed for Ninja, correct? Thus please check for Ninja in 
`CMAKE_GENERATOR ` before adding the compiler flag.

REPOSITORY
  R240 Extra CMake Modules

REVISION DETAIL
  https://phabricator.kde.org/D3733

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: elvisangelaccio, #frameworks, #buildsystem, kfunk
Cc: kfunk

[Differential] [Updated] D3548: Add begin/end insert/remove columns to RearrangeColumns

[kfunk (Kevin Funk)]

kfunk added a reviewer: dfaure.

REPOSITORY
  R275 KItemModels

REVISION DETAIL
  https://phabricator.kde.org/D3548

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: lepagevalleeemmanuel, #frameworks, dfaure
Cc: kfunk, ltoscano

[Differential] [Commented On] D3830: Add a new FindGperf module

[kfunk (Kevin Funk)]

kfunk added a comment.


  Windows: We have working gperf recipe in Craft => we're fine.
  
  QtWebKit already had an (optional) dependency on gperf.

REPOSITORY
  R240 Extra CMake Modules

REVISION DETAIL
  https://phabricator.kde.org/D3830

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: pino, #frameworks, #buildsystem, #windows, kde-mac
Cc: kfunk, rjvbb, adridg

[Differential] [Commented On] D2545: Cleanup KSharedUiServerProxy before qApp exists

[kfunk (Kevin Funk)]

kfunk added a comment.


  In https://phabricator.kde.org/D2545#69298, @thiago wrote:
  
  > In https://phabricator.kde.org/D2545#69187, @kfunk wrote:
  >
  > > In https://phabricator.kde.org/D2545#69091, @thiago wrote:
  > >
  > > > There doesn't seem to be a way of doing some clean up before the 
threads are forcibly killed.
  > > >
  > > > Maybe if I abuse qtmain().
  > >
  > >
  > > This would only fix it for non-console GUI applications if I understand 
correctly. That's what we have here, but we'd still keep console applications 
buggy. There's no way to inject code before ExitProcess() with a plain console 
application.
  >
  >
  > I was reading the ucrt source code yesterday and it does look like it 
processes atexit() registrations prior to ExitProcess(). So I'll go for that.
  
  
  It does not work with `atext()` -- I still get the same hang if I use that.
  
  So:
  
  - `std::atexit(...)` -> hang
  - `qAddPostRoutine(...)` -> no hang
  
  > There is a side-effect: I have to bring back the patch that makes QtDBus 
DLL non-unloadable, since otherwise it could crash during exit if had already 
been unloaded (was loaded by a plugin).
  > 
  > BTW, do you know if this problem happens with MinGW?
  
  According to this comment here https://bugs.kde.org/show_bug.cgi?id=366596#c5 
it also happens when built with MinGW.

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D2545

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: kfunk, vonreth, dfaure
Cc: thiago, albertvaka, mutlaqja, arrowdodger, #frameworks

[Differential] [Closed] D3691: Update QMake syntax highlighting file

[kfunk (Kevin Funk)]

This revision was automatically updated to reflect the committed changes.
Closed by commit R216:c3fc1271d6ad: Update QMake syntax highlighting file 
(authored by kfunk).

CHANGED PRIOR TO COMMIT
  https://phabricator.kde.org/D3691?vs=9048=9163#toc

REPOSITORY
  R216 Syntax Highlighting

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D3691?vs=9048=9163

REVISION DETAIL
  https://phabricator.kde.org/D3691

AFFECTED FILES
  data/generators/qmake-gen.py
  data/syntax/qmake.xml

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: kfunk, #frameworks, vkrause
Cc: vkrause

[Differential] [Commented On] D3691: Update QMake syntax highlighting file

[kfunk (Kevin Funk)]

kfunk added a comment.


  Bump

REPOSITORY
  R216 Syntax Highlighting

REVISION DETAIL
  https://phabricator.kde.org/D3691

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: kfunk, #frameworks

[Differential] [Commented On] D2075: Fix bug in kfiledialog.cpp that causes crashing when native widgets are used.

[kfunk (Kevin Funk)]

kfunk added a comment.


  In https://phabricator.kde.org/D2075#66751, @jonathans wrote:
  
  > Agreed that would be more robust. In writing the patch I was seeking 
consistency with those functions that already did the test, so those would also 
need to be updated. Are there any situations where the two tests would yield a 
different result, ie d->native is true and d->w is non-null?
  
  
  Probably not. But I think it makes more sense to check the pointer you're 
actually going to dereference in the next statement.
  
  Could you update the patch? And also fix the `nullptr` issues?

REPOSITORY
  R239  KDELibs4Support

REVISION DETAIL
  https://phabricator.kde.org/D2075

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: jonathans, #frameworks, dfaure, kfunk
Cc: kfunk, aacid

[Differential] [Closed] D3702: kconfig_compiler: Use nullptr in generated code

[kfunk (Kevin Funk)]

This revision was automatically updated to reflect the committed changes.
Closed by commit R237:e6c88f67e2cb: kconfig_compiler: Use nullptr in generated 
code (authored by kfunk).

REPOSITORY
  R237 KConfig

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D3702?vs=9072=9075

REVISION DETAIL
  https://phabricator.kde.org/D3702

AFFECTED FILES
  autotests/kconfig_compiler/test10.cpp.ref
  autotests/kconfig_compiler/test4.cpp.ref
  autotests/kconfig_compiler/test5.cpp.ref
  autotests/kconfig_compiler/test8b.cpp.ref
  autotests/kconfig_compiler/test_dpointer.cpp.ref
  autotests/kconfig_compiler/test_signal.cpp.ref
  src/kconfig_compiler/kconfig_compiler.cpp

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: kfunk, #frameworks, davidedmundson

[Differential] [Updated] D3702: kconfig_compiler: Use nullptr in generated code

[kfunk (Kevin Funk)]

kfunk added a reviewer: Frameworks.

REPOSITORY
  R237 KConfig

REVISION DETAIL
  https://phabricator.kde.org/D3702

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: kfunk, #frameworks

[Differential] [Commented On] D2545: Cleanup KSharedUiServerProxy before qApp exists

[kfunk (Kevin Funk)]

kfunk added a comment.


  In https://phabricator.kde.org/D2545#69091, @thiago wrote:
  
  > There doesn't seem to be a way of doing some clean up before the threads 
are forcibly killed.
  >
  > Maybe if I abuse qtmain().
  
  
  This would only fix it for non-console GUI applications if I understand 
correctly. That's what we have here, but we'd still keep console applications 
buggy. There's no way to inject code before ExitProcess() with a plain console 
application.

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D2545

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: kfunk, vonreth, dfaure
Cc: thiago, albertvaka, mutlaqja, arrowdodger, #frameworks

[Differential] [Commented On] D2545: Cleanup KSharedUiServerProxy before qApp exists

[kfunk (Kevin Funk)]

kfunk added a comment.


  For the record, since I don't see an easy fix I'm pondering about patching 
qtbase in craft.git:
  
  Ideas:
  
  a ) Add this to QDBusConnectionManager ctor:
  
qAddPostRoutine([]() {
QMetaObject::invokeMethod(QDBusConnectionManager::instance(), "quit");
});
  
  b) Just cherry-pick https://codereview.qt-project.org/#/c/147261/1 (tried to 
fix the same issue, but got abandoned)
  
  This might be killing DBus connections too early, but it's better than 
hanging the process on exit (I'm being pragmatic here).

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D2545

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: kfunk, vonreth, dfaure
Cc: thiago, albertvaka, mutlaqja, arrowdodger, #frameworks

[Differential] [Commented On] D2545: Cleanup KSharedUiServerProxy before qApp exists

[kfunk (Kevin Funk)]

kfunk added a comment.


  > Here's the other problem: it's possible for threads to simply disappear on 
Windows. Given that I see "dllmain" in the backtrace (though not DllMain), I 
can't rule out that this has happened. Qt 5.6 has a workaround to another 
deadlock caused by Windows. Can you try to cherry-pick 
3ec57107cedb154f256e3ad001ea5475cc64fa94 from 5.8?
  
  With 3ec57107cedb154f256e3ad001ea5475cc64fa94 applied: Still dead-locking, 
same backtrace.

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D2545

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: kfunk, vonreth, dfaure
Cc: thiago, albertvaka, mutlaqja, arrowdodger, #frameworks

[Differential] [Updated, 674 lines] D3691: Update QMake syntax highlighting file

[kfunk (Kevin Funk)]

kfunk updated this revision to Diff 9048.
kfunk added a comment.


  Add generator

REPOSITORY
  R216 Syntax Highlighting

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D3691?vs=9047=9048

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D3691

AFFECTED FILES
  data/generators/qmake-gen.py
  data/syntax/qmake.xml

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: kfunk, #frameworks

[Differential] [Updated] D3691: Update QMake syntax highlighting file

[kfunk (Kevin Funk)]

kfunk added a reviewer: Frameworks.

REPOSITORY
  R216 Syntax Highlighting

REVISION DETAIL
  https://phabricator.kde.org/D3691

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: kfunk, #frameworks

[Differential] [Updated] D3586: [WIP] Fix logging category usage on Windows

[kfunk (Kevin Funk)]

kfunk added a reviewer: Frameworks.

REPOSITORY
  R241 KIO

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D3586

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: vonreth, sandsmark, kfunk, leinir, #frameworks
Cc: kfunk, leinir, mutlaqja

[Differential] [Requested Changes To] D2075: Fix bug in kfiledialog.cpp that causes crashing when native widgets are used.

[kfunk (Kevin Funk)]

kfunk requested changes to this revision.
kfunk added a reviewer: kfunk.
kfunk added inline comments.
This revision now requires changes to proceed.

INLINE COMMENTS

> kfiledialog.cpp:607
> +if (d->native) {
> +return;
> +}

Should we rather check for `!d->w` here and below? Would make more sense IMO.

REPOSITORY
  R239  KDELibs4Support

REVISION DETAIL
  https://phabricator.kde.org/D2075

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: jonathans, #frameworks, dfaure, kfunk
Cc: kfunk, aacid

[Differential] [Updated] D3568: Find the correct path to the cmake command

[kfunk (Kevin Funk)]

kfunk edited reviewers, added: Frameworks; removed: Framework: Syntax 
Hightlighting.

BRANCH
  find-cmake-binary

REVISION DETAIL
  https://phabricator.kde.org/D3568

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: obogdan, kfunk, #frameworks
Cc: kfunk

[Differential] [Commented On] D2545: Cleanup KSharedUiServerProxy before qApp exists

[kfunk (Kevin Funk)]

kfunk added a comment.


  In https://phabricator.kde.org/D2545#65976, @dfaure wrote:
  
  > Actually, I think Thiago's still waiting for a backtrace of all threads.
  
  
  I think I've already said this via other channels: There's only one thread 
left at this point.

BRANCH
  master

REVISION DETAIL
  https://phabricator.kde.org/D2545

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: kfunk, vonreth, dfaure
Cc: albertvaka, mutlaqja, arrowdodger, #frameworks

[Differential] [Updated] D2075: Fix bug in kfiledialog.cpp that causes crashing when native widgets are used.

[kfunk (Kevin Funk)]

kfunk added a reviewer: dfaure.

REVISION DETAIL
  https://phabricator.kde.org/D2075

EMAIL PREFERENCES
  https://phabricator.kde.org/settings/panel/emailpreferences/

To: jonathans, #frameworks, dfaure

[Differential] [Updated] D2075: Fix bug in kfiledialog.cpp that causes crashing when native widgets are used.

[kfunk (Kevin Funk)]

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 (Kevin Funk)]

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 (Kevin Funk)]

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 (Kevin Funk)]

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 (Kevin Funk)]

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 (Kevin Funk)]

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 (Kevin Funk)]

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 (Kevin Funk)]

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 (Kevin Funk)]

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

[Differential] [Closed] D3091: Windows: Don't display error dialogs

[kfunk (Kevin Funk)]

kfunk closed this revision.
kfunk added a comment.


  Pushed:
  
  commit f544380db86301f4833b2149dd46dca47acc8042
  Author: Kevin Funk 
  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 (Kevin Funk)]

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 (Kevin Funk)]

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 (Kevin Funk)]

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 (Kevin Funk)]

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 (Kevin Funk)]

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

[Differential] [Commented On] D2545: Cleanup KSharedUiServerProxy before qApp exists

[kfunk (Kevin Funk)]

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 (Kevin Funk)]

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 (Kevin Funk)]

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 (Kevin Funk)]

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 (Kevin Funk)]

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 (Kevin Funk)]

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

[Differential] [Closed] D2142: KGlobalAccel: Fix deadlock on exit under Windows

[kfunk (Kevin Funk)]

kfunk closed this revision.
kfunk added a comment.


  Pushed.
  
  commit 2bdb684a872aff26c01f5e1fc175fbaf663ad8ba
  Author: Kevin Funk 
  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 (Kevin Funk)]

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 (Kevin Funk)]

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 (Kevin Funk)]

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