D25119: KF5::BreezeIcons shared library to have all breeze/breeze-dark icons and default to breeze icons set

vonreth added a comment. In D25119#676781 , @cullmann wrote: > Hmm, I don't think you need a theme chooser, we just need to watch for the palette change signal and then trigger again the theme name setting based on the background palette. >

D25119: KF5::BreezeIcons shared library to have all breeze/breeze-dark icons and default to breeze icons set

vonreth added a comment. And change those during runtime REPOSITORY R266 Breeze Icons REVISION DETAIL https://phabricator.kde.org/D25119 To: cullmann, #frameworks, vonreth, dfaure, rempt, ndavis, mart, alex-l Cc: aboyer, sars, wrobelda, davidedmundson, broulik, rempt,

D25119: KF5::BreezeIcons shared library to have all breeze/breeze-dark icons and default to breeze icons set

vonreth added a comment. Yea my idea didn't make much sense. Reading your comment I realized that we should probably implement a theme chooser in a kf5 and provide the themes as plugins, that way we could also provide other themes, not just breeze. REPOSITORY R266 Breeze Icons REVISION

D25119: KF5::BreezeIcons shared library to have all breeze/breeze-dark icons and default to breeze icons set

vonreth added a comment. I love it. Could we add a option to the exported target, so that you can basically always link that library with out an effect, until a cmake flag is set? This way we can bring it to all applications and enable it from craft. -DENABLE_BREEZE_BINARY_THEME=ON (or

D29258: Don't use notifybysnore.h on MSYS2

vonreth accepted this revision. vonreth added a comment. This revision is now accepted and ready to land. It probably won't hurt REPOSITORY R289 KNotifications BRANCH master REVISION DETAIL https://phabricator.kde.org/D29258 To: wojnilowicz, broulik, brute4s99, vonreth Cc: vonreth,

D29258: Don't use notifybysnore.h on MSYS2

vonreth added a comment. You could also just deploy the binary with msys. REPOSITORY R289 KNotifications REVISION DETAIL https://phabricator.kde.org/D29258 To: wojnilowicz, broulik, brute4s99, vonreth Cc: vonreth, kde-frameworks-devel, LeGast00n, cblack, michaelh, ngraham, bruns

D29461: Fix kio-extras build on Windows

vonreth added inline comments. INLINE COMMENTS > brute4s99 wrote in CMakeLists.txt:26 > I just built libssh 0.9.4, and kio-extras builds fine without this line > change. Maybe we should try getting latest libssh onboard. Can I help in that > somehow @vonreth ? yes pls update libssh

D29258: Don't use notifybysnore.h on MSYS2

vonreth requested changes to this revision. vonreth added a comment. This revision now requires changes to proceed. Well the code is header only, so gcc can use it too. Everything needed is to pull in a binary, that's not ideal but I guess its better than no notifications on Windows.

D27893: Use fallback also on Windows not only mac

vonreth added reviewers: broulik, bcooksley, jjazeix, cullmann. REPOSITORY R289 KNotifications REVISION DETAIL https://phabricator.kde.org/D27893 To: vonreth, broulik, bcooksley, jjazeix, cullmann Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D27893: Use fallback also on Windows not only mac

vonreth created this revision. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. vonreth requested review of this revision. REPOSITORY R289 KNotifications BRANCH master REVISION DETAIL https://phabricator.kde.org/D27893 AFFECTED FILES

D27887: Fix config include

This revision was automatically updated to reflect the committed changes. Closed by commit R289:3d455200c586: Fix config include (authored by vonreth). REPOSITORY R289 KNotifications CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D27887?vs=77081=77083 REVISION DETAIL

D27887: Fix config include

vonreth created this revision. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. vonreth requested review of this revision. REPOSITORY R289 KNotifications BRANCH master REVISION DETAIL https://phabricator.kde.org/D27887 AFFECTED FILES

D27355: POC: Make kstatusnotifieritem available without dbus

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit R289:e369e43b807e: Make kstatusnotifieritem available without dbus (authored by vonreth). REPOSITORY R289 KNotifications

D27647: throw out the windows variant of the smb slave

vonreth accepted this revision. This revision is now accepted and ready to land. REPOSITORY R320 KIO Extras BRANCH smb-no-win REVISION DETAIL https://phabricator.kde.org/D27647 To: sitter, ngraham, #windows, vonreth Cc: kde-frameworks-devel, kfm-devel, pberestov, iasensio, fprice,

D27332: try to fix issue with ispellchecker on windows

vonreth accepted this revision. vonreth added a comment. This revision is now accepted and ready to land. looks good but in a future version of sonnet plugins should be unloaded when shutting down and we should properly unload stuff. REPOSITORY R246 Sonnet BRANCH arcpatch-D27332

D12283: Allow to load dictionaries form a rcc

vonreth abandoned this revision. vonreth added a comment. as we now have a native windows backend there is no point in this REPOSITORY R246 Sonnet REVISION DETAIL https://phabricator.kde.org/D12283 To: vonreth, mlaurent Cc: ahmadsamir, kde-frameworks-devel, habacker, LeGast00n, cblack,

D27355: POC: Make kstatusnotifieritem available without dbus

vonreth added a comment. While preparing this I discovered that we should also review the locations where if MAC is used, as thats not the only platform that should use the systray icon REPOSITORY R289 KNotifications REVISION DETAIL https://phabricator.kde.org/D27355 To: vonreth,

D27355: POC: Make kstatusnotifieritem available without dbus

vonreth retitled this revision from "Make kstatusnotifieritem available without dbus" to "POC: Make kstatusnotifieritem available without dbus". vonreth edited the summary of this revision. vonreth added reviewers: bcooksley, jjazeix. REPOSITORY R289 KNotifications REVISION DETAIL

D27355: Make kstatusnotifieritem available without dbus

vonreth created this revision. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. vonreth requested review of this revision. REPOSITORY R289 KNotifications BRANCH master REVISION DETAIL https://phabricator.kde.org/D27355 AFFECTED FILES CMakeLists.txt

D26801: Really fix the Windows backend for KNotifications

vonreth added inline comments. INLINE COMMENTS > notifybysnore.cpp:90 > +KNotification *notification = m_notifications.value(id); > +if (notification == 0) { > qCDebug(LOG_KNOTIFICATIONS) << "Notification not found!"; if (!notification) or at least compare with

D26801: Really fix the Windows backend for KNotifications

vonreth added inline comments. INLINE COMMENTS > anthonyfieroni wrote in notifybysnore.cpp:143 > So the problem isn't here. notification id should be always valid and not > calculated here. I'm not sure what would be correct here but just ignoring those notifications sounds like a bad idea.

D26756: Strip richtext on Windows, simplify how rich teyt is stripped in KPassivePopup

This revision was automatically updated to reflect the committed changes. Closed by commit R289:4f2b04a171c3: Strip richtext on Windows (authored by vonreth). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D26756?vs=73885=73887#toc REPOSITORY R289 KNotifications CHANGES SINCE LAST

D26756: Strip richtext on Windows, simplify how rich teyt is stripped in KPassivePopup

vonreth updated this revision to Diff 73885. vonreth added a comment. whitespace REPOSITORY R289 KNotifications CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26756?vs=73884=73885 BRANCH master REVISION DETAIL https://phabricator.kde.org/D26756 AFFECTED FILES

D26756: Strip richtext on Windows, simplify how rich teyt is stripped in KPassivePopup

vonreth updated this revision to Diff 73884. vonreth retitled this revision from "Strip richtext on Windows, simplify how rich teyt is stripped in KPassivePopup " to "Strip richtext on Windows, simplify how rich teyt is stripped in KPassivePopup". vonreth added a comment. Whitespace

D26756: Strip richtext on Windows, simplify how rich teyt is stripped in KPassivePopup

vonreth retitled this revision from "Strip richtext on Windows, as Windows does not support it" to "Strip richtext on Windows, simplify how rich teyt is stripped in KPassivePopup ". vonreth edited the summary of this revision. REPOSITORY R289 KNotifications REVISION DETAIL

D26756: Strip richtext on Windows, as Windows does not support it

vonreth added a comment. And here in clean with proper commits https://github.com/TheOneRing/knotifications/tree/rhichtext REPOSITORY R289 KNotifications REVISION DETAIL https://phabricator.kde.org/D26756 To: vonreth, broulik, brute4s99 Cc: nicolasfella, kde-frameworks-devel,

D26756: Strip richtext on Windows, as Windows does not support it

vonreth updated this revision to Diff 73875. vonreth added a comment. - Drop dependency to KF5Codecs by using the new stripRichText function REPOSITORY R289 KNotifications CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26756?vs=73850=73875 BRANCH master REVISION DETAIL

D26757: Reduce dependencies on Windows

vonreth added inline comments. INLINE COMMENTS > nicolasfella wrote in CMakeLists.txt:42 > This results in KStatusNotifierItem not being built on Windows. Is KSNI any > useful on Windows at all? Nope :D REPOSITORY R289 KNotifications BRANCH master REVISION DETAIL

D26757: Reduce dependencies on Windows

This revision was automatically updated to reflect the committed changes. Closed by commit R289:424fc980d9e3: Reduce dependencies on Windows (authored by vonreth). REPOSITORY R289 KNotifications CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26757?vs=73851=73866 REVISION DETAIL

D26757: Reduce dependencies on Windows

vonreth added reviewers: broulik, brute4s99, vkrause. REPOSITORY R289 KNotifications REVISION DETAIL https://phabricator.kde.org/D26757 To: vonreth, broulik, brute4s99, vkrause Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D26757: Reduce dependencies on Windows

vonreth created this revision. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. vonreth requested review of this revision. REPOSITORY R289 KNotifications BRANCH master REVISION DETAIL https://phabricator.kde.org/D26757 AFFECTED FILES CMakeLists.txt

D26756: Strip richtext on Windows, as Windows does not support it

vonreth updated this revision to Diff 73850. vonreth added a comment. Switch to QTextDocumentFragment REPOSITORY R289 KNotifications CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D26756?vs=73849=73850 BRANCH master REVISION DETAIL https://phabricator.kde.org/D26756

D26756: Strip richtext on Windows, as Windows does not support it

vonreth added reviewers: broulik, brute4s99. REPOSITORY R289 KNotifications REVISION DETAIL https://phabricator.kde.org/D26756 To: vonreth, broulik, brute4s99 Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D26756: Strip richtext on Windows, as Windows does not support it

vonreth created this revision. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. vonreth requested review of this revision. REPOSITORY R289 KNotifications BRANCH master REVISION DETAIL https://phabricator.kde.org/D26756 AFFECTED FILES

D26607: Fix build on Windows

vonreth created this revision. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. vonreth requested review of this revision. REPOSITORY R241 KIO BRANCH master REVISION DETAIL https://phabricator.kde.org/D26607 AFFECTED FILES tests/kioslavetest.cpp To:

D26103: Put Q_REQUIRED_RESULT in front of the functions to fix builds with [[nodiscard]]

This revision was automatically updated to reflect the committed changes. Closed by commit R241:5cd949cce9f8: Put Q_REQUIRED_RESULT in front of the functions to fix builds with [[nodiscard]] (authored by vonreth). REPOSITORY R241 KIO CHANGES SINCE LAST UPDATE

D26103: Put Q_REQUIRED_RESULT in front of the functions to fix builds with [[nodiscard]]

vonreth added a reviewer: sitter. REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D26103 To: vonreth, sitter Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D26103: Put Q_REQUIRED_RESULT in front of the functions to fix builds with [[nodiscard]]

vonreth created this revision. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. vonreth requested review of this revision. REPOSITORY R241 KIO BRANCH master REVISION DETAIL https://phabricator.kde.org/D26103 AFFECTED FILES src/ioslaves/ftp/ftp.h To:

D25622: partial solution to accept accentuated characters on windows

vonreth added a comment. In D25622#576498 , @bruns wrote: > Again, #windows does not care ... The team is very small... REPOSITORY R286 KFileMetaData BRANCH master REVISION DETAIL

D25599: Fix plugin-metadata translations on windows

vonreth added a comment. In D25599#571880 , @sars wrote: > So the conclusion is that we are not using setDefault() yet and ki18n & gettext uses the LANGUAGE env so the logical thing, for now, is to also use LANGUAGE env here in KCoreAddons for

D25599: Fix plugin-metadata translations on windows

vonreth added a comment. In D25599#571158 , @aacid wrote: > In D25599#570678 , @vonreth wrote: > > > Are you sure?

D25599: Fix plugin-metadata translations on windows

vonreth added a comment. Are you sure? https://github.com/KDE/ki18n/blob/a5477f812c563ffd7fb08b6ec4bd573bc49bb76a/src/klocalizedstring.cpp#L146 REPOSITORY R244 KCoreAddons REVISION DETAIL https://phabricator.kde.org/D25599 To: sars, aacid, vonreth Cc: kde-frameworks-devel, LeGast00n,

D25599: Fix plugin-metadata translations on windows

vonreth added a comment. Well it sounds to me like we might need both. But the issue this pr tries to tackle would probably be gone with setDefault REPOSITORY R244 KCoreAddons REVISION DETAIL https://phabricator.kde.org/D25599 To: sars, aacid, vonreth Cc: kde-frameworks-devel,

D25599: Fix plugin-metadata translations on windows

vonreth added a comment. Why can't we use setDefault? Your linked patch does not look like it would change the behaviour on Windows? REPOSITORY R244 KCoreAddons REVISION DETAIL https://phabricator.kde.org/D25599 To: sars, aacid, vonreth Cc: kde-frameworks-devel, LeGast00n, GB_2,

D25599: Fix plugin-metadata translations on windows

vonreth added a comment. The doc doesn't mention the LANGUAGE var. QLocale::QLocale() Constructs a QLocale object initialized with the default locale. If no default locale was set using setDefault(), this locale will be the same as the one returned by system(). See

D25244: Implement ISpellChecker backend for Windows >= 8

vonreth accepted this revision. This revision is now accepted and ready to land. REPOSITORY R246 Sonnet BRANCH sonnet_ispellchecker_win REVISION DETAIL https://phabricator.kde.org/D25244 To: cullmann, #frameworks, vonreth Cc: mludwig, kde-frameworks-devel, LeGast00n, GB_2, michaelh,

D25244: Implement ISpellChecker backend for Windows >= 8

vonreth added inline comments. INLINE COMMENTS > ispellcheckerclient.cpp:33 > +// init com if needed > +m_wasCOMInitialized = SUCCEEDED(CoInitializeEx(nullptr, > COINIT_MULTITHREADED)); > + is that needed? It should be handled by the qt main loop? > ispellcheckerclient.cpp:59 > +

D24989: support multiple rcc files with icons themes

vonreth requested changes to this revision. vonreth added a subscriber: dfaure. vonreth added a comment. This revision now requires changes to proceed. Rest looks good to me. @dfaure you created the initial version what do you think? INLINE COMMENTS > kicontheme.cpp:79 > +for (const

D24841: Use modern way to set the C/CXX standard

vonreth added a comment. In D24841#552600 , @cgiboudeaux wrote: > Another issue caused by the new CMake 3.5 dependency, some tests fail: > > > > > > > 59 - ecm_setup_version-old_simple (Failed) > > 60 -

D24841: Use modern way to set the C/CXX standard

This revision was automatically updated to reflect the committed changes. Closed by commit R240:6e3c794e7c63: Use modern way to set the C/CXX standad (authored by vonreth). CHANGED PRIOR TO COMMIT https://phabricator.kde.org/D24841?vs=68488=68560#toc REPOSITORY R240 Extra CMake Modules

D24841: Use modern way to set the C/CXX standad

vonreth updated this revision to Diff 68488. vonreth added a comment. - Raise CMake requirements to 3.5 REPOSITORY R240 Extra CMake Modules CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D24841?vs=68486=68488 BRANCH cmake_c_standard REVISION DETAIL

D24841: Use modern way to set the C/CXX standad

vonreth added a comment. I guess we should do that in a separate review? REPOSITORY R240 Extra CMake Modules REVISION DETAIL https://phabricator.kde.org/D24841 To: vonreth, dfaure, cullmann Cc: aacid, chehrlic, kde-frameworks-devel, kde-buildsystem, LeGast00n, GB_2, bencreasy,

D24841: Use modern way to set the C/CXX standad

vonreth updated this revision to Diff 68486. vonreth added a comment. Fix REPOSITORY R240 Extra CMake Modules CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D24841?vs=68478=68486 BRANCH cmake_c_standard REVISION DETAIL https://phabricator.kde.org/D24841 AFFECTED FILES

D24841: Use modern way to set the C/CXX standad

vonreth updated this revision to Diff 68478. vonreth added a comment. CMAKE_CXX_STANDARD_REQUIRED REPOSITORY R240 Extra CMake Modules CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D24841?vs=68475=68478 BRANCH cmake_c_standard REVISION DETAIL

D24841: Use modern way to set the C/CXX standad

vonreth edited the summary of this revision. REPOSITORY R240 Extra CMake Modules REVISION DETAIL https://phabricator.kde.org/D24841 To: vonreth, dfaure, cullmann Cc: chehrlic, kde-frameworks-devel, kde-buildsystem, LeGast00n, GB_2, bencreasy, michaelh, ngraham, bruns

D24841: Use modern way to set the C/CXX standad

vonreth added a comment. In D24841#551653 , @chehrlic wrote: > I would also use CMAKE_CXX_STANDARD_REQUIRED to make sure the compiler actually *can* c++11, otherwise the flag is only added when the compiler supports it. Should not make much

D24841: Use modern way to set the C/CXX standad

vonreth updated this revision to Diff 68475. vonreth added a comment. Change message REPOSITORY R240 Extra CMake Modules CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D24841?vs=68473=68475 BRANCH cmake_c_standard REVISION DETAIL https://phabricator.kde.org/D24841 AFFECTED

D24841: Use modern way to set the C/CXX standad

vonreth added reviewers: dfaure, cullmann. REPOSITORY R240 Extra CMake Modules REVISION DETAIL https://phabricator.kde.org/D24841 To: vonreth, dfaure, cullmann Cc: kde-frameworks-devel, kde-buildsystem, LeGast00n, GB_2, bencreasy, michaelh, ngraham, bruns

D24841: Use modern way to set the C/CXX standad

vonreth created this revision. Herald added projects: Frameworks, Build System. Herald added subscribers: kde-buildsystem, kde-frameworks-devel. vonreth requested review of this revision. REVISION SUMMARY Raise C from C89 ti C90 as C89 is not supported by the CMAKE flag

D23282: add `-pid` parameter to Snore backend calls

vonreth accepted this revision. This revision is now accepted and ready to land. REPOSITORY R289 KNotifications BRANCH pid (branched from master) REVISION DETAIL https://phabricator.kde.org/D23282 To: brute4s99, #kde_connect, #frameworks, vonreth Cc: kde-frameworks-devel, vonreth,

D21661: add snoretoast backend for KNotifications on Windows

vonreth accepted this revision. REPOSITORY R289 KNotifications BRANCH arcpatch-D21661 REVISION DETAIL https://phabricator.kde.org/D21661 To: brute4s99, broulik, sredman, vonreth, albertvaka Cc: nicolasfella, pino, kde-frameworks-devel, LeGast00n, michaelh, ngraham, bruns

D21661: add snoretoast backend for KNotifications on Windows

vonreth added inline comments. INLINE COMMENTS > knotificationmanager.cpp:363 > + > +#include "moc_knotificationmanager_p.cpp" pls fix the duplication REPOSITORY R289 KNotifications BRANCH arcpatch-D21661 REVISION DETAIL https://phabricator.kde.org/D21661 To: brute4s99, broulik,

D22727: allow kio-extras to build with mingw on win32 and remove unnecessary includes

vonreth accepted this revision. vonreth added a reviewer: dfaure. vonreth added a comment. This revision is now accepted and ready to land. Looks good. Why not the first time  REPOSITORY R320 KIO Extras BRANCH master REVISION DETAIL https://phabricator.kde.org/D22727 To:

D22659: Fix bracket

vonreth added a comment. Applications/19.08 was branched before the commit introducing this issue REPOSITORY R320 KIO Extras REVISION DETAIL https://phabricator.kde.org/D22659 To: vonreth, brute4s99 Cc: ltoscano, kde-frameworks-devel, kfm-devel, aprcela, fprice, LeGast00n, sbergeron,

D22659: Fix bracket

This revision was automatically updated to reflect the committed changes. Closed by commit R320:872083cefb55: Fix bracket (authored by vonreth). REPOSITORY R320 KIO Extras CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D22659?vs=62336=62342 REVISION DETAIL

D22659: Fix bracket

vonreth added a reviewer: brute4s99. REPOSITORY R320 KIO Extras REVISION DETAIL https://phabricator.kde.org/D22659 To: vonreth, brute4s99 Cc: kde-frameworks-devel, kfm-devel, aprcela, fprice, LeGast00n, sbergeron, fbampaloukas, alexde, feverfew, meven, michaelh, spoorun, navarromorales,

D22659: Fix bracket

vonreth created this revision. Herald added projects: Dolphin, Frameworks. Herald added subscribers: kfm-devel, kde-frameworks-devel. vonreth requested review of this revision. REPOSITORY R320 KIO Extras BRANCH master REVISION DETAIL https://phabricator.kde.org/D22659 AFFECTED FILES

D22105: WIP : Fix SFTP Plugin of KIO for Windows

vonreth added a comment. Please revert the changes to qCDebug REPOSITORY R320 KIO Extras REVISION DETAIL https://phabricator.kde.org/D22105 To: brute4s99, albertvaka, vonreth, sredman, sitter, dfaure Cc: andriusr, kde-frameworks-devel, kfm-devel, aprcela, fprice, LeGast00n, sbergeron,

D22105: WIP : Fix SFTP Plugin of KIO for Windows

vonreth accepted this revision. vonreth added a comment. This revision is now accepted and ready to land. looks good REPOSITORY R320 KIO Extras BRANCH arcpatch-D22105 REVISION DETAIL https://phabricator.kde.org/D22105 To: brute4s99, albertvaka, vonreth, sredman, sitter, dfaure Cc:

D22105: WIP : Fix SFTP Plugin of KIO for Windows

vonreth added a comment. I think you need to ```set(CMAKE_CXX_STANDARD 17)``` to make this compile with mingw REPOSITORY R320 KIO Extras REVISION DETAIL https://phabricator.kde.org/D22105 To: brute4s99, albertvaka, vonreth, sredman, sitter, dfaure Cc: andriusr, kde-frameworks-devel,

D22105: WIP : Fix SFTP Plugin of KIO for Windows

vonreth added inline comments. INLINE COMMENTS > brute4s99 wrote in kio_sftp.cpp:2257 > `S_IFLNK` is not defined on WIndows Does https://code.woboq.org/qt5/qtbase/mkspecs/common/posix/qplatformdefs.h.html#111 work? REPOSITORY R320 KIO Extras REVISION DETAIL

D22105: WIP : Fix SFTP Plugin of KIO for Windows

vonreth added a comment. Any reason why you skip the symlinks? I mean displaying thm should be fine, dolphin probably can also follow them. Creating them on windows is a bit more problematic. REPOSITORY R320 KIO Extras REVISION DETAIL https://phabricator.kde.org/D22105 To:

D22105: WIP : Fix SFTP Plugin of KIO for Windows

vonreth added reviewers: sitter, dfaure. REPOSITORY R320 KIO Extras REVISION DETAIL https://phabricator.kde.org/D22105 To: brute4s99, albertvaka, vonreth, sredman, sitter, dfaure Cc: andriusr, kde-frameworks-devel, kfm-devel, fprice, LeGast00n, fbampaloukas, alexde, feverfew, meven,

D22105: WIP : Fix SFTP Plugin of KIO for Windows

vonreth requested changes to this revision. vonreth added a comment. This revision now requires changes to proceed. We also should add a maintainer for review INLINE COMMENTS > CMakeLists.txt:12 > > -set(QT_MIN_VERSION "5.8.0") > +set(QT_MIN_VERSION "5.10.0") > set(KF5_MIN_VERSION

D21661: add snoretoast backend for KNotifications on Windows

vonreth added inline comments. INLINE COMMENTS > shubham wrote in notifybysnore.h:44 > I doubt this compiles...You can't initialise variables inside the class Have you heard of this new c++ standard? C++11? REPOSITORY R289 KNotifications REVISION DETAIL https://phabricator.kde.org/D21661

D21661: add snoretoast backend for KNotifications on Windows

vonreth added inline comments. INLINE COMMENTS > pino wrote in notifybysnore.cpp:92 > please document what is the issue, so in the future the workaround can be > dropped I can't put my finger on it I can only tell that

D21661: add snoretoast backend for KNotifications on Windows

vonreth added inline comments. INLINE COMMENTS > notifybysnore.cpp:72 > +sock->deleteLater(); > +sock->close(); > +const QString data = deleteLater should also close the socket. Also don't use a pointer after you deleted it, even with a deleteLater. REPOSITORY R289

D19903: Fix build with msvc

vonreth added inline comments. INLINE COMMENTS > lazy_val.h:41 > public: > -operator decltype(_f()) () const > +operator auto() const -> decltype(_f()) > { Hm recently found out that this code doesn't compile with gcc. REPOSITORY R159 KActivities Statistics BRANCH master

D20907: Disable broken and no longer needed workaround code with msvc2019

This revision was automatically updated to reflect the committed changes. Closed by commit R314:57aacba17c66: Disable broken and no longer needed workaround code with msvc2019 (authored by vonreth). REPOSITORY R314 KJs CHANGES SINCE LAST UPDATE

D21661: add snoretoast backend for KNotifications on Windows

vonreth added inline comments. INLINE COMMENTS > pino wrote in notifybysnore.cpp:63 > - splitRef here to not allocate strings that are not used anyway (since they > are split again later on) > - you are using a C++11 for loop on a Qt container, and this will detach it > -- you need to store

D21661: add snoretoast backend for KNotifications on Windows

vonreth added inline comments. INLINE COMMENTS > notifybysnore.cpp:23 > + > +#include > + Is anything from Windows needed? The header is huge. > vonreth wrote in notifybysnore.cpp:180 > Use proc->deleteLater() which will call the destructor >

D21661: add snoretoast backend for KNotifications on Windows

vonreth added inline comments. INLINE COMMENTS > albertvaka wrote in notifybysnore.h:45 > Isn't LibSnore abstracting this? Why do we need to call the exe instead of > using some method in LibSnore? Libsnore is something different, yes its an abstract "notification framework" but ... its

D21661: add snoretoast backend for KNotifications on Windows

vonreth added inline comments. INLINE COMMENTS > notifybysnore.cpp:46 > +iconDir = new QTemporaryDir(); > +server = new QLocalServer(); > +server->listen(qApp->applicationName()); @brute4s99 Have you heard of the how Qt manages the life time of QObjects? If you pass this class as

D21661: add snoretoast backend for KNotifications on Windows

vonreth added a comment. In D21661#476607 , @sredman wrote: > In D21661#476571 , @pino wrote: > > > In D21661#476156 , @pino wrote: > > > > > how is

D21661: add snoretoast backend for KNotifications on Windows

vonreth added inline comments. INLINE COMMENTS > CMakeLists.txt:45 > + set_package_properties(LibSnoreToast PROPERTIES TYPE REQUIRED > + PURPOSE "Enable support for the snorenotify framework" > + DESCRIPTION "a cross-platform notification framework" It's snoretoast not

D21599: Disable valgrind detection when compiling with MSVC

vonreth added inline comments. INLINE COMMENTS > database.cpp:49 > +// the macros without ifdef'ing these in place > +#if defined _MSC_VER && !defined NVALGRIND > +#define NVALGRIND 1 Id suggest to use Q_CC_MSVC but it looks ok REPOSITORY R293 Baloo REVISION DETAIL

D20907: Disable broken and no longer needed workaround code with msvc2019

vonreth edited the summary of this revision. REPOSITORY R314 KJs REVISION DETAIL https://phabricator.kde.org/D20907 To: vonreth, dfaure, arichardson Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D20907: Disable broken and no longer needed workaround code with msvc2019

vonreth retitled this revision from "Disable boken and no longer needed workaround code" to " Disable broken and no longer needed workaround code with msvc2019". vonreth edited the summary of this revision. vonreth added reviewers: dfaure, arichardson. REPOSITORY R314 KJs REVISION DETAIL

D20907: Disable boken and no longer needed workaround code

vonreth created this revision. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. vonreth requested review of this revision. REVISION SUMMARY See: int main() { const auto inf = std::numeric_limits::infinity(); print(_MSC_VER);

D20499: Support gzipping of svgz also on windows, using 7z

vonreth added a comment. 7z will be around as it is the second core (craft) dep after wget REPOSITORY R242 Plasma Framework (Library) REVISION DETAIL https://phabricator.kde.org/D20499 To: kossebau, #plasma, #windows Cc: vonreth, cullmann, kde-frameworks-devel, michaelh, ngraham, bruns

D19903: Fix build with msvc

vonreth added a comment. So then the workaround is correct, I haven't tried whether the original code compiles with CMAKE_CXX_STANDARD=17 REPOSITORY R159 KActivities Statistics REVISION DETAIL https://phabricator.kde.org/D19903 To: vonreth, mlaurent, davidedmundson, aacid Cc:

D19903: Fix build with msvc

vonreth added a comment. Msvc2017. I think frameworks is still c++11? REPOSITORY R159 KActivities Statistics REVISION DETAIL https://phabricator.kde.org/D19903 To: vonreth, mlaurent, davidedmundson, aacid Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D19903: Fix build with msvc

vonreth added a comment. btw .vim-template:cpp .vim-template:h aren't valid file names on windows, and look strange for unix too REPOSITORY R159 KActivities Statistics REVISION DETAIL https://phabricator.kde.org/D19903 To: vonreth, mlaurent, davidedmundson, aacid Cc:

D19903: Fix build with msvc

vonreth added reviewers: mlaurent, davidedmundson, aacid. REPOSITORY R159 KActivities Statistics REVISION DETAIL https://phabricator.kde.org/D19903 To: vonreth, mlaurent, davidedmundson, aacid Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D19903: Fix build with msvc

vonreth created this revision. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. vonreth requested review of this revision. REVISION SUMMARY For the decltype change see https://stackoverflow.com/a/51347067 REPOSITORY R159 KActivities Statistics BRANCH

D19902: Remove illegal file names

vonreth abandoned this revision. REPOSITORY R159 KActivities Statistics REVISION DETAIL https://phabricator.kde.org/D19902 To: vonreth Cc: kde-frameworks-devel, michaelh, ngraham, bruns

D19902: Remove illegal file names

vonreth updated this revision to Diff 54390. vonreth added a comment. - Fix build with msvc REPOSITORY R159 KActivities Statistics CHANGES SINCE LAST UPDATE https://phabricator.kde.org/D19902?vs=54389=54390 BRANCH master REVISION DETAIL https://phabricator.kde.org/D19902 AFFECTED

D19902: Remove illegal file names

vonreth created this revision. Herald added a project: Frameworks. Herald added a subscriber: kde-frameworks-devel. vonreth requested review of this revision. REPOSITORY R159 KActivities Statistics BRANCH master REVISION DETAIL https://phabricator.kde.org/D19902 AFFECTED FILES

D18822: Fix Windows build [hopefully]

vonreth requested changes to this revision. vonreth added a comment. This revision now requires changes to proceed. Doesn't build yet, we really need pre commit ci Scanning dependencies of target KF5AuthCore_autogen [ 1%] Automatic MOC for target KF5AuthCore [ 1%] Built

D18547: Don't enable -Wzero-as-null-pointer-constant on apple clang

vonreth added a comment. In D18547#400298 , @aacid wrote: > Why? Why it breaks the cmake macro or why apple clang doesn't support it? The second one I can't answer, for the first, its the same reason why we had to bump the cmake

  1   2   >