D22105: WIP : Fix SFTP Plugin of KIO for Windows

2019-07-21 Thread Piyush Aggarwal
brute4s99 updated this revision to Diff 62254.
brute4s99 marked an inline comment as done.
brute4s99 added a comment.


  updated!

REPOSITORY
  R320 KIO Extras

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D22105?vs=62237=62254

BRANCH
  arcpatch-D22105

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

AFFECTED FILES
  CMakeLists.txt
  sftp/CMakeLists.txt
  sftp/kio_sftp.cpp

To: brute4s99, albertvaka, vonreth, sredman, sitter, dfaure
Cc: pino, andriusr, kde-frameworks-devel, kfm-devel, aprcela, fprice, 
LeGast00n, sbergeron, fbampaloukas, alexde, feverfew, meven, michaelh, spoorun, 
navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp, mikesomov


D22105: WIP : Fix SFTP Plugin of KIO for Windows

2019-07-21 Thread Piyush Aggarwal
brute4s99 edited the summary of this revision.

REPOSITORY
  R320 KIO Extras

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

To: brute4s99, albertvaka, vonreth, sredman, sitter, dfaure
Cc: pino, andriusr, kde-frameworks-devel, kfm-devel, aprcela, fprice, 
LeGast00n, sbergeron, fbampaloukas, alexde, feverfew, meven, michaelh, spoorun, 
navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp, mikesomov


D22105: WIP : Fix SFTP Plugin of KIO for Windows

2019-07-21 Thread Piyush Aggarwal
brute4s99 updated this revision to Diff 62237.
brute4s99 marked 3 inline comments as done.
brute4s99 added a comment.


  updated

REPOSITORY
  R320 KIO Extras

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D22105?vs=62187=62237

BRANCH
  arcpatch-D22105

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

AFFECTED FILES
  CMakeLists.txt
  sftp/CMakeLists.txt
  sftp/kio_sftp.cpp

To: brute4s99, albertvaka, vonreth, sredman, sitter, dfaure
Cc: pino, andriusr, kde-frameworks-devel, kfm-devel, aprcela, fprice, 
LeGast00n, sbergeron, fbampaloukas, alexde, feverfew, meven, michaelh, spoorun, 
navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp, mikesomov


D22105: WIP : Fix SFTP Plugin of KIO for Windows

2019-07-21 Thread Piyush Aggarwal
brute4s99 added inline comments.

INLINE COMMENTS

> dfaure wrote in kio_sftp.cpp:2257
> This is marked as Done, but it's not Done. In fact it matches my own 
> suggestion above, so I agree with Albert ;-)

so sorry, it seems I updated a previous version of the diff.

REPOSITORY
  R320 KIO Extras

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

To: brute4s99, albertvaka, vonreth, sredman, sitter, dfaure
Cc: pino, andriusr, kde-frameworks-devel, kfm-devel, aprcela, fprice, 
LeGast00n, sbergeron, fbampaloukas, alexde, feverfew, meven, michaelh, spoorun, 
navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp, mikesomov


D22105: WIP : Fix SFTP Plugin of KIO for Windows

2019-07-21 Thread Piyush Aggarwal
brute4s99 updated this revision to Diff 62187.
brute4s99 added a comment.


  updated

REPOSITORY
  R320 KIO Extras

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D22105?vs=62159=62187

BRANCH
  arcpatch-D22105

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

AFFECTED FILES
  CMakeLists.txt
  sftp/CMakeLists.txt
  sftp/kio_sftp.cpp

To: brute4s99, albertvaka, vonreth, sredman, sitter, dfaure
Cc: pino, andriusr, kde-frameworks-devel, kfm-devel, aprcela, fprice, 
LeGast00n, sbergeron, fbampaloukas, alexde, feverfew, meven, michaelh, spoorun, 
navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp, mikesomov


D22105: WIP : Fix SFTP Plugin of KIO for Windows

2019-07-21 Thread Piyush Aggarwal
brute4s99 marked 6 inline comments as done.

REPOSITORY
  R320 KIO Extras

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

To: brute4s99, albertvaka, vonreth, sredman, sitter, dfaure
Cc: pino, andriusr, kde-frameworks-devel, kfm-devel, aprcela, fprice, 
LeGast00n, sbergeron, fbampaloukas, alexde, feverfew, meven, michaelh, spoorun, 
navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp, mikesomov


D22105: WIP : Fix SFTP Plugin of KIO for Windows

2019-07-21 Thread Piyush Aggarwal
brute4s99 added inline comments.

INLINE COMMENTS

> pino wrote in kio_sftp.cpp:2050-2051
> what is this commented code for?

it uses buff.st_atime . Since I'm removing use of buff, I'm not sure how to 
handle this. For now I've commented out setting the file access time 
instruction for now.

REPOSITORY
  R320 KIO Extras

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

To: brute4s99, albertvaka, vonreth, sredman, sitter, dfaure
Cc: pino, andriusr, kde-frameworks-devel, kfm-devel, aprcela, fprice, 
LeGast00n, sbergeron, fbampaloukas, alexde, feverfew, meven, michaelh, spoorun, 
navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp, mikesomov


D22105: WIP : Fix SFTP Plugin of KIO for Windows

2019-07-20 Thread Piyush Aggarwal
brute4s99 updated this revision to Diff 62159.
brute4s99 marked 4 inline comments as done.
brute4s99 added a comment.


  udpated

REPOSITORY
  R320 KIO Extras

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D22105?vs=61972=62159

BRANCH
  arcpatch-D22105

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

AFFECTED FILES
  CMakeLists.txt
  sftp/CMakeLists.txt
  sftp/kio_sftp.cpp

To: brute4s99, albertvaka, vonreth, sredman, sitter, dfaure
Cc: andriusr, kde-frameworks-devel, kfm-devel, aprcela, fprice, LeGast00n, 
sbergeron, fbampaloukas, alexde, feverfew, meven, michaelh, spoorun, 
navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp, mikesomov


D22554: Finer No-Dbus on Windows

2019-07-19 Thread Piyush Aggarwal
brute4s99 marked an inline comment as done.

REPOSITORY
  R289 KNotifications

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

To: brute4s99, nicolasfella, broulik
Cc: andriusr, kde-frameworks-devel, LeGast00n, sbergeron, michaelh, ngraham, 
bruns


D22554: Finer No-Dbus on Windows

2019-07-19 Thread Piyush Aggarwal
brute4s99 updated this revision to Diff 62069.
brute4s99 added a comment.


  updated

REPOSITORY
  R289 KNotifications

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D22554?vs=62051=62069

BRANCH
  arcpatch-D22554

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

AFFECTED FILES
  src/CMakeLists.txt

To: brute4s99, nicolasfella, broulik
Cc: andriusr, kde-frameworks-devel, LeGast00n, sbergeron, michaelh, ngraham, 
bruns


D22554: Finer No-Dbus on Windows

2019-07-19 Thread Piyush Aggarwal
brute4s99 added a comment.


  In D22554#497984 , @nicolasfella 
wrote:
  
  > Now HAVE_DBUSMENUQT won't be set. Previously it was set to 0 in this case
  
  
  Which line number are you referring to?

REPOSITORY
  R289 KNotifications

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

To: brute4s99, nicolasfella, broulik
Cc: andriusr, kde-frameworks-devel, LeGast00n, sbergeron, michaelh, ngraham, 
bruns


D22554: Finer No-Dbus on Windows

2019-07-19 Thread Piyush Aggarwal
brute4s99 marked an inline comment as done.

REPOSITORY
  R289 KNotifications

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

To: brute4s99, nicolasfella, broulik
Cc: kde-frameworks-devel, LeGast00n, sbergeron, michaelh, ngraham, bruns


D22554: Finer No-Dbus on Windows

2019-07-19 Thread Piyush Aggarwal
brute4s99 updated this revision to Diff 62051.
brute4s99 added a comment.


  updated

REPOSITORY
  R289 KNotifications

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D22554?vs=62048=62051

BRANCH
  arcpatch-D22554

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

AFFECTED FILES
  src/CMakeLists.txt

To: brute4s99, nicolasfella, broulik
Cc: kde-frameworks-devel, LeGast00n, sbergeron, michaelh, ngraham, bruns


D22554: Finer No-Dbus on Windows

2019-07-19 Thread Piyush Aggarwal
brute4s99 created this revision.
brute4s99 added reviewers: nicolasfella, broulik.
brute4s99 added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
brute4s99 requested review of this revision.

REVISION SUMMARY
  remove another dependency related to DBus from Windows builds

REPOSITORY
  R289 KNotifications

BRANCH
  master

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

AFFECTED FILES
  src/CMakeLists.txt

To: brute4s99, nicolasfella, broulik
Cc: kde-frameworks-devel, LeGast00n, sbergeron, michaelh, ngraham, bruns


D22105: WIP : Fix SFTP Plugin of KIO for Windows

2019-07-18 Thread Piyush Aggarwal
brute4s99 added inline comments.

INLINE COMMENTS

> dfaure wrote in kio_sftp.cpp:2037
> Why ReadWrite, if we know it doesn't exist?
> 
> Does setFileTime() even need open() first? I wouldn't have thought so.

`setFileTime()` needs the file to be open. 
Source : https://doc.qt.io/qt-5/qfiledevice.html#setFileTime

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, fbampaloukas, alexde, feverfew, meven, michaelh, spoorun, 
navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp, mikesomov


D22105: WIP : Fix SFTP Plugin of KIO for Windows

2019-07-18 Thread Piyush Aggarwal
brute4s99 updated this revision to Diff 61972.
brute4s99 marked 7 inline comments as done.
brute4s99 added a comment.


  updated wrt new comments. @dfaure please take another look! 

REPOSITORY
  R320 KIO Extras

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D22105?vs=61419=61972

BRANCH
  arcpatch-D22105

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

AFFECTED FILES
  CMakeLists.txt
  sftp/CMakeLists.txt
  sftp/kio_sftp.cpp

To: brute4s99, albertvaka, vonreth, sredman, sitter, dfaure
Cc: andriusr, kde-frameworks-devel, kfm-devel, aprcela, fprice, LeGast00n, 
sbergeron, fbampaloukas, alexde, feverfew, meven, michaelh, spoorun, 
navarromorales, firef, ngraham, andrebarros, bruns, emmanuelp, mikesomov


D21660: remove dbus for windows build and change audio dep logic

2019-07-18 Thread Piyush Aggarwal
This revision was automatically updated to reflect the committed changes.
Closed by commit R289:be466180db21: remove dbus for windows build and change 
audio dep logic (authored by brute4s99).

REPOSITORY
  R289 KNotifications

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D21660?vs=61716=61968

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

AFFECTED FILES
  CMakeLists.txt
  src/knotificationmanager.cpp

To: brute4s99, broulik, nicolasfella
Cc: bcooksley, apol, nicolasfella, kde-frameworks-devel, LeGast00n, sbergeron, 
michaelh, ngraham, bruns


D21660: remove dbus for windows build and change audio dep logic

2019-07-18 Thread Piyush Aggarwal
brute4s99 added a comment.


  landing it

REPOSITORY
  R289 KNotifications

BRANCH
  arcpatch-D21660_1

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

To: brute4s99, broulik, nicolasfella
Cc: bcooksley, apol, nicolasfella, kde-frameworks-devel, LeGast00n, sbergeron, 
michaelh, ngraham, bruns


D21660: remove dbus for windows build and change audio dep logic

2019-07-17 Thread Piyush Aggarwal
brute4s99 marked 8 inline comments as done.

REPOSITORY
  R289 KNotifications

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

To: brute4s99, broulik, nicolasfella
Cc: bcooksley, apol, nicolasfella, kde-frameworks-devel, LeGast00n, sbergeron, 
michaelh, ngraham, bruns


D21660: remove dbus for windows build and change audio dep logic

2019-07-13 Thread Piyush Aggarwal
brute4s99 updated this revision to Diff 61716.
brute4s99 added a comment.


  updated

REPOSITORY
  R289 KNotifications

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D21660?vs=61639=61716

BRANCH
  arcpatch-D21660_1

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

AFFECTED FILES
  CMakeLists.txt
  src/knotificationmanager.cpp

To: brute4s99, broulik, nicolasfella
Cc: bcooksley, apol, nicolasfella, kde-frameworks-devel, LeGast00n, sbergeron, 
michaelh, ngraham, bruns


D21660: remove dbus for windows build and change audio dep logic

2019-07-12 Thread Piyush Aggarwal
brute4s99 retitled this revision from "remove dbus and change audio dep logic" 
to "remove dbus for windows build and change audio dep logic".

REPOSITORY
  R289 KNotifications

BRANCH
  arcpatch-D21660_1

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

To: brute4s99, broulik, nicolasfella
Cc: bcooksley, apol, nicolasfella, kde-frameworks-devel, LeGast00n, michaelh, 
ngraham, bruns


D21660: remove dbus and change audio dep logic

2019-07-12 Thread Piyush Aggarwal
brute4s99 retitled this revision from "change audio dep logic wrt win32" to 
"remove dbus and change audio dep logic".
brute4s99 edited the summary of this revision.

REPOSITORY
  R289 KNotifications

BRANCH
  arcpatch-D21660_1

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

To: brute4s99, broulik, nicolasfella
Cc: bcooksley, apol, nicolasfella, kde-frameworks-devel, LeGast00n, michaelh, 
ngraham, bruns


D21660: change audio dep logic wrt win32

2019-07-12 Thread Piyush Aggarwal
brute4s99 updated this revision to Diff 61639.
brute4s99 added a comment.


  no more dbus on Windows build of KNotifications

REPOSITORY
  R289 KNotifications

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D21660?vs=61422=61639

BRANCH
  arcpatch-D21660_1

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

AFFECTED FILES
  CMakeLists.txt
  src/knotificationmanager.cpp

To: brute4s99, broulik, nicolasfella
Cc: bcooksley, apol, nicolasfella, kde-frameworks-devel, LeGast00n, michaelh, 
ngraham, bruns


D21660: change audio dep logic wrt win32

2019-07-12 Thread Piyush Aggarwal
brute4s99 added inline comments.

INLINE COMMENTS

> nicolasfella wrote in CMakeLists.txt:38
> But this is KNotifications, not KDE Connect. So we need DBus for 
> KNotifications on Windows?

ah, yeah we could bypass DBus on Windows in KNotifications

REPOSITORY
  R289 KNotifications

BRANCH
  arcpatch-D21660

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

To: brute4s99, broulik, nicolasfella
Cc: bcooksley, apol, nicolasfella, kde-frameworks-devel, LeGast00n, michaelh, 
ngraham, bruns


D21660: change audio dep logic wrt win32

2019-07-11 Thread Piyush Aggarwal
brute4s99 added inline comments.

INLINE COMMENTS

> brute4s99 wrote in CMakeLists.txt:38
> I'm looking into the DBus problem, we currently use DBus for the system tray 
> icon on Windows. Once that is resolved, I should be able to completely remove 
> DBus from the Windows build

Update: nope. I think it's still use for the dbus interfaces. I confirmed by 
ending the `dbus-daemon.exe` process by hand to see it. Turns out, dbus is 
quite important for KDE Connect regardless of the OS. (:

REPOSITORY
  R289 KNotifications

BRANCH
  arcpatch-D21660

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

To: brute4s99, broulik, nicolasfella
Cc: bcooksley, apol, nicolasfella, kde-frameworks-devel, LeGast00n, michaelh, 
ngraham, bruns


D21660: change audio dep logic wrt win32

2019-07-09 Thread Piyush Aggarwal
brute4s99 added inline comments.

INLINE COMMENTS

> nicolasfella wrote in CMakeLists.txt:38
> Yes. It is neither related to Windows nor audio. What you can to is to add 
> Windows to the condition to not look for DBus on Windows, but please do that 
> in a separate patch. Or move the find call for DBus down to the auto checks 
> inside the if

I'm looking into the DBus problem, we currently use DBus for the system tray 
icon on Windows. Once that is resolved, I should be able to completely remove 
DBus from the Windows build

REPOSITORY
  R289 KNotifications

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

To: brute4s99, broulik, nicolasfella
Cc: bcooksley, apol, nicolasfella, kde-frameworks-devel, LeGast00n, michaelh, 
ngraham, bruns


D21660: change audio dep logic wrt win32

2019-07-09 Thread Piyush Aggarwal
brute4s99 updated this revision to Diff 61422.
brute4s99 added a comment.


  updated

REPOSITORY
  R289 KNotifications

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D21660?vs=61421=61422

BRANCH
  arcpatch-D21660

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

AFFECTED FILES
  CMakeLists.txt

To: brute4s99, broulik, nicolasfella
Cc: bcooksley, apol, nicolasfella, kde-frameworks-devel, LeGast00n, michaelh, 
ngraham, bruns


D21660: change audio dep logic wrt win32

2019-07-09 Thread Piyush Aggarwal
brute4s99 added inline comments.

INLINE COMMENTS

> nicolasfella wrote in CMakeLists.txt:38
> This seems to be entirely urelated to audio?

I was trying to simplify code there by removing `NOT`. Should I remove this 
change?

REPOSITORY
  R289 KNotifications

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

To: brute4s99, broulik, nicolasfella
Cc: bcooksley, apol, nicolasfella, kde-frameworks-devel, LeGast00n, michaelh, 
ngraham, bruns


D21660: change audio dep logic wrt win32

2019-07-09 Thread Piyush Aggarwal
brute4s99 edited the summary of this revision.

REPOSITORY
  R289 KNotifications

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

To: brute4s99, broulik, nicolasfella
Cc: bcooksley, apol, nicolasfella, kde-frameworks-devel, LeGast00n, michaelh, 
ngraham, bruns


D21660: change audio dep logic wrt win32

2019-07-09 Thread Piyush Aggarwal
brute4s99 updated this revision to Diff 61421.
brute4s99 added a comment.


  rebased

REPOSITORY
  R289 KNotifications

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D21660?vs=59551=61421

BRANCH
  arcpatch-D21660

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

AFFECTED FILES
  CMakeLists.txt

To: brute4s99, broulik, nicolasfella
Cc: bcooksley, apol, nicolasfella, kde-frameworks-devel, LeGast00n, michaelh, 
ngraham, bruns


D21660: change audio dep logic wrt win32

2019-07-09 Thread Piyush Aggarwal
brute4s99 added a comment.


  may I land this?

REPOSITORY
  R289 KNotifications

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

To: brute4s99, broulik
Cc: bcooksley, apol, nicolasfella, kde-frameworks-devel, LeGast00n, michaelh, 
ngraham, bruns


D22105: WIP : Fix SFTP Plugin of KIO for Windows

2019-07-09 Thread Piyush Aggarwal
brute4s99 added inline comments.

INLINE COMMENTS

> kio_sftp.cpp:29
>  #include 
> -#include 
>  

this builds without `utime.h` on my system (Arch Linux with latest Plasma, Qt 
and other packages), . Please inform if someone else has issues without this 
header.

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, michaelh, spoorun, navarromorales, firef, ngraham, 
andrebarros, bruns, emmanuelp, mikesomov


D22105: WIP : Fix SFTP Plugin of KIO for Windows

2019-07-09 Thread Piyush Aggarwal
brute4s99 updated this revision to Diff 61419.
brute4s99 added a comment.


  Updated with `QT_` pre-procs for S_IFDIR, S_IFLNK and others.

REPOSITORY
  R320 KIO Extras

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D22105?vs=61225=61419

BRANCH
  arcpatch-D22105

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

AFFECTED FILES
  CMakeLists.txt
  sftp/CMakeLists.txt
  sftp/kio_sftp.cpp

To: brute4s99, albertvaka, vonreth, sredman, sitter, dfaure
Cc: andriusr, kde-frameworks-devel, kfm-devel, fprice, LeGast00n, fbampaloukas, 
alexde, feverfew, meven, michaelh, spoorun, navarromorales, firef, ngraham, 
andrebarros, bruns, emmanuelp, mikesomov


D21660: change audio dep logic wrt win32

2019-07-05 Thread Piyush Aggarwal
brute4s99 marked 2 inline comments as done.

REPOSITORY
  R289 KNotifications

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

To: brute4s99, broulik
Cc: bcooksley, apol, nicolasfella, kde-frameworks-devel, LeGast00n, michaelh, 
ngraham, bruns


D21660: change audio dep logic wrt win32

2019-07-05 Thread Piyush Aggarwal
brute4s99 added a comment.


  In D21660#481421 , @bcooksley 
wrote:
  
  > Where possible D-Bus should be avoided on Windows.
  
  
  Thanks for taking a look, @bcooksley! We already use dbus-daemon.exe for a 
lot of talking with the notifications and (I think many) other features, so we 
can't really avoid it just now.

REPOSITORY
  R289 KNotifications

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

To: brute4s99, broulik
Cc: bcooksley, apol, nicolasfella, kde-frameworks-devel, LeGast00n, michaelh, 
ngraham, bruns


D22105: WIP : Fix SFTP Plugin of KIO for Windows

2019-07-05 Thread Piyush Aggarwal
brute4s99 added inline comments.

INLINE COMMENTS

> vonreth wrote in kio_sftp.cpp:2257
> Does 
> https://code.woboq.org/qt5/qtbase/mkspecs/common/posix/qplatformdefs.h.html#111
>  work?

yes! thanks Hannah! 

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, michaelh, spoorun, navarromorales, firef, ngraham, 
andrebarros, bruns, emmanuelp, mikesomov


D22105: WIP : Fix SFTP Plugin of KIO for Windows

2019-07-05 Thread Piyush Aggarwal
brute4s99 marked 9 inline comments as done.

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, michaelh, spoorun, navarromorales, firef, ngraham, 
andrebarros, bruns, emmanuelp, mikesomov


D22105: WIP : Fix SFTP Plugin of KIO for Windows

2019-07-05 Thread Piyush Aggarwal
brute4s99 updated this revision to Diff 61225.
brute4s99 added a comment.


  added handling for WIndows. I'll try it on linux and revert on this patch!

REPOSITORY
  R320 KIO Extras

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D22105?vs=61035=61225

BRANCH
  arcpatch-D22105

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

AFFECTED FILES
  CMakeLists.txt
  sftp/CMakeLists.txt
  sftp/kio_sftp.cpp

To: brute4s99, albertvaka, vonreth, sredman, sitter, dfaure
Cc: andriusr, kde-frameworks-devel, kfm-devel, fprice, LeGast00n, fbampaloukas, 
alexde, feverfew, meven, michaelh, spoorun, navarromorales, firef, ngraham, 
andrebarros, bruns, emmanuelp, mikesomov


D22105: WIP : Fix SFTP Plugin of KIO for Windows

2019-07-03 Thread Piyush Aggarwal
brute4s99 added inline comments.

INLINE COMMENTS

> albertvaka wrote in kio_sftp.cpp:2257
> I think what they mean is that if you remove the ifdef it should just work.

`S_IFLNK` is not defined on WIndows

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, michaelh, spoorun, navarromorales, firef, ngraham, 
andrebarros, bruns, emmanuelp, mikesomov


D22105: WIP : Fix SFTP Plugin of KIO for Windows

2019-07-03 Thread Piyush Aggarwal
brute4s99 added inline comments.

INLINE COMMENTS

> albertvaka wrote in kio_sftp.cpp:2257
> I think what they mean is that if you remove the ifdef it should just work.

ah, gotcha!

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, michaelh, spoorun, navarromorales, firef, ngraham, 
andrebarros, bruns, emmanuelp, mikesomov


D21661: add snoretoast backend for KNotifications on Windows

2019-07-03 Thread Piyush Aggarwal
brute4s99 added a comment.


  I believe I have covered all the issues with the patch now. (:

REPOSITORY
  R289 KNotifications

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

2019-07-03 Thread Piyush Aggarwal
brute4s99 updated this revision to Diff 61084.
brute4s99 added a comment.


  renamed variables (start with `m_` now)

REPOSITORY
  R289 KNotifications

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D21661?vs=60035=61084

BRANCH
  arcpatch-D21661

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

AFFECTED FILES
  src/CMakeLists.txt
  src/knotification.cpp
  src/knotificationmanager.cpp
  src/notifybysnore.cpp
  src/notifybysnore.h

To: brute4s99, broulik, sredman, vonreth, albertvaka
Cc: nicolasfella, pino, kde-frameworks-devel, LeGast00n, michaelh, ngraham, 
bruns


D22105: WIP : Fix SFTP Plugin of KIO for Windows

2019-07-03 Thread Piyush Aggarwal
brute4s99 added a comment.


  In D22105#489954 , @vonreth wrote:
  
  > 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.
  
  
  For my use case, I could not find any possibility of symlinks, (traversing 
Android filesystem) so I left it blank in here for any future dev to fix it.

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, michaelh, spoorun, navarromorales, firef, ngraham, 
andrebarros, bruns, emmanuelp, mikesomov


D22105: WIP : Fix SFTP Plugin of KIO for Windows

2019-07-03 Thread Piyush Aggarwal
brute4s99 marked an inline comment as done.
brute4s99 added inline comments.

INLINE COMMENTS

> vonreth wrote in kio_sftp.cpp:402
> What about the QFileInfo?

For my use case, I could not find any possibility of symlinks, (traversing 
Android filesystem) so I left it blank in here for any future dev to fix it.

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, michaelh, spoorun, navarromorales, firef, ngraham, 
andrebarros, bruns, emmanuelp, mikesomov


D22105: WIP : Fix SFTP Plugin of KIO for Windows

2019-07-02 Thread Piyush Aggarwal
brute4s99 added inline comments.

INLINE COMMENTS

> vonreth wrote in CMakeLists.txt:12
> What requires this bump?

ah, I thought QFileDevice needs it. Reverting this.

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, michaelh, spoorun, navarromorales, firef, ngraham, 
andrebarros, bruns, emmanuelp, mikesomov


D22105: WIP : Fix SFTP Plugin of KIO for Windows

2019-07-02 Thread Piyush Aggarwal
brute4s99 updated this revision to Diff 61035.
brute4s99 marked 6 inline comments as done.

REPOSITORY
  R320 KIO Extras

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D22105?vs=60701=61035

BRANCH
  arcpatch-D22105

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

AFFECTED FILES
  CMakeLists.txt
  sftp/kio_sftp.cpp

To: brute4s99, albertvaka, vonreth, sredman, sitter, dfaure
Cc: andriusr, kde-frameworks-devel, kfm-devel, fprice, LeGast00n, fbampaloukas, 
alexde, feverfew, meven, michaelh, spoorun, navarromorales, firef, ngraham, 
andrebarros, bruns, emmanuelp, mikesomov


D22221: updated

2019-07-02 Thread Piyush Aggarwal
brute4s99 abandoned this revision.

REPOSITORY
  R320 KIO Extras

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

To: brute4s99
Cc: kde-frameworks-devel, kfm-devel, fprice, LeGast00n, fbampaloukas, alexde, 
feverfew, meven, michaelh, spoorun, navarromorales, firef, ngraham, 
andrebarros, bruns, emmanuelp, mikesomov


D22221: updated

2019-07-02 Thread Piyush Aggarwal
brute4s99 created this revision.
Herald added projects: Dolphin, Frameworks.
Herald added subscribers: kfm-devel, kde-frameworks-devel.
brute4s99 requested review of this revision.

REPOSITORY
  R320 KIO Extras

BRANCH
  arcpatch-D22105

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

AFFECTED FILES
  CMakeLists.txt
  sftp/kio_sftp.cpp

To: brute4s99
Cc: kde-frameworks-devel, kfm-devel, fprice, LeGast00n, fbampaloukas, alexde, 
feverfew, meven, michaelh, spoorun, navarromorales, firef, ngraham, 
andrebarros, bruns, emmanuelp, mikesomov


D22105: WIP : Fix SFTP Plugin of KIO for Windows

2019-06-26 Thread Piyush Aggarwal
brute4s99 edited the summary of this revision.

REPOSITORY
  R320 KIO Extras

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

To: brute4s99, albertvaka, vonreth, sredman
Cc: kde-frameworks-devel, kfm-devel, fprice, LeGast00n, fbampaloukas, alexde, 
feverfew, meven, michaelh, spoorun, navarromorales, firef, ngraham, 
andrebarros, bruns, emmanuelp, mikesomov


D22105: WIP : Fix SFTP Plugin of KIO for Windows

2019-06-26 Thread Piyush Aggarwal
brute4s99 created this revision.
brute4s99 added reviewers: albertvaka, vonreth, sredman.
Herald added projects: Dolphin, Frameworks.
Herald added subscribers: kfm-devel, kde-frameworks-devel.
brute4s99 requested review of this revision.

REVISION SUMMARY
  CAUTION : Still WIP. The main functionality works, but it's not a pleasant 
for the end user for now.
  
  The fixed plugin works on Windows. Right now, KDE Connect uses it to 
establish an initial connection over SFTP, which can then be offloaded to any 
sftp:// handling application.
  
  Currently, this can be achieved seamlessly by using WinSCP 
 which is a free and open source SFTP Client.

TEST PLAN
  0. Install WinSCP : https://winscp.net/eng/download.php.
  
  1. set Craft to use `master` for `kio-extras`
  
craft --set version=master kio-extras
  
  
  
  2. install `kio-extras` and all deps
  
craft -i kio-extras
  
  
  
  3. apply this patch
  
  4. re-build `kio-extras`
  
craft --compile --install --qmerge kio-extras
  
  
  
  5. checkout `milestone2` branch from 
invent.kde.org/piyushaggarwal/kdeconnect-kde
  
  6. build `kdeconnect-kde` again
  
craft --compile --install --qmerge kdeconnect-kde
  
  
  
  7. Run `kdeconnect-indicator.exe` from within `CraftRoot/bin/`
  
  8. Right  Click on the dark icon in sys tray, go to your phone and click on 
**Browse Device**.
  
  9. Press YES, followed by an OK on password prompt (the correct password 
comes pre-filled).
  
  10. Expect WinSCP to take point from there on.

REPOSITORY
  R320 KIO Extras

BRANCH
  master

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

AFFECTED FILES
  CMakeLists.txt
  sftp/kio_sftp.cpp

To: brute4s99, albertvaka, vonreth, sredman
Cc: kde-frameworks-devel, kfm-devel, fprice, LeGast00n, fbampaloukas, alexde, 
feverfew, meven, michaelh, spoorun, navarromorales, firef, ngraham, 
andrebarros, bruns, emmanuelp, mikesomov


D21661: add snoretoast backend for KNotifications on Windows

2019-06-26 Thread Piyush Aggarwal
brute4s99 marked 10 inline comments as done.

REPOSITORY
  R289 KNotifications

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

2019-06-19 Thread Piyush Aggarwal
brute4s99 added inline comments.

INLINE COMMENTS

> brute4s99 wrote in notifybysnore.cpp:84
> well, I just found out the patch had broken functionality that I fixed just 
> after putting here an `else return`! 

I have fixed the issue now.

REPOSITORY
  R289 KNotifications

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

2019-06-18 Thread Piyush Aggarwal
brute4s99 updated this revision to Diff 60035.
brute4s99 marked 9 inline comments as done.

REPOSITORY
  R289 KNotifications

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D21661?vs=59835=60035

BRANCH
  arcpatch-D21661

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

AFFECTED FILES
  src/CMakeLists.txt
  src/knotification.cpp
  src/knotificationmanager.cpp
  src/notifybysnore.cpp
  src/notifybysnore.h

To: brute4s99, broulik, sredman, vonreth, albertvaka
Cc: nicolasfella, pino, kde-frameworks-devel, LeGast00n, michaelh, ngraham, 
bruns


D21661: add snoretoast backend for KNotifications on Windows

2019-06-18 Thread Piyush Aggarwal
brute4s99 added inline comments.

INLINE COMMENTS

> pino wrote in notifybysnore.cpp:84
> if the notification is not found, this will be an uninitialized pointer; TBH 
> if the search for the notification with the specified id fails, then it 
> should be better to return earlier, as it means the notification is unknown

well, I just found out the patch had broken functionality that I fixed just 
after putting here an `else return`! 

> pino wrote in notifybysnore.cpp:168-171
> the logic here is swapped: if `waitForStarted()` returns false, that means 
> the process did not start successfully; also, after `finish()` you must 
> return earlier (do not forget to delete the process), otherwise the rest of 
> the code does things as if the process run fine

I'm removing this code chunk because we already check for successful show of 
notif in the following `connect`.

> pino wrote in notifybysnore.cpp:44
> > if you could guide me on how to update the docs on the website, that'd be 
> > great!
> 
> which website?

https://api.kde.org/frameworks/knotifications/html/index.html

REPOSITORY
  R289 KNotifications

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

To: brute4s99, broulik, sredman, vonreth, albertvaka
Cc: nicolasfella, pino, kde-frameworks-devel, LeGast00n, michaelh, ngraham, 
bruns


D21660: change audio dep logic wrt win32

2019-06-18 Thread Piyush Aggarwal
brute4s99 marked an inline comment as done.
brute4s99 added inline comments.

INLINE COMMENTS

> nicolasfella wrote in CMakeLists.txt:42
> We don't need DBus on Windows, do we?

we don't, I guess, but dbus-daemon.exe still runs in the background so I can't 
say. 
The functionality doesn't seem to have been affected by removing this in my 
build, though.

REPOSITORY
  R289 KNotifications

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

To: brute4s99, broulik
Cc: apol, nicolasfella, kde-frameworks-devel, LeGast00n, michaelh, ngraham, 
bruns


D21661: add snoretoast backend for KNotifications on Windows

2019-06-14 Thread Piyush Aggarwal
brute4s99 updated this revision to Diff 59835.
brute4s99 added a comment.


  updated acc to workaround for MSVC2019 suggested by Hannah
  added inline comment for it at L92 -- notifybysnore.cpp

REPOSITORY
  R289 KNotifications

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D21661?vs=59826=59835

BRANCH
  win32 (branched from master)

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

AFFECTED FILES
  src/CMakeLists.txt
  src/knotification.cpp
  src/knotificationmanager.cpp
  src/notifybysnore.cpp
  src/notifybysnore.h

To: brute4s99, broulik, sredman, vonreth, albertvaka
Cc: nicolasfella, pino, kde-frameworks-devel, LeGast00n, michaelh, ngraham, 
bruns


D21661: add snoretoast backend for KNotifications on Windows

2019-06-14 Thread Piyush Aggarwal
brute4s99 updated this revision to Diff 59826.
brute4s99 added a comment.


  rebased on upstream/master

REPOSITORY
  R289 KNotifications

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D21661?vs=59774=59826

BRANCH
  win32 (branched from master)

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

AFFECTED FILES
  src/CMakeLists.txt
  src/knotification.cpp
  src/knotificationmanager.cpp
  src/notifybysnore.cpp
  src/notifybysnore.h

To: brute4s99, broulik, sredman, vonreth, albertvaka
Cc: nicolasfella, pino, kde-frameworks-devel, LeGast00n, michaelh, ngraham, 
bruns


D21661: add snoretoast backend for KNotifications on Windows

2019-06-13 Thread Piyush Aggarwal
brute4s99 updated this revision to Diff 59774.

REPOSITORY
  R289 KNotifications

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D21661?vs=59766=59774

BRANCH
  win32 (branched from master)

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

AFFECTED FILES
  src/CMakeLists.txt
  src/knotification.cpp
  src/knotificationmanager.cpp
  src/notifybysnore.cpp
  src/notifybysnore.h

To: brute4s99, broulik, sredman, vonreth, albertvaka
Cc: nicolasfella, pino, kde-frameworks-devel, LeGast00n, michaelh, ngraham, 
bruns


D21661: add snoretoast backend for KNotifications on Windows

2019-06-13 Thread Piyush Aggarwal
brute4s99 updated this revision to Diff 59766.

REPOSITORY
  R289 KNotifications

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D21661?vs=59765=59766

BRANCH
  win32 (branched from master)

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

AFFECTED FILES
  src/CMakeLists.txt
  src/knotification.cpp
  src/knotificationmanager.cpp
  src/notifybysnore.cpp
  src/notifybysnore.h

To: brute4s99, broulik, sredman, vonreth, albertvaka
Cc: nicolasfella, pino, kde-frameworks-devel, LeGast00n, michaelh, ngraham, 
bruns


D21661: add snoretoast backend for KNotifications on Windows

2019-06-13 Thread Piyush Aggarwal
brute4s99 updated this revision to Diff 59765.
brute4s99 marked an inline comment as done.

REPOSITORY
  R289 KNotifications

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D21661?vs=59574=59765

BRANCH
  win32 (branched from master)

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

AFFECTED FILES
  src/CMakeLists.txt
  src/knotification.cpp
  src/knotificationmanager.cpp
  src/notifybysnore.cpp
  src/notifybysnore.h

To: brute4s99, broulik, sredman, vonreth, albertvaka
Cc: nicolasfella, pino, kde-frameworks-devel, LeGast00n, michaelh, ngraham, 
bruns


D21661: add snoretoast backend for KNotifications on Windows

2019-06-13 Thread Piyush Aggarwal
brute4s99 marked 67 inline comments as done.
brute4s99 added a comment.


  updated code incoming. I think I should make a new diff for further 
discussions, as this one is quite riddled with suggestions now. Are there any 
more issues with this patch or should I continue with a new one instead? I'm 
willing to fix it further if you have some suggestions! ✊

INLINE COMMENTS

> nicolasfella wrote in notifybysnore.cpp:44
> This should be documented somewhere else, too. Either the API dox or the KDE 
> wiki

I've added more inline docs for now. @pino if you could guide me on how to 
update the docs on the website, that'd be great! 

REPOSITORY
  R289 KNotifications

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

2019-06-10 Thread Piyush Aggarwal
brute4s99 updated this revision to Diff 59574.

REPOSITORY
  R289 KNotifications

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D21661?vs=59573=59574

BRANCH
  win32 (branched from master)

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

AFFECTED FILES
  src/CMakeLists.txt
  src/knotification.cpp
  src/knotificationmanager.cpp
  src/notifybysnore.cpp
  src/notifybysnore.h

To: brute4s99, broulik, sredman, vonreth, albertvaka
Cc: nicolasfella, pino, kde-frameworks-devel, LeGast00n, michaelh, ngraham, 
bruns


D21661: add snoretoast backend for KNotifications on Windows

2019-06-10 Thread Piyush Aggarwal
brute4s99 updated this revision to Diff 59573.

REPOSITORY
  R289 KNotifications

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D21661?vs=59572=59573

BRANCH
  win32 (branched from master)

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

AFFECTED FILES
  src/CMakeLists.txt
  src/knotification.cpp
  src/knotificationmanager.cpp
  src/notifybyportal.h
  src/notifybysnore.cpp
  src/notifybysnore.h

To: brute4s99, broulik, sredman, vonreth, albertvaka
Cc: nicolasfella, pino, kde-frameworks-devel, LeGast00n, michaelh, ngraham, 
bruns


D21661: add snoretoast backend for KNotifications on Windows

2019-06-10 Thread Piyush Aggarwal
brute4s99 added a comment.


  Oh! Apologies, Pino. Actually I accidentally referred to you as toscanos from 
IRC, so I deleted that comment. I'll avoid deleting them from now on.

INLINE COMMENTS

> pino wrote in notifybysnore.cpp:38
> `QCoreApplication` is enough (see below)

Actually, I also use `applicationDisplayName()` as a fallback in case the 
notification does not have a `title()` set. One of the use cases was in KDE 
Connect itself (the pairing notification)

> vonreth wrote in notifybysnore.cpp:47
> I think the server should be a bit more unique,  what if two process of that 
> name exist?
> How about the full application path as a qHash?

Ooh! Sounds fun! I can do that :D

> vonreth wrote in notifybysnore.h:44
> The amount of notifications should be small in which case a map has a better 
> performance.

I referred to this: https://woboq.com/blog/qmap_qhash_benchmark.html

REPOSITORY
  R289 KNotifications

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

2019-06-10 Thread Piyush Aggarwal
brute4s99 updated this revision to Diff 59572.
brute4s99 added a comment.


  updated acc to new reviews by Pino and Hannah

REPOSITORY
  R289 KNotifications

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D21661?vs=59425=59572

BRANCH
  win32 (branched from master)

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

AFFECTED FILES
  src/CMakeLists.txt
  src/knotification.cpp
  src/knotificationmanager.cpp
  src/notifybyportal.h
  src/notifybysnore.cpp
  src/notifybysnore.h

To: brute4s99, broulik, sredman, vonreth, albertvaka
Cc: nicolasfella, pino, kde-frameworks-devel, LeGast00n, michaelh, ngraham, 
bruns


D21660: change audio dep logic wrt win32

2019-06-10 Thread Piyush Aggarwal
brute4s99 retitled this revision from "simplify conditions and remove audio 
dependency for win32" to "change audio dep logic wrt win32".

REPOSITORY
  R289 KNotifications

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

To: brute4s99, broulik
Cc: apol, nicolasfella, kde-frameworks-devel, LeGast00n, michaelh, ngraham, 
bruns


D21660: simplify conditions and remove audio dependency for win32

2019-06-10 Thread Piyush Aggarwal
brute4s99 updated this revision to Diff 59551.
brute4s99 added a comment.


  fixed indentation

REPOSITORY
  R289 KNotifications

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D21660?vs=59376=59551

BRANCH
  simplify (branched from master)

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

AFFECTED FILES
  CMakeLists.txt

To: brute4s99, broulik
Cc: apol, nicolasfella, kde-frameworks-devel, LeGast00n, michaelh, ngraham, 
bruns


D21661: add snoretoast backend for KNotifications on Windows

2019-06-08 Thread Piyush Aggarwal
brute4s99 marked 4 inline comments as done.

REPOSITORY
  R289 KNotifications

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

To: brute4s99, broulik, sredman, vonreth, albertvaka
Cc: pino, kde-frameworks-devel, LeGast00n, michaelh, ngraham, bruns


D21661: add snoretoast backend for KNotifications on Windows

2019-06-08 Thread Piyush Aggarwal
brute4s99 updated this revision to Diff 59425.
brute4s99 marked 2 inline comments as done.
brute4s99 added a comment.


  updated acc to review by Hannah

REPOSITORY
  R289 KNotifications

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D21661?vs=59401=59425

BRANCH
  win32 (branched from master)

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

AFFECTED FILES
  src/CMakeLists.txt
  src/knotification.cpp
  src/knotificationmanager.cpp
  src/notifybysnore.cpp
  src/notifybysnore.h

To: brute4s99, broulik, sredman, vonreth, albertvaka
Cc: pino, kde-frameworks-devel, LeGast00n, michaelh, ngraham, bruns


D21661: add snoretoast backend for KNotifications on Windows

2019-06-08 Thread Piyush Aggarwal
brute4s99 added a comment.


  updating acc to review by pino

REPOSITORY
  R289 KNotifications

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

To: brute4s99, broulik, sredman, vonreth, albertvaka
Cc: pino, kde-frameworks-devel, LeGast00n, michaelh, ngraham, bruns


D21661: add snoretoast backend for KNotifications on Windows

2019-06-08 Thread Piyush Aggarwal
brute4s99 marked an inline comment as done.

REPOSITORY
  R289 KNotifications

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

To: brute4s99, broulik, sredman, vonreth, albertvaka
Cc: pino, kde-frameworks-devel, LeGast00n, michaelh, ngraham, bruns


D21661: add snoretoast backend for KNotifications on Windows

2019-06-08 Thread Piyush Aggarwal
brute4s99 updated this revision to Diff 59401.
brute4s99 marked an inline comment as done.
brute4s99 added a comment.


  updated wrt review by toscanos

REPOSITORY
  R289 KNotifications

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D21661?vs=59379=59401

BRANCH
  win32 (branched from master)

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

AFFECTED FILES
  src/CMakeLists.txt
  src/knotification.cpp
  src/knotificationmanager.cpp
  src/notifybysnore.cpp
  src/notifybysnore.h

To: brute4s99, broulik, sredman, vonreth, albertvaka
Cc: pino, kde-frameworks-devel, LeGast00n, michaelh, ngraham, bruns


D21661: add snoretoast backend for KNotifications on Windows

2019-06-08 Thread Piyush Aggarwal
brute4s99 marked 22 inline comments as done and an inline comment as not done.
brute4s99 added a comment.


  update incoming

INLINE COMMENTS

> pino wrote in CMakeLists.txt:48-49
> why are these two needed? if snoretoast require them, then its cmake config 
> file must require them, so that the above `find_package(LibSnoreToast)` is 
> enough

the interfacing plugin (notifybysnore) requires QLocalServer and QLocalSocket, 
which are from Qt5Network.
Qt5Core has QProcess, QDir, QTemporaryDir and a bunch of other headers, which 
are required for the plugin as well.

> pino wrote in knotificationmanager.cpp:76
> this does not seem related to snoretoast

yeah, looks like it picked up changes from commits on master. apologies for 
this.

REPOSITORY
  R289 KNotifications

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

To: brute4s99, broulik, sredman, vonreth, albertvaka
Cc: pino, kde-frameworks-devel, LeGast00n, michaelh, ngraham, bruns


D21661: add snoretoast backend for KNotifications on Windows

2019-06-07 Thread Piyush Aggarwal
brute4s99 updated this revision to Diff 59379.
brute4s99 marked 2 inline comments as done.
brute4s99 added a comment.


  updated wrt review.

REPOSITORY
  R289 KNotifications

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D21661?vs=59377=59379

BRANCH
  win32 (branched from master)

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

AFFECTED FILES
  src/CMakeLists.txt
  src/knotification.cpp
  src/knotificationmanager.cpp
  src/notifybysnore.cpp
  src/notifybysnore.h
  src/snoretoastactions.h

To: brute4s99, broulik, sredman, vonreth, albertvaka
Cc: kde-frameworks-devel, LeGast00n, michaelh, ngraham, bruns


D21661: add snoretoast backend for KNotifications on Windows

2019-06-07 Thread Piyush Aggarwal
brute4s99 marked 5 inline comments as done.
brute4s99 added inline comments.

INLINE COMMENTS

> sredman wrote in CMakeLists.txt:43
> Do you know whether this requires MSVC, or can SnoreToast.exe be built with 
> MSVC and KNotifications be built with an unspecified compiler?

AFAIU it *should work fine with any compiler*, once SnoreToast is installed 
within the system one way or the other. (:

> sredman wrote in notifybysnore.cpp:44
> Did this end up being documented?

Yep! In 
https://piyushagg.home.blog/2019/06/07/gsoc19-project-milestone-1/

REPOSITORY
  R289 KNotifications

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

To: brute4s99, broulik, sredman, vonreth, albertvaka
Cc: kde-frameworks-devel, LeGast00n, michaelh, ngraham, bruns


D21659: add .vscode to .gitignore

2019-06-07 Thread Piyush Aggarwal
This revision was automatically updated to reflect the committed changes.
Closed by commit R289:e4461e0ba978: add .vscode to .gitignore (authored by 
brute4s99).

REPOSITORY
  R289 KNotifications

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D21659?vs=59375=59378

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

AFFECTED FILES
  .gitignore

To: brute4s99, broulik, nicolasfella
Cc: kde-frameworks-devel, LeGast00n, michaelh, ngraham, bruns


D21661: add snoretoast backend for KNotifications on Windows

2019-06-07 Thread Piyush Aggarwal
brute4s99 edited the summary of this revision.
brute4s99 added reviewers: broulik, sredman, vonreth, albertvaka.

REPOSITORY
  R289 KNotifications

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

To: brute4s99, broulik, sredman, vonreth, albertvaka
Cc: kde-frameworks-devel, LeGast00n, michaelh, ngraham, bruns


D21661: add snoretoast backend for KNotifications on Windows

2019-06-07 Thread Piyush Aggarwal
brute4s99 added a dependency: D21602: Add NSIS Script for KDE Connect.

REPOSITORY
  R289 KNotifications

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

To: brute4s99
Cc: kde-frameworks-devel, LeGast00n, michaelh, ngraham, bruns


D21660: simplify conditions and remove audio dependency for win32

2019-06-07 Thread Piyush Aggarwal
brute4s99 added a dependency: D21657: remove phonon from deps if building for 
win32.

REPOSITORY
  R289 KNotifications

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

To: brute4s99, broulik
Cc: kde-frameworks-devel, LeGast00n, michaelh, ngraham, bruns


D21661: add snoretoast backend for KNotifications on Windows

2019-06-07 Thread Piyush Aggarwal
brute4s99 created this revision.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
brute4s99 requested review of this revision.

REPOSITORY
  R289 KNotifications

BRANCH
  win32 (branched from master)

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

AFFECTED FILES
  src/CMakeLists.txt
  src/knotification.cpp
  src/knotificationmanager.cpp
  src/notifybysnore.cpp
  src/notifybysnore.h
  src/snoretoastactions.h

To: brute4s99
Cc: kde-frameworks-devel, LeGast00n, michaelh, ngraham, bruns


D21660: simplify conditions and remove audio dependency for win32

2019-06-07 Thread Piyush Aggarwal
brute4s99 retitled this revision from "simplify conditions" to "simplify 
conditions and remove audio dependency for win32".
brute4s99 edited the summary of this revision.
brute4s99 added a reviewer: broulik.

REPOSITORY
  R289 KNotifications

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

To: brute4s99, broulik
Cc: kde-frameworks-devel, LeGast00n, michaelh, ngraham, bruns


D21660: simplify conditions

2019-06-07 Thread Piyush Aggarwal
brute4s99 created this revision.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
brute4s99 requested review of this revision.

REPOSITORY
  R289 KNotifications

BRANCH
  simplify (branched from master)

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

AFFECTED FILES
  CMakeLists.txt

To: brute4s99
Cc: kde-frameworks-devel, LeGast00n, michaelh, ngraham, bruns


D21659: add .vscode to .gitignore

2019-06-07 Thread Piyush Aggarwal
brute4s99 created this revision.
brute4s99 added a reviewer: broulik.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
brute4s99 requested review of this revision.

REPOSITORY
  R289 KNotifications

BRANCH
  vscode (branched from master)

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

AFFECTED FILES
  .gitignore

To: brute4s99, broulik
Cc: kde-frameworks-devel, LeGast00n, michaelh, ngraham, bruns


<    1   2