D27951: Allow users to change dropAction to MoveAction through kdeglobals

[Tranter Madi]

trmdi added a comment.


  In D27951#645212 , @meven wrote:
  
  > I might take over to finish it eventually.
  
  
  Yes, please. I'm very glad that you support this. :)

REPOSITORY
  R241 KIO

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

To: trmdi, ngraham, dfaure, meven, #vdg, davidedmundson
Cc: bruns, elvisangelaccio, davidedmundson, meven, kde-frameworks-devel, 
LeGast00n, cblack, GB_2, michaelh, ngraham

D27951: Allow users to change dropAction to MoveAction through kdeglobals

[Tranter Madi]

trmdi abandoned this revision.
trmdi added a comment.


  In D27951#641173 , @meven wrote:
  
  > In D27951#639386 , @trmdi wrote:
  >
  > > In D27951#639350 , @meven 
wrote:
  > >
  > > > Anything that concerns a kdeconnet:/ url is handled by the kdeconnect 
ioslave, including the stat call made in `KIO::StatJob`.
  > > >  It is in the kdeconnect code base, the issue you point to is there.
  > >
  > >
  > > Thanks, got it. Then could we simply use KMountPoint instead of 
UDS_Device_ID ? KMountPoint seems more reliable.
  >
  >
  > But KMountPoint induces more cost (it makes a syscall and a bunch of 
parsing).
  >  So instead I would recommend fixing the issue in kdeconnect (and 
potentially other ioslave) as it will fix things here and elsewhere potentially 
too.
  >
  > Also `local = url.isLocalFile` should return false for kdeconnect in the 
first place.
  >  So I belive you can avoid using KMountPoint at all and use only stat.
  
  
  No, kde connect files has local urls.
  Fixing every ioslave is too difficult. 
  So I should drop this idea.

REPOSITORY
  R241 KIO

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

To: trmdi, ngraham, dfaure, meven, #vdg, davidedmundson
Cc: bruns, elvisangelaccio, davidedmundson, meven, kde-frameworks-devel, 
LeGast00n, cblack, GB_2, michaelh, ngraham

D27951: Allow users to change dropAction to MoveAction through kdeglobals

[Tranter Madi]

trmdi added a comment.


  In D27951#639350 , @meven wrote:
  
  > Anything that concerns a kdeconnet:/ url is handled by the kdeconnect 
ioslave, including the stat call made in `KIO::StatJob`.
  >  It is in the kdeconnect code base, the issue you point to is there.
  
  
  Thanks, got it. Then could we simply use KMountPoint instead of UDS_Device_ID 
? KMountPoint seems more reliable.

REPOSITORY
  R241 KIO

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

To: trmdi, ngraham, dfaure, meven, #vdg, davidedmundson
Cc: elvisangelaccio, davidedmundson, meven, kde-frameworks-devel, LeGast00n, 
cblack, GB_2, michaelh, ngraham, bruns

D27951: Allow users to change dropAction to MoveAction through kdeglobals

[Tranter Madi]

trmdi added a comment.


  In D27951#637690 , @meven wrote:
  
  > Look into kde connect kioslave.
  >  That does not concern this diff much though.
  
  
  I don't understand this. I'm using KMountPoint to check if source and dest 
are on the same partion, it's working as expected.
  But if I switch to use `KIO::StatJob`, it will fail, because for the file 
inside a KDE Connect directory, `KIO::UDSEntry::UDS_DEVICE_ID` return the same 
value with the root partion.

REPOSITORY
  R241 KIO

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

To: trmdi, ngraham, dfaure, meven, #vdg, davidedmundson
Cc: elvisangelaccio, davidedmundson, meven, kde-frameworks-devel, LeGast00n, 
cblack, GB_2, michaelh, ngraham, bruns

D27951: Allow users to change dropAction to MoveAction through kdeglobals

[Tranter Madi]

trmdi added a comment.


  In D27951#637038 , @dfaure wrote:
  
  > Indeed. Bug in kio_file, fixed in D28388 
.
  
  
  Ok, it works for symlinks now.
  How about normal files in a KDE Connect's mountpoint? It still have the same 
device id with the root partition.

REPOSITORY
  R241 KIO

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

To: trmdi, ngraham, dfaure, meven, #vdg, davidedmundson
Cc: elvisangelaccio, davidedmundson, meven, kde-frameworks-devel, LeGast00n, 
cblack, GB_2, michaelh, ngraham, bruns

D27951: Allow users to change dropAction to MoveAction through kdeglobals

[Tranter Madi]

trmdi added a comment.


  In D27951#636980 , @dfaure wrote:
  
  > Seems to work for symlinks here. We *don't* want it to follow symlinks, 
right?
  >
  > I have /opt a symlink to /d/opt, where /d is on another partition
  >
  > $ kioclient5 openProperties /opt
  >  DEVICE 65025
  >
  > $ kioclient5 openProperties /d/opt 
  >  DEVICE 65028
  >
  > Different, as expected.
  
  
  No, we want /opt and /d/opt to have the same device id.

REPOSITORY
  R241 KIO

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

To: trmdi, ngraham, dfaure, meven, #vdg, davidedmundson
Cc: elvisangelaccio, davidedmundson, meven, kde-frameworks-devel, LeGast00n, 
cblack, GB_2, michaelh, ngraham, bruns

D27951: Allow users to change dropAction to MoveAction through kdeglobals

[Tranter Madi]

trmdi added a comment.


  In D27951#636646 , @dfaure wrote:
  
  > Note sure how you're testing UDS_* but you need to pass KIO::StatInode to 
KIO::statDetails for inode and device ID to be filled in.
  >
  > Testcase: apply http://www.davidfaure.fr/2020/uds_device_test.diff then 
`kioclient5 openProperties ~/.bashrc`
  
  
  
  
  > But at least it should work for regular files and directories, right?
  
  Yes, it does.
  But it still doesn't work as expected for symlink and KDE Connect directory. 
These 2 cases have the same device id with the root partition. Do I do it wrong?

REPOSITORY
  R241 KIO

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

To: trmdi, ngraham, dfaure, meven, #vdg, davidedmundson
Cc: elvisangelaccio, davidedmundson, meven, kde-frameworks-devel, LeGast00n, 
cblack, GB_2, michaelh, ngraham, bruns

D27951: Allow users to change dropAction to MoveAction through kdeglobals

[Tranter Madi]

trmdi updated this revision to Diff 78691.
trmdi added a comment.


  Improve the logic a bit.

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27951?vs=78427=78691

BRANCH
  add-dndToMove (branched from master)

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

AFFECTED FILES
  src/widgets/dropjob.cpp

To: trmdi, ngraham, dfaure, meven, #vdg, davidedmundson
Cc: elvisangelaccio, davidedmundson, meven, kde-frameworks-devel, LeGast00n, 
cblack, GB_2, michaelh, ngraham, bruns

D27951: Allow users to change dropAction to MoveAction through kdeglobals

[Tranter Madi]

trmdi added a comment.


  In D27951#635596 , @dfaure wrote:
  
  > Symlinks are a bit special, we could add a way to ask kio_file not to 
follow them.
  >
  > But at least it should work for regular files and directories, right?
  >
  > Anyhow, we don't have a kio_file result here so I guess my point is moot.
  
  
  No. See this:
  
[ "UDS_NAME"="/run/media/trmdi/7E7A4CDA7A4C9137" 
"UDS_SIZE"=4607182418800017408 "UDS_DEVICE_ID"=0 "UDS_INODE"=0 
"UDS_FILE_TYPE"=20480 "UDS_ACCESS"=864 "UDS_MODIFICATION_TIME"=140181869572387 
"UDS_ACCESS_TIME"=0 "UDS_USER"="" "UDS_GROUP"="" ]
"/run/media/trmdi/7E7A4CDA7A4C9137"  -  "/dev/sda2"

[ "UDS_NAME"="/home/trmdi/Downloads" "UDS_SIZE"=4607182418800017408 
"UDS_DEVICE_ID"=0 "UDS_INODE"=0 "UDS_FILE_TYPE"=20480 "UDS_ACCESS"=864 
"UDS_MODIFICATION_TIME"=140181869572387 "UDS_ACCESS_TIME"=0 "UDS_USER"="" 
"UDS_GROUP"="" ]
"/home/thien/Downloads"  -  "/dev/sda1"

REPOSITORY
  R241 KIO

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

To: trmdi, ngraham, dfaure, meven, #vdg, davidedmundson
Cc: elvisangelaccio, davidedmundson, meven, kde-frameworks-devel, LeGast00n, 
cblack, GB_2, michaelh, ngraham, bruns

D27951: Allow users to change dropAction to MoveAction through kdeglobals

[Tranter Madi]

trmdi added a comment.


  In D27951#633820 , @dfaure wrote:
  
  > Implementation note: the fastest way to know if two items are on the same 
partition is to compare KIO::UDSEntry::UDS_DEVICE_ID values. But unfortunately 
it looks like this code takes URLs, not proper KFileItems created from a 
kio_file job... I guess that means statbuf.st_dev directly, on Unix?
  
  
  `KIO::UDSEntry::UDS_DEVICE_ID` doesn't provide wanted info in this case. See 
my example:
  
[ "UDS_NAME"="/home/trmdi/Downloads/mount/Data" "UDS_SIZE"=0 
"UDS_DEVICE_ID"=4607182418800017408 "UDS_INODE"=30008928 "UDS_FILE_TYPE"=0 
"UDS_ACCESS"=0 "UDS_MODIFICATION_TIME"=140192419498368 
"UDS_ACCESS_TIME"=22970816 "UDS_USER"="root" "UDS_GROUP"="" ]

[ "UDS_NAME"="/home/trmdi/Downloads/mount" "UDS_SIZE"=0 
"UDS_DEVICE_ID"=4607182418800017408 "UDS_INODE"=30008928 "UDS_FILE_TYPE"=0 
"UDS_ACCESS"=0 "UDS_MODIFICATION_TIME"=140192419498368 
"UDS_ACCESS_TIME"=22970816 "UDS_USER"="root" "UDS_GROUP"="" ]
  
  (Data is a symlink to a directory in /dev/sda2, mount is in /dev/sda1)
  
  While `KMointPoint` is still right:
  
"/home/trmdi/Downloads/mount"  -  "/dev/sda1"
"/home/trmdi/Downloads/mount/Data"  -  "/dev/sda2"

REPOSITORY
  R241 KIO

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

To: trmdi, ngraham, dfaure, meven, #vdg, davidedmundson
Cc: elvisangelaccio, davidedmundson, meven, kde-frameworks-devel, LeGast00n, 
cblack, GB_2, michaelh, ngraham, bruns

D27951: Allow users to change dropAction to MoveAction through kdeglobals

[Tranter Madi]

trmdi updated this revision to Diff 78427.
trmdi added a comment.


  Improve the symlink case, any symlink is considered as on the same device 
with any destination.

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27951?vs=78426=78427

BRANCH
  add-dndToMove (branched from master)

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

AFFECTED FILES
  src/widgets/dropjob.cpp

To: trmdi, ngraham, dfaure, meven, #vdg, davidedmundson
Cc: elvisangelaccio, davidedmundson, meven, kde-frameworks-devel, LeGast00n, 
cblack, GB_2, michaelh, ngraham, bruns

D27951: Allow users to change dropAction to MoveAction through kdeglobals

[Tranter Madi]

trmdi added inline comments.

INLINE COMMENTS

> davidedmundson wrote in dropjob.cpp:286
> Why QStorageInfo? We're in kio. There's a KMountPoint which is similar
> 
> I suspect this is a recursive list up the tree resolving symlinks. This will 
> mean blocking stat calls, so this somewhat undermines a lot of the recent 
> work in that field.

> I suspect this is a recursive list up the tree resolving symlinks. This will 
> mean blocking stat calls, so this somewhat undermines a lot of the recent 
> work in that field.

I don't understand this. Could you please explain a bit more?

> davidedmundson wrote in dropjob.cpp:287
> If destination is a symlink on the same partition but that symlink points to 
> another partition what happens?
> 
> I haven't checked myself, but if we are doing this I need us to be super 
> super sure.

They are considered as different partitions, I checked.

> davidedmundson wrote in dropjob.cpp:309
> This will trigger when both storage devices are invalid, which isn't what we 
> want.

I don't understand this. How could devices be invalid if we are dropping valid 
urls to a valid destUrl?

REPOSITORY
  R241 KIO

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

To: trmdi, ngraham, dfaure, meven, #vdg, davidedmundson
Cc: elvisangelaccio, davidedmundson, meven, kde-frameworks-devel, LeGast00n, 
cblack, GB_2, michaelh, ngraham, bruns

D27951: Allow users to change dropAction to MoveAction through kdeglobals

[Tranter Madi]

trmdi updated this revision to Diff 78426.
trmdi marked 2 inline comments as done.
trmdi added a comment.


  - Always move files if all of them are symlinks
  - Use KMountPoint

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27951?vs=77511=78426

BRANCH
  add-dndToMove (branched from master)

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

AFFECTED FILES
  src/widgets/dropjob.cpp

To: trmdi, ngraham, dfaure, meven, #vdg, davidedmundson
Cc: elvisangelaccio, davidedmundson, meven, kde-frameworks-devel, LeGast00n, 
cblack, GB_2, michaelh, ngraham, bruns

D27951: Allow users to change dropAction to MoveAction through kdeglobals

[Tranter Madi]

trmdi added a comment.


  Friendly ping.

REPOSITORY
  R241 KIO

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

To: trmdi, ngraham, dfaure, meven, #vdg, davidedmundson
Cc: elvisangelaccio, davidedmundson, meven, kde-frameworks-devel, LeGast00n, 
cblack, GB_2, michaelh, ngraham, bruns

D27951: Allow users to change dropAction to MoveAction through kdeglobals

[Tranter Madi]

trmdi added a comment.


  In D27951#627849 , 
@elvisangelaccio wrote:
  
  > > With dndToMove=true in kdeglobals, drag files will move them without 
showing the menu. (holding Shift shows it)
  >
  > Are we going to expose this setting in Plasma?
  
  
  Yes: https://phabricator.kde.org/D27998

REPOSITORY
  R241 KIO

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

To: trmdi, ngraham, dfaure, meven, #vdg, davidedmundson
Cc: elvisangelaccio, davidedmundson, meven, kde-frameworks-devel, LeGast00n, 
cblack, GB_2, michaelh, ngraham, bruns

D27951: Allow users to change dropAction to MoveAction through kdeglobals

[Tranter Madi]

trmdi added a comment.


  In D27951#627283 , @ngraham wrote:
  
  > My interpretation of the use case is that a user who chooses dndToMove 
prefers Move over Copy and Link, but not necessarily Extract or Set as 
Wallpaper, since those are context-specific options that exist because they are 
potentially more useful than move, copy, or link.
  >
  > > If he wants to see the menu, he could hold the Shift modifier key.
  >
  > If he doesn't know that there are other options, then he has no way of 
knowing that he should hold down the shift key to see them.
  
  
  What if he is familiar with Windows behavior, or he likes the move action 
most? In that case the menu still is an annoyance to them.
  When he chose that, it's likely he likes that the most, and he dislikes/ 
doesn't care about others.
  I think the current implementation is balanced.

REPOSITORY
  R241 KIO

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

To: trmdi, ngraham, dfaure, meven, #vdg, davidedmundson
Cc: davidedmundson, meven, kde-frameworks-devel, LeGast00n, cblack, GB_2, 
michaelh, ngraham, bruns

D27951: Allow users to change dropAction to MoveAction through kdeglobals

[Tranter Madi]

trmdi added a comment.


  In D27951#627041 , @ngraham wrote:
  
  > In D27951#626346 , @trmdi wrote:
  >
  > > When drop an image to the desktop in the FolderView layout, there would 
be no menu, so users can not change the wallpaper by drag in this case. 
How do you think about this?
  >
  >
  > In fact there are other options that I didn't think about. For example when 
dragging-and-dropping an archive, there's an "extract here" item that would be 
hidden with this patch.
  >
  > Could we maybe generalize the logic so that it always shows the full drop 
menu if there are any custom items in it like "Expand here" or "Set as 
Wallpaper"? That way it will only move if you have the setting turned on and 
there really are no other ambiguous options.
  
  
  I think the user has to choose. When he chose dndToMove, that means he 
prefers this behavior over others.
  If he wants to see the menu, he could hold the Shift modifier key.

REPOSITORY
  R241 KIO

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

To: trmdi, ngraham, dfaure, meven, #vdg, davidedmundson
Cc: davidedmundson, meven, kde-frameworks-devel, LeGast00n, cblack, GB_2, 
michaelh, ngraham, bruns

D27951: Allow users to change dropAction to MoveAction through kdeglobals

[Tranter Madi]

trmdi updated this revision to Diff 77511.
trmdi added a comment.


  Minor improvement

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27951?vs=77508=77511

BRANCH
  add-dndToMove (branched from master)

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

AFFECTED FILES
  src/widgets/dropjob.cpp

To: trmdi, ngraham, dfaure, meven, #vdg, davidedmundson
Cc: davidedmundson, meven, kde-frameworks-devel, LeGast00n, cblack, GB_2, 
michaelh, ngraham, bruns

D27951: Allow users to change dropAction to MoveAction through kdeglobals

[Tranter Madi]

trmdi edited the summary of this revision.
trmdi edited the test plan for this revision.

REPOSITORY
  R241 KIO

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

To: trmdi, ngraham, dfaure, meven, #vdg, davidedmundson
Cc: davidedmundson, meven, kde-frameworks-devel, LeGast00n, cblack, GB_2, 
michaelh, ngraham, bruns

D27951: Allow users to change dropAction to MoveAction through kdeglobals

[Tranter Madi]

trmdi updated this revision to Diff 77508.
trmdi added a comment.


  Allow to explicitly show the menu when holding Shift

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27951?vs=77433=77508

BRANCH
  add-dndToMove (branched from master)

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

AFFECTED FILES
  src/widgets/dropjob.cpp

To: trmdi, ngraham, dfaure, meven, #vdg, davidedmundson
Cc: davidedmundson, meven, kde-frameworks-devel, LeGast00n, cblack, GB_2, 
michaelh, ngraham, bruns

D27951: Allow users to change dropAction to MoveAction through kdeglobals

[Tranter Madi]

trmdi added a comment.


  In D27951#626135 , @ngraham wrote:
  
  > it needs more discussion first...
  
  
  When drop an image to the desktop in the FolderView layout, there would be no 
menu, so users can not change the wallpaper in this case. How do you think 
about this?

REPOSITORY
  R241 KIO

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

To: trmdi, ngraham, dfaure, meven, #vdg, davidedmundson
Cc: davidedmundson, meven, kde-frameworks-devel, LeGast00n, cblack, GB_2, 
michaelh, ngraham, bruns

D27951: Allow users to change dropAction to MoveAction through kdeglobals

[Tranter Madi]

trmdi added a comment.


  In D27951#625881 , @ngraham wrote:
  
  > Excellent. The behavior seems correct to me now. There are a few more 
issues I've like to bring up:
  >
  > - While dragging, the cursor always shows a "copying" icon, even though 
with the patch local drags result in a move. Anytime the file will be moved 
without showing the drop menu, it should show the move cursor, which looks like 
a grabbing hand
  
  
  Because this was set from QDrag, we have to patch Dolphin for this.
  
  > - When dragging and dropping a file to a writable location on another 
partition, we should always show the drop menu. See the discussion in 
https://bugs.kde.org/show_bug.cgi?id=392531. I don't think there's any chance 
that David will accept the patch in its current state if it moves by default 
across partitions/disks without asking first.
  
  Done, please test it.

REPOSITORY
  R241 KIO

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

To: trmdi, ngraham, dfaure, meven
Cc: meven, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, 
bruns

D27951: Allow users to change dropAction to MoveAction through kdeglobals

[Tranter Madi]

trmdi updated this revision to Diff 77433.
trmdi added a comment.


  Apply only when all the sources and the dest are on a same device.

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27951?vs=77391=77433

BRANCH
  add-dndToMove (branched from master)

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

AFFECTED FILES
  src/widgets/dropjob.cpp

To: trmdi, ngraham, dfaure, meven
Cc: meven, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, 
bruns

D27951: Allow users to change dropAction to MoveAction through kdeglobals

[Tranter Madi]

trmdi updated this revision to Diff 77391.
trmdi added a comment.


  Code style

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27951?vs=77390=77391

BRANCH
  add-dndToMove (branched from master)

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

AFFECTED FILES
  src/widgets/dropjob.cpp

To: trmdi, ngraham, dfaure, meven
Cc: meven, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, 
bruns

D27951: Allow users to change dropAction to MoveAction through kdeglobals

[Tranter Madi]

trmdi updated this revision to Diff 77390.
trmdi added a comment.


  Code style

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27951?vs=77389=77390

BRANCH
  add-dndToMove (branched from master)

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

AFFECTED FILES
  src/widgets/dropjob.cpp

To: trmdi, ngraham, dfaure, meven
Cc: meven, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, 
bruns

D27951: Allow users to change dropAction to MoveAction through kdeglobals

[Tranter Madi]

trmdi updated this revision to Diff 77389.
trmdi added a comment.


  Apply only when all items are local.

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27951?vs=77351=77389

BRANCH
  add-dndToMove (branched from master)

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

AFFECTED FILES
  src/widgets/dropjob.cpp

To: trmdi, ngraham, dfaure, meven
Cc: meven, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, 
bruns

D27951: Allow users to change dropAction to MoveAction through kdeglobals

[Tranter Madi]

trmdi added a comment.


  In D27951#625470 , @ngraham wrote:
  
  > However dragging a URL still doesn't show the menu; it just downloads the 
entire page. We need the drop menu in this case because it's probably more 
common to want to create a link to the page than it is to download the entire 
thing.
  >
  > If introspecting the mimetype of every URL is too costly, could we maybe 
short-circuit the logic if the URL scheme is HTTP or HTTPS?
  
  
  Shouldn't this need another patch?
  
  > Also I notice that the cursor still shows a plus sign icon when dragging a 
local file, even though it will be moved, not copied.
  
  I think this need a patch in Dolphin. Could you help me to do it?

REPOSITORY
  R241 KIO

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

To: trmdi, ngraham, dfaure, meven
Cc: meven, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, 
bruns

D27951: Allow users to change dropAction to MoveAction through kdeglobals

[Tranter Madi]

trmdi updated this revision to Diff 77351.
trmdi added a comment.


  Move comment

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27951?vs=77350=77351

BRANCH
  add-dndToMove (branched from master)

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

AFFECTED FILES
  src/widgets/dropjob.cpp

To: trmdi, ngraham, dfaure, meven
Cc: meven, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, 
bruns

D27951: Allow users to change dropAction to MoveAction through kdeglobals

[Tranter Madi]

trmdi retitled this revision from "Allow users to change dropAction to 
MoveAction through workspace kcm" to "Allow users to change dropAction to 
MoveAction through kdeglobals".

REPOSITORY
  R241 KIO

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

To: trmdi, ngraham, dfaure, meven
Cc: meven, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, 
bruns

D27951: Allow users to change dropAction to MoveAction through kdeglobals

[Tranter Madi]

trmdi marked an inline comment as done.

REPOSITORY
  R241 KIO

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

To: trmdi, ngraham, dfaure, meven
Cc: meven, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, 
bruns

D27951: Allow users to change dropAction to MoveAction through workspace kcm

[Tranter Madi]

trmdi updated this revision to Diff 77350.
trmdi added a comment.


  Address meven's comment

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27951?vs=77349=77350

BRANCH
  add-dndToMove (branched from master)

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

AFFECTED FILES
  src/widgets/dropjob.cpp

To: trmdi, ngraham, dfaure, meven
Cc: meven, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, 
bruns

D27951: Allow users to change dropAction to MoveAction through workspace kcm

[Tranter Madi]

trmdi retitled this revision from "Allow users to change dropAction to 
MoveAction through workspace-option kcm" to "Allow users to change dropAction 
to MoveAction through workspace kcm".

REPOSITORY
  R241 KIO

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

To: trmdi, ngraham, dfaure, meven
Cc: meven, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, 
bruns

D27951: Allow users to change dropAction to MoveAction through workspace-option kcm

[Tranter Madi]

trmdi marked an inline comment as done.

REPOSITORY
  R241 KIO

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

To: trmdi, ngraham, dfaure, meven
Cc: meven, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, 
bruns

D27951: Allow users to change dropAction to MoveAction through workspace-option kcm

[Tranter Madi]

trmdi updated this revision to Diff 77349.
trmdi added a comment.


  Move fileItem back to the local scope

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27951?vs=77329=77349

BRANCH
  add-dndToMove (branched from master)

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

AFFECTED FILES
  src/widgets/dropjob.cpp

To: trmdi, ngraham, dfaure, meven
Cc: meven, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, 
bruns

D27951: Allow users to change dropAction to MoveAction through workspace-option kcm

[Tranter Madi]

trmdi updated this revision to Diff 77329.
trmdi added a comment.


  - Check possible actions in determineDropAction

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D27951?vs=77298=77329

BRANCH
  add-dndToMove (branched from master)

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

AFFECTED FILES
  src/widgets/dropjob.cpp

To: trmdi, ngraham, dfaure
Cc: meven, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, 
bruns

D27961: GIT_SILENT Upgrade ECM and KF5 version requirements for 5.68.0 release.

[Tranter Madi]

trmdi created this revision.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
trmdi requested review of this revision.

REPOSITORY
  R241 KIO

BRANCH
  master

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

AFFECTED FILES
  CMakeLists.txt
  src/widgets/dropjob.cpp

To: trmdi
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D27961: GIT_SILENT Upgrade ECM and KF5 version requirements for 5.68.0 release.

[Tranter Madi]

trmdi abandoned this revision.

REPOSITORY
  R241 KIO

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

To: trmdi
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D27951: Allow users to change dropAction to MoveAction through workspace-option kcm

[Tranter Madi]

trmdi planned changes to this revision.

REPOSITORY
  R241 KIO

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

To: trmdi, ngraham, dfaure
Cc: meven, kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, 
bruns

D27951: Allow users to change dropAction to MoveAction through workspace-option kcm

[Tranter Madi]

trmdi edited the test plan for this revision.

REPOSITORY
  R241 KIO

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

To: trmdi, ngraham, dfaure
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D27951: Allow users to change dropAction to MoveAction through workspace-option kcm

[Tranter Madi]

trmdi added reviewers: ngraham, dfaure.

REPOSITORY
  R241 KIO

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

To: trmdi, ngraham, dfaure
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D27951: Allow users to change dropAction to MoveAction through workspace-option kcm

[Tranter Madi]

trmdi created this revision.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
trmdi requested review of this revision.

REVISION SUMMARY
  Lots of users request to be able to change the default behavior of the 
drag action to MoveAction.

TEST PLAN
  With dndTomove=true in kdeglobals, drag files will move them without 
showing the menu when no key modifier is pressed.

REPOSITORY
  R241 KIO

BRANCH
  add-dndToMove (branched from master)

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

AFFECTED FILES
  src/widgets/dropjob.cpp

To: trmdi
Cc: kde-frameworks-devel, LeGast00n, cblack, GB_2, michaelh, ngraham, bruns

D27122: Tweaked contrast effect values, adjusted transparency accordingly, switched from Background to ViewBackground

[Tranter Madi]

trmdi added a comment.


  Hmm, I think you need to rebase to remove unrelated files in this patch e.g. 
`dropjob.cpp` ... ?

REPOSITORY
  R242 Plasma Framework (Library)

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

To: niccolove, ndavis, ngraham, cblack
Cc: trmdi, cblack, ndavis, filipf, ngraham, davidedmundson, 
kde-frameworks-devel, manueljlin, Orage, LeGast00n, The-Feren-OS-Dev, 
konkinartem, ian, jguidon, Ghost6, jraleigh, zachus, MrPepe, fbampaloukas, 
squeakypancakes, alexde, IohannesPetros, GB_2, trickyricky26, ragreen, 
michaelh, crozbo, ZrenBot, firef, bruns, skadinna, himcesjf, lesliezhai, 
ali-mohamed, jensreuterberg, aaronhoneycutt, abetts, sebas, apol, ahiemstra, 
mbohlender, mart

D26691: Optimize code when dropping files into the desktop

[Tranter Madi]

trmdi added a comment.


  In D26691#605802 , @jjazeix wrote:
  
  > Compilation fails: 
https://build.kde.org/view/Failing/job/Plasma/job/plasma-framework/job/kf5-qt5%20SUSEQt5.12/288/console
  >  Missing the new file in the CMakeLists.txt 
(https://cgit.kde.org/plasma-framework.git/tree/src/scriptengines/qml/CMakeLists.txt#n9)?
  >
  > Fixed in 
https://cgit.kde.org/plasma-framework.git/commit/?id=2ea27fb06887237d49c63f6b7e74702bffef57ea
  
  
  Thanks. :)

REPOSITORY
  R242 Plasma Framework (Library)

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

To: trmdi, #plasma, mart, broulik, #vdg, davidedmundson
Cc: jjazeix, davidedmundson, anthonyfieroni, #plasma, kde-frameworks-devel, 
LeGast00n, GB_2, michaelh, ngraham, bruns

D26691: Optimize code when dropping files into the desktop

[Tranter Madi]

This revision was automatically updated to reflect the committed changes.
Closed by commit R242:f8be3ea7102f: Optimize code when dropping files into the 
desktop (authored by trmdi).

REPOSITORY
  R242 Plasma Framework (Library)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D26691?vs=74672=74975

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

AFFECTED FILES
  src/scriptengines/qml/plasmoid/containmentinterface.cpp
  src/scriptengines/qml/plasmoid/containmentinterface.h
  src/scriptengines/qml/plasmoid/dropmenu.cpp
  src/scriptengines/qml/plasmoid/dropmenu.h

To: trmdi, #plasma, mart, broulik, #vdg, davidedmundson
Cc: davidedmundson, anthonyfieroni, #plasma, kde-frameworks-devel, LeGast00n, 
GB_2, michaelh, ngraham, bruns

D26691: Optimize code when dropping files into the desktop

[Tranter Madi]

trmdi added a comment.


  @mart
  Do you have any objection?

REPOSITORY
  R242 Plasma Framework (Library)

BRANCH
  improve-file-drop-menu (branched from master)

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

To: trmdi, #plasma, mart, broulik, #vdg, davidedmundson
Cc: davidedmundson, anthonyfieroni, #plasma, kde-frameworks-devel, LeGast00n, 
GB_2, michaelh, ngraham, bruns

D26691: Optimize code when dropping files into the desktop

[Tranter Madi]

trmdi updated this revision to Diff 74672.
trmdi added a comment.


  - Rebase

REPOSITORY
  R242 Plasma Framework (Library)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D26691?vs=74036=74672

BRANCH
  improve-file-drop-menu (branched from master)

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

AFFECTED FILES
  src/scriptengines/qml/plasmoid/containmentinterface.cpp
  src/scriptengines/qml/plasmoid/containmentinterface.h
  src/scriptengines/qml/plasmoid/dropmenu.cpp
  src/scriptengines/qml/plasmoid/dropmenu.h

To: trmdi, #plasma, mart, broulik, #vdg, davidedmundson
Cc: davidedmundson, anthonyfieroni, #plasma, kde-frameworks-devel, LeGast00n, 
GB_2, michaelh, ngraham, bruns

D26484: Add KIO::DropJobFlag to allow manually showing the menu

[Tranter Madi]

trmdi added a dependent revision: D26514: Delay popup for containments.

REPOSITORY
  R241 KIO

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

To: trmdi, #frameworks, davidedmundson, elvisangelaccio, mart, dfaure
Cc: ngraham, broulik, anthonyfieroni, kde-frameworks-devel, LeGast00n, GB_2, 
michaelh, bruns

D26691: Optimize code when dropping files into the desktop

[Tranter Madi]

trmdi edited the summary of this revision.

REPOSITORY
  R242 Plasma Framework (Library)

BRANCH
  improve-file-drop-menu

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

To: trmdi, #plasma, mart, broulik, #vdg, davidedmundson
Cc: davidedmundson, anthonyfieroni, #plasma, kde-frameworks-devel, LeGast00n, 
GB_2, michaelh, ngraham, bruns

D26691: Optimize code when dropping files into the desktop

[Tranter Madi]

trmdi marked an inline comment as done.

REPOSITORY
  R242 Plasma Framework (Library)

BRANCH
  improve-file-drop-menu

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

To: trmdi, #plasma, mart, broulik, #vdg, davidedmundson
Cc: davidedmundson, anthonyfieroni, #plasma, kde-frameworks-devel, LeGast00n, 
GB_2, michaelh, ngraham, bruns

D26691: Optimize code when dropping files into the desktop

[Tranter Madi]

trmdi updated this revision to Diff 74036.
trmdi marked 2 inline comments as done.
trmdi added a comment.


  - Address comment

REPOSITORY
  R242 Plasma Framework (Library)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D26691?vs=74030=74036

BRANCH
  improve-file-drop-menu

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

AFFECTED FILES
  src/scriptengines/qml/plasmoid/containmentinterface.cpp
  src/scriptengines/qml/plasmoid/containmentinterface.h
  src/scriptengines/qml/plasmoid/dropmenu.cpp
  src/scriptengines/qml/plasmoid/dropmenu.h

To: trmdi, #plasma, mart, broulik, #vdg, davidedmundson
Cc: davidedmundson, anthonyfieroni, #plasma, kde-frameworks-devel, LeGast00n, 
GB_2, michaelh, ngraham, bruns

D26691: Optimize code when dropping files into the desktop

[Tranter Madi]

trmdi marked 3 inline comments as done.
trmdi added inline comments.

INLINE COMMENTS

> davidedmundson wrote in containmentinterface.cpp:548
> can you be sure urls has at least 1 at this point?

Yes, because all `clearDataForMimeJob` are called from inside of 
`mimeTypeRetrieved` which is only called from here: 
https://phabricator.kde.org/source/plasma-framework/browse/master/src/scriptengines/qml/plasmoid/containmentinterface.cpp$454

REPOSITORY
  R242 Plasma Framework (Library)

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

To: trmdi, #plasma, mart, broulik, #vdg, davidedmundson
Cc: davidedmundson, anthonyfieroni, #plasma, kde-frameworks-devel, LeGast00n, 
GB_2, michaelh, ngraham, bruns

D26691: Optimize code when dropping files into the desktop

[Tranter Madi]

trmdi updated this revision to Diff 74030.
trmdi added a comment.


  - Code style

REPOSITORY
  R242 Plasma Framework (Library)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D26691?vs=74025=74030

BRANCH
  improve-file-drop-menu

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

AFFECTED FILES
  src/scriptengines/qml/plasmoid/containmentinterface.cpp
  src/scriptengines/qml/plasmoid/containmentinterface.h
  src/scriptengines/qml/plasmoid/dropmenu.cpp
  src/scriptengines/qml/plasmoid/dropmenu.h

To: trmdi, #plasma, mart, broulik, #vdg, davidedmundson
Cc: davidedmundson, anthonyfieroni, #plasma, kde-frameworks-devel, LeGast00n, 
GB_2, michaelh, ngraham, bruns

D26691: Optimize code when dropping files into the desktop

[Tranter Madi]

trmdi updated this revision to Diff 74025.
trmdi added a comment.


  - Handle the case in which the menu is not shown

REPOSITORY
  R242 Plasma Framework (Library)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D26691?vs=73815=74025

BRANCH
  improve-file-drop-menu

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

AFFECTED FILES
  src/scriptengines/qml/plasmoid/containmentinterface.cpp
  src/scriptengines/qml/plasmoid/containmentinterface.h
  src/scriptengines/qml/plasmoid/dropmenu.cpp
  src/scriptengines/qml/plasmoid/dropmenu.h

To: trmdi, #plasma, mart, broulik, #vdg, davidedmundson
Cc: davidedmundson, anthonyfieroni, #plasma, kde-frameworks-devel, LeGast00n, 
GB_2, michaelh, ngraham, bruns

D26691: Optimize code when dropping files into the desktop

[Tranter Madi]

trmdi added inline comments.

INLINE COMMENTS

> davidedmundson wrote in dropmenu.cpp:48
> where does this menu get deleted? It seems to leak

It is set to be deleted on close, at line 43.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: trmdi, #plasma, mart, broulik, #vdg, davidedmundson
Cc: davidedmundson, anthonyfieroni, #plasma, kde-frameworks-devel, LeGast00n, 
GB_2, michaelh, ngraham, bruns

D26691: Optimize code when dropping files into the desktop

[Tranter Madi]

trmdi updated this revision to Diff 73815.
trmdi added a comment.


  - Add DropMenu class to combine choices/dropjobMenu
  - Only 1 menu can be created
  - Fix some possible memory leaks

REPOSITORY
  R242 Plasma Framework (Library)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D26691?vs=73716=73815

BRANCH
  improve-file-drop-menu

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

AFFECTED FILES
  src/scriptengines/qml/plasmoid/containmentinterface.cpp
  src/scriptengines/qml/plasmoid/containmentinterface.h
  src/scriptengines/qml/plasmoid/dropmenu.cpp
  src/scriptengines/qml/plasmoid/dropmenu.h

To: trmdi, #plasma, mart, broulik, #vdg, davidedmundson
Cc: davidedmundson, anthonyfieroni, #plasma, kde-frameworks-devel, LeGast00n, 
GB_2, michaelh, ngraham, bruns

D26691: Optimize code when dropping files into the desktop

[Tranter Madi]

trmdi updated this revision to Diff 73716.
trmdi added a comment.


  - Change the cursor to the busy state when the first file is not local

REPOSITORY
  R242 Plasma Framework (Library)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D26691?vs=73694=73716

BRANCH
  improve-file-drop-menu

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

AFFECTED FILES
  src/scriptengines/qml/plasmoid/containmentinterface.cpp
  src/scriptengines/qml/plasmoid/containmentinterface.h

To: trmdi, #plasma, mart, broulik, #vdg
Cc: anthonyfieroni, #plasma, kde-frameworks-devel, LeGast00n, GB_2, michaelh, 
ngraham, bruns

D26691: Optimize code when dropping files into the desktop

[Tranter Madi]

trmdi updated this revision to Diff 73694.
trmdi marked an inline comment as done.
trmdi added a comment.


  - Remove unneeded code

REPOSITORY
  R242 Plasma Framework (Library)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D26691?vs=73693=73694

BRANCH
  improve-file-drop-menu

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

AFFECTED FILES
  src/scriptengines/qml/plasmoid/containmentinterface.cpp
  src/scriptengines/qml/plasmoid/containmentinterface.h

To: trmdi, #plasma, mart, broulik, #vdg
Cc: anthonyfieroni, #plasma, kde-frameworks-devel, LeGast00n, GB_2, michaelh, 
ngraham, bruns

D26691: Optimize code when dropping files into the desktop

[Tranter Madi]

trmdi marked an inline comment as done.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: trmdi, #plasma, mart, broulik, #vdg
Cc: anthonyfieroni, #plasma, kde-frameworks-devel, LeGast00n, GB_2, michaelh, 
ngraham, bruns

D26691: Optimize code when dropping files into the desktop

[Tranter Madi]

trmdi updated this revision to Diff 73693.
trmdi added a comment.


  - Remove unneeded code

REPOSITORY
  R242 Plasma Framework (Library)

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D26691?vs=73634=73693

BRANCH
  improve-file-drop-menu

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

AFFECTED FILES
  src/scriptengines/qml/plasmoid/containmentinterface.cpp
  src/scriptengines/qml/plasmoid/containmentinterface.h

To: trmdi, #plasma, mart, broulik, #vdg
Cc: anthonyfieroni, #plasma, kde-frameworks-devel, LeGast00n, GB_2, michaelh, 
ngraham, bruns

D26691: Optimize code when dropping files into the desktop

[Tranter Madi]

trmdi added reviewers: broulik, VDG.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: trmdi, #plasma, mart, broulik, #vdg
Cc: anthonyfieroni, #plasma, kde-frameworks-devel, LeGast00n, GB_2, michaelh, 
ngraham, bruns

D26691: Optimize code when dropping files into the desktop

[Tranter Madi]

trmdi edited the summary of this revision.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: trmdi, #plasma, mart
Cc: #plasma, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D26691: Optimize code when dropping files into the desktop

[Tranter Madi]

trmdi edited the summary of this revision.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: trmdi, #plasma, mart
Cc: #plasma, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D26691: Optimize code when dropping files into the desktop

[Tranter Madi]

trmdi edited the summary of this revision.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: trmdi, #plasma, mart
Cc: #plasma, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D26691: Optimize code when dropping files into the desktop

[Tranter Madi]

trmdi retitled this revision from "[WIP] Optimize code when dropping files into 
the desktop" to "Optimize code when dropping files into the desktop".

REPOSITORY
  R242 Plasma Framework (Library)

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

To: trmdi, #plasma, mart
Cc: #plasma, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D26691: [WIP] Optimize code when dropping files into the desktop

[Tranter Madi]

trmdi edited the summary of this revision.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: trmdi, #plasma, mart
Cc: #plasma, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D26691: [WIP] Optimize code when dropping files into the desktop

[Tranter Madi]

trmdi added reviewers: Plasma, mart.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: trmdi, #plasma, mart
Cc: #plasma, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D26691: [WIP] Optimize code when dropping files into the desktop

[Tranter Madi]

trmdi edited the summary of this revision.
trmdi added a subscriber: Plasma.

REPOSITORY
  R242 Plasma Framework (Library)

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

To: trmdi
Cc: #plasma, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D26691: [WIP] Optimize code when dropping files into the desktop

[Tranter Madi]

trmdi created this revision.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
trmdi requested review of this revision.

REPOSITORY
  R242 Plasma Framework (Library)

BRANCH
  improve-file-drop-menu

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

AFFECTED FILES
  src/scriptengines/qml/plasmoid/containmentinterface.cpp
  src/scriptengines/qml/plasmoid/containmentinterface.h

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

D26484: Add KIO::DropJobFlag to allow manually showing the menu

[Tranter Madi]

This revision was automatically updated to reflect the committed changes.
Closed by commit R241:fbb2be9aad19: Add KIO::DropJobFlag to allow manually 
showing the menu (authored by trmdi).

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D26484?vs=73399=73400

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

AFFECTED FILES
  src/widgets/dropjob.cpp
  src/widgets/dropjob.h

To: trmdi, #frameworks, davidedmundson, elvisangelaccio, mart, dfaure
Cc: ngraham, broulik, anthonyfieroni, kde-frameworks-devel, LeGast00n, GB_2, 
michaelh, bruns

D26484: Add KIO::DropJobFlag to allow manually showing the menu

[Tranter Madi]

trmdi updated this revision to Diff 73399.
trmdi added a comment.


  - Comment

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D26484?vs=73364=73399

BRANCH
  master

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

AFFECTED FILES
  src/widgets/dropjob.cpp
  src/widgets/dropjob.h

To: trmdi, #frameworks, davidedmundson, elvisangelaccio, mart, dfaure
Cc: ngraham, broulik, anthonyfieroni, kde-frameworks-devel, LeGast00n, GB_2, 
michaelh, bruns

D26484: Add KIO::DropJobFlag to allow manually showing the menu

[Tranter Madi]

trmdi updated this revision to Diff 73364.
trmdi added a comment.


  - Typo

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D26484?vs=73363=73364

BRANCH
  master

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

AFFECTED FILES
  src/widgets/dropjob.cpp
  src/widgets/dropjob.h

To: trmdi, #frameworks, davidedmundson, elvisangelaccio, mart, dfaure
Cc: ngraham, broulik, anthonyfieroni, kde-frameworks-devel, LeGast00n, GB_2, 
michaelh, bruns

D26484: Add KIO::DropJobFlag to allow manually showing the menu

[Tranter Madi]

trmdi marked 3 inline comments as done.

REPOSITORY
  R241 KIO

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

To: trmdi, #frameworks, davidedmundson, elvisangelaccio, mart, dfaure
Cc: ngraham, broulik, anthonyfieroni, kde-frameworks-devel, LeGast00n, GB_2, 
michaelh, bruns

D26484: Add KIO::DropJobFlag to allow manually showing the menu

[Tranter Madi]

trmdi updated this revision to Diff 73363.
trmdi added a comment.


  - Improve comment/doc

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D26484?vs=73337=73363

BRANCH
  master

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

AFFECTED FILES
  src/widgets/dropjob.cpp
  src/widgets/dropjob.h

To: trmdi, #frameworks, davidedmundson, elvisangelaccio, mart, dfaure
Cc: ngraham, broulik, anthonyfieroni, kde-frameworks-devel, LeGast00n, GB_2, 
michaelh, bruns

D26484: Add KIO::DropJobFlag to allow manually showing the menu

[Tranter Madi]

trmdi edited the summary of this revision.

REPOSITORY
  R241 KIO

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

To: trmdi, #frameworks, davidedmundson, elvisangelaccio, mart, dfaure
Cc: ngraham, broulik, anthonyfieroni, kde-frameworks-devel, LeGast00n, GB_2, 
michaelh, bruns

D26484: Add KIO::DropJobFlag to allow manually showing the menu

[Tranter Madi]

trmdi retitled this revision from "Add KIO::DropJobFlag flag and 
DropJob::menuPopup()" to "Add KIO::DropJobFlag to allow manually showing the 
menu".

REPOSITORY
  R241 KIO

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

To: trmdi, #frameworks, davidedmundson, elvisangelaccio, mart, dfaure
Cc: ngraham, broulik, anthonyfieroni, kde-frameworks-devel, LeGast00n, GB_2, 
michaelh, bruns

D26484: Add KIO::DropJobFlag flag and DropJob::menuPopup()

[Tranter Madi]

trmdi retitled this revision from "Add KIO::DelayPopup flag and 
DropJob::menuPopup()" to "Add KIO::DropJobFlag flag and DropJob::menuPopup()".

REPOSITORY
  R241 KIO

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

To: trmdi, #frameworks, davidedmundson, elvisangelaccio, mart, dfaure
Cc: ngraham, broulik, anthonyfieroni, kde-frameworks-devel, LeGast00n, GB_2, 
michaelh, bruns

D26484: Add KIO::DelayPopup flag and DropJob::menuPopup()

[Tranter Madi]

trmdi updated this revision to Diff 73337.
trmdi added a comment.


  - Move the flag to DropJob header, rename

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D26484?vs=73273=73337

BRANCH
  master

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

AFFECTED FILES
  src/widgets/dropjob.cpp
  src/widgets/dropjob.h

To: trmdi, #frameworks, davidedmundson, elvisangelaccio, mart, dfaure
Cc: ngraham, broulik, anthonyfieroni, kde-frameworks-devel, LeGast00n, GB_2, 
michaelh, bruns

D26484: Add KIO::DelayPopup flag and DropJob::menuPopup()

[Tranter Madi]

trmdi marked 5 inline comments as done.

REPOSITORY
  R241 KIO

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

To: trmdi, #frameworks, davidedmundson, elvisangelaccio, mart, dfaure
Cc: ngraham, broulik, anthonyfieroni, kde-frameworks-devel, LeGast00n, GB_2, 
michaelh, bruns

D26484: Add KIO::DelayPopup flag and DropJob::menuPopup()

[Tranter Madi]

trmdi retitled this revision from "Popup menu again to reposition it" to "Add 
KIO::DelayPopup flag and DropJob::menuPopup()".
trmdi edited the summary of this revision.

REPOSITORY
  R241 KIO

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

To: trmdi, #frameworks, davidedmundson, elvisangelaccio, mart, dfaure
Cc: ngraham, broulik, anthonyfieroni, kde-frameworks-devel, LeGast00n, GB_2, 
michaelh, bruns

D26484: Popup menu again to reposition it

[Tranter Madi]

trmdi updated this revision to Diff 73273.
trmdi added a comment.


  - use the flag approach and add DropJob::menuPopup()

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D26484?vs=73175=73273

BRANCH
  master

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

AFFECTED FILES
  src/core/job_base.h
  src/widgets/dropjob.cpp
  src/widgets/dropjob.h

To: trmdi, #frameworks, davidedmundson, elvisangelaccio, mart, dfaure
Cc: ngraham, broulik, anthonyfieroni, kde-frameworks-devel, LeGast00n, GB_2, 
michaelh, bruns

D26484: Popup menu again to reposition it

[Tranter Madi]

trmdi added inline comments.

INLINE COMMENTS

> elvisangelaccio wrote in dropjob.cpp:415
> `QCursor::pos()` should be avoided as it's not reliable on wayland.
> 
> The whole "popup() again" here looks like a workaround to me. Can't this be 
> fixed on the plasma side?

> Can't this be fixed on the plasma side?

I think it can't, because KIO::DropJob popup the menu, only afterward Plasma 
adds new items into it.

REPOSITORY
  R241 KIO

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

To: trmdi, #frameworks, davidedmundson, elvisangelaccio, mart, dfaure
Cc: ngraham, broulik, anthonyfieroni, kde-frameworks-devel, LeGast00n, GB_2, 
michaelh, bruns

D26484: Popup menu again to reposition it

[Tranter Madi]

trmdi added inline comments.

INLINE COMMENTS

> elvisangelaccio wrote in dropjob.cpp:415
> `QCursor::pos()` should be avoided as it's not reliable on wayland.
> 
> The whole "popup() again" here looks like a workaround to me. Can't this be 
> fixed on the plasma side?

I'm having a plan to make it popup only one time.

> QCursor::pos() should be avoided as it's not reliable on wayland.

this is how it does currently, I just copy the original code. This could need 
to be resolved in another patch.

REPOSITORY
  R241 KIO

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

To: trmdi, #frameworks, davidedmundson, elvisangelaccio, mart, dfaure
Cc: ngraham, broulik, anthonyfieroni, kde-frameworks-devel, LeGast00n, GB_2, 
michaelh, bruns

D26484: Popup menu again to reposition it

[Tranter Madi]

trmdi added a comment.


  In D26484#591782 , @dfaure wrote:
  
  > Sorry I have no idea about the calling side in plasma. It sounds buggy 
indeed, if those are all calls on the same DropJob instance.
  
  
  Yes, you could check it here: 
https://phabricator.kde.org/source/plasma-framework/browse/master/src/scriptengines/qml/plasmoid/containmentinterface.cpp
  So do you mean this should be fixed to call 
`DropJob::setApplicationActions()` only one time?
  
  And do you @mart agree with that?

REPOSITORY
  R241 KIO

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

To: trmdi, #frameworks, davidedmundson, elvisangelaccio, mart, dfaure
Cc: ngraham, broulik, anthonyfieroni, kde-frameworks-devel, LeGast00n, GB_2, 
michaelh, bruns

D26484: Popup menu again to reposition it

[Tranter Madi]

trmdi added a comment.


  @mart @dfaure
  
  I have some question.
  1, Should `DropJob::setApplicationActions()` be called only one time?
  2, I see this when I drop multiple file types at once to the desktop (e.g. 2 
jpg and 1 zip), the `DropJob::setApplicationActions()` is called 3 times as 
follow:
  
Mimetype Job returns. "image/jpeg"
Received a suitable dropEvent at QPoint(1199,237)
Creating menu for: "image/jpeg" QPoint(1199,237)

Mimetype Job returns. "application/zip"
Received a suitable dropEvent at QPoint(1199,237)
Creating menu for: "application/zip" QPoint(1199,237)

Mimetype Job returns. "image/jpeg"
Received a suitable dropEvent at QPoint(1199,237)
Creating menu for: "image/jpeg" QPoint(1199,237)
  
  But finally only the last one's appActions are added to the menu. This is a 
bit weird.
  I want to understand your intention to make this patch as good as possible.

REPOSITORY
  R241 KIO

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

To: trmdi, #frameworks, davidedmundson, elvisangelaccio, mart, dfaure
Cc: ngraham, broulik, anthonyfieroni, kde-frameworks-devel, LeGast00n, GB_2, 
michaelh, bruns

D26484: Popup menu again to reposition it

[Tranter Madi]

trmdi retitled this revision from "Add a new parameter for delaying showing 
menu" to "Popup menu again to reposition it".

REPOSITORY
  R241 KIO

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

To: trmdi, #frameworks, davidedmundson, elvisangelaccio, mart, dfaure
Cc: broulik, anthonyfieroni, kde-frameworks-devel, LeGast00n, GB_2, michaelh, 
ngraham, bruns

D26484: Add a new parameter for delaying showing menu

[Tranter Madi]

trmdi removed a dependent revision: D26514: Delay popup for containments.

REPOSITORY
  R241 KIO

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

To: trmdi, #frameworks, davidedmundson, elvisangelaccio, mart, dfaure
Cc: broulik, anthonyfieroni, kde-frameworks-devel, LeGast00n, GB_2, michaelh, 
ngraham, bruns

D26484: Add a new parameter for delaying showing menu

[Tranter Madi]

trmdi edited the summary of this revision.

REPOSITORY
  R241 KIO

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

To: trmdi, #frameworks, davidedmundson, elvisangelaccio, mart, dfaure
Cc: broulik, anthonyfieroni, kde-frameworks-devel, LeGast00n, GB_2, michaelh, 
ngraham, bruns

D26484: Add a new parameter for delaying showing menu

[Tranter Madi]

trmdi updated this revision to Diff 73175.
trmdi added a comment.


  - Simplify the approach

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D26484?vs=73114=73175

BRANCH
  master

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

AFFECTED FILES
  src/widgets/dropjob.cpp

To: trmdi, #frameworks, davidedmundson, elvisangelaccio, mart, dfaure
Cc: broulik, anthonyfieroni, kde-frameworks-devel, LeGast00n, GB_2, michaelh, 
ngraham, bruns

D26484: Add a new parameter for delaying showing menu

[Tranter Madi]

trmdi planned changes to this revision.

REPOSITORY
  R241 KIO

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

To: trmdi, #frameworks, davidedmundson, elvisangelaccio, mart, dfaure
Cc: broulik, anthonyfieroni, kde-frameworks-devel, LeGast00n, GB_2, michaelh, 
ngraham, bruns

D26484: Add a new parameter for delaying showing menu

[Tranter Madi]

trmdi updated this revision to Diff 73114.
trmdi added a comment.


  - Improve code
  - Improve code

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D26484?vs=73113=73114

BRANCH
  master

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

AFFECTED FILES
  src/core/job_base.h
  src/widgets/dropjob.cpp

To: trmdi, #frameworks, davidedmundson, elvisangelaccio, mart, dfaure
Cc: broulik, anthonyfieroni, kde-frameworks-devel, LeGast00n, GB_2, michaelh, 
ngraham, bruns

D26484: Add a new parameter for delaying showing menu

[Tranter Madi]

trmdi updated this revision to Diff 73113.
trmdi added a comment.


  - Improve code

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D26484?vs=73112=73113

BRANCH
  master

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

AFFECTED FILES
  src/core/job_base.h
  src/widgets/dropjob.cpp

To: trmdi, #frameworks, davidedmundson, elvisangelaccio, mart, dfaure
Cc: broulik, anthonyfieroni, kde-frameworks-devel, LeGast00n, GB_2, michaelh, 
ngraham, bruns

D26484: Add a new parameter for delaying showing menu

[Tranter Madi]

trmdi updated this revision to Diff 73112.
trmdi added a comment.


  - Remove the new signal and show a "Fetching data" menu while delaying

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D26484?vs=73076=73112

BRANCH
  master

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

AFFECTED FILES
  src/core/job_base.h
  src/widgets/dropjob.cpp

To: trmdi, #frameworks, davidedmundson, elvisangelaccio, mart, dfaure
Cc: broulik, anthonyfieroni, kde-frameworks-devel, LeGast00n, GB_2, michaelh, 
ngraham, bruns

D26484: Add a new parameter for delaying showing menu

[Tranter Madi]

trmdi planned changes to this revision.

REPOSITORY
  R241 KIO

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

To: trmdi, #frameworks, davidedmundson, elvisangelaccio, mart, dfaure
Cc: broulik, anthonyfieroni, kde-frameworks-devel, LeGast00n, GB_2, michaelh, 
ngraham, bruns

D26484: Add a new parameter for delaying showing menu

[Tranter Madi]

trmdi edited the summary of this revision.

REPOSITORY
  R241 KIO

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

To: trmdi, #frameworks, davidedmundson, elvisangelaccio, mart, dfaure
Cc: broulik, anthonyfieroni, kde-frameworks-devel, LeGast00n, GB_2, michaelh, 
ngraham, bruns

D26484: Add a new parameter for delaying showing menu

[Tranter Madi]

trmdi marked an inline comment as done.

REPOSITORY
  R241 KIO

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

To: trmdi, #frameworks, davidedmundson, elvisangelaccio, mart, dfaure
Cc: broulik, anthonyfieroni, kde-frameworks-devel, LeGast00n, GB_2, michaelh, 
ngraham, bruns

D26484: Add a new parameter for delaying showing menu

[Tranter Madi]

trmdi updated this revision to Diff 73076.
trmdi added a comment.


  - Move delayPopup to JobFlag

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D26484?vs=73030=73076

BRANCH
  master

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

AFFECTED FILES
  src/core/job_base.h
  src/widgets/dropjob.cpp
  src/widgets/dropjob.h

To: trmdi, #frameworks, davidedmundson, elvisangelaccio, mart, dfaure
Cc: broulik, anthonyfieroni, kde-frameworks-devel, LeGast00n, GB_2, michaelh, 
ngraham, bruns

D26484: Add a new parameter for delaying showing menu

[Tranter Madi]

trmdi marked an inline comment as done.

REPOSITORY
  R241 KIO

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

To: trmdi, #frameworks, davidedmundson, elvisangelaccio, mart, dfaure
Cc: anthonyfieroni, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, 
bruns

D26484: Add a new parameter for delaying showing menu

[Tranter Madi]

trmdi updated this revision to Diff 73030.
trmdi added a comment.


  - Add a new function instead of changing the old exported one

REPOSITORY
  R241 KIO

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D26484?vs=72975=73030

BRANCH
  master

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

AFFECTED FILES
  src/widgets/dropjob.cpp
  src/widgets/dropjob.h

To: trmdi, #frameworks, davidedmundson, elvisangelaccio, mart, dfaure
Cc: anthonyfieroni, kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, 
bruns

D26484: Add a new parameter for delaying showing menu

[Tranter Madi]

trmdi added a dependent revision: D26514: Delay popup for containments.

REPOSITORY
  R241 KIO

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

To: trmdi, #frameworks, davidedmundson, elvisangelaccio, mart, dfaure
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns

D26484: Add a new parameter for delaying showing menu

[Tranter Madi]

trmdi edited the summary of this revision.

REPOSITORY
  R241 KIO

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

To: trmdi, #frameworks, davidedmundson, elvisangelaccio, mart, dfaure
Cc: kde-frameworks-devel, LeGast00n, GB_2, michaelh, ngraham, bruns