D27951: Allow users to change dropAction to MoveAction through kdeglobals

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:

D27951: Allow users to change dropAction to MoveAction through kdeglobals

meven added a comment. In D27951#641207 , @trmdi wrote: > In D27951#641173 , @meven wrote: > > > In D27951#639386 , @trmdi wrote: > > > > > In

D27951: Allow users to change dropAction to MoveAction through kdeglobals

trmdi abandoned this revision. trmdi added a comment. In D27951#641173 , @meven wrote: > In D27951#639386 , @trmdi wrote: > > > In D27951#639350 , @meven

D27951: Allow users to change dropAction to MoveAction through kdeglobals

meven added a comment. 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

D27951: Allow users to change dropAction to MoveAction through kdeglobals

bruns added a comment. In D27951#639352 , @meven wrote: > In D27951#639350 , @meven wrote: > > > > Another question, will UDSEntry::UDS_DEVICE_ID be available on Windows? > > > > I am not a

D27951: Allow users to change dropAction to MoveAction through kdeglobals

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

D27951: Allow users to change dropAction to MoveAction through kdeglobals

meven added a comment. In D27951#639350 , @meven wrote: > > Another question, will UDSEntry::UDS_DEVICE_ID be available on Windows? > > I am not a Windows specialist but I imagine we could hack together an equivalent solution (I am saying

D27951: Allow users to change dropAction to MoveAction through kdeglobals

meven added a comment. In D27951#637792 , @trmdi wrote: > In D27951#637690 , @meven wrote: > > > Look into kde connect kioslave. > > That does not concern this diff much though. > > > I

D27951: Allow users to change dropAction to MoveAction through kdeglobals

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

D27951: Allow users to change dropAction to MoveAction through kdeglobals

meven added a comment. In D27951#637118 , @trmdi wrote: > In D27951#637038 , @dfaure wrote: > > > Indeed. Bug in kio_file, fixed in D28388 . > > > Ok, it

D27951: Allow users to change dropAction to MoveAction through kdeglobals

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

D27951: Allow users to change dropAction to MoveAction through kdeglobals

dfaure added a comment. Indeed. Bug in kio_file, fixed in D28388 . REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D27951 To: trmdi, ngraham, dfaure, meven, #vdg, davidedmundson Cc: elvisangelaccio, davidedmundson, meven,

D27951: Allow users to change dropAction to MoveAction through kdeglobals

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

D27951: Allow users to change dropAction to MoveAction through kdeglobals

dfaure added a comment. 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

D27951: Allow users to change dropAction to MoveAction through kdeglobals

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

D27951: Allow users to change dropAction to MoveAction through kdeglobals

dfaure added a comment. 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` REPOSITORY R241

D27951: Allow users to change dropAction to MoveAction through kdeglobals

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

D27951: Allow users to change dropAction to MoveAction through kdeglobals

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

D27951: Allow users to change dropAction to MoveAction through kdeglobals

dfaure added a comment. 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. REPOSITORY R241 KIO REVISION

D27951: Allow users to change dropAction to MoveAction through kdeglobals

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

D27951: Allow users to change dropAction to MoveAction through kdeglobals

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

D27951: Allow users to change dropAction to MoveAction through kdeglobals

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

D27951: Allow users to change dropAction to MoveAction through kdeglobals

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

D27951: Allow users to change dropAction to MoveAction through kdeglobals

dfaure added a comment. OK, I looked at the code more closely and I see now that on different partitions it will still show the menu, rather than make any assumptions. This still creates a risk for surprises, as to whether the menu will appear or not (on Windows one can look at two

D27951: Allow users to change dropAction to MoveAction through kdeglobals

ngraham added a comment. Just to add a different perspective on this, while I agree that "many users want it" isn't an automatic justification, I don't think it should carry no weight, either. We are making this software for users, and we ought to pay attention to their desires. Yes, it's

D27951: Allow users to change dropAction to MoveAction through kdeglobals

dfaure added a comment. I fully agree with the other David, FWIW. 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,

D27951: Allow users to change dropAction to MoveAction through kdeglobals

davidedmundson added a comment. I've done a code review, they need following up regardless. --- As you saw in the bug report I don't like the feature, and I'm against it. Though I can't veto changes. Repeating myself on bugzilla. The two things I really hate as

D27951: Allow users to change dropAction to MoveAction through kdeglobals

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

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?

D27951: Allow users to change dropAction to MoveAction through kdeglobals

elvisangelaccio added a comment. > 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? REPOSITORY R241 KIO REVISION DETAIL https://phabricator.kde.org/D27951 To: trmdi,

D27951: Allow users to change dropAction to MoveAction through kdeglobals

ngraham added a comment. All right, let's see what #VDG people and @davidedmundson have to say. I would like for this to go in in some form, but we have to make sure that everyone's okay with it. REPOSITORY R241 KIO REVISION DETAIL

D27951: Allow users to change dropAction to MoveAction through kdeglobals

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

D27951: Allow users to change dropAction to MoveAction through kdeglobals

ngraham added a comment. In D27951#627078 , @trmdi wrote: > I think the user has to choose. When he chose dndToMove, that means he prefers the move action to others (copy, link, extract, set as wallpaper...) My interpretation of the use

D27951: Allow users to change dropAction to MoveAction through kdeglobals

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

D27951: Allow users to change dropAction to MoveAction through kdeglobals

ngraham added a comment. 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

D27951: Allow users to change dropAction to MoveAction through kdeglobals

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

D27951: Allow users to change dropAction to MoveAction through kdeglobals

ngraham added a dependent revision: D27998: [KCMs/Workspace] Add option for dndToMove. 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,

D27951: Allow users to change dropAction to MoveAction through kdeglobals

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,

D27951: Allow users to change dropAction to MoveAction through kdeglobals

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

D27951: Allow users to change dropAction to MoveAction through kdeglobals

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

D27951: Allow users to change dropAction to MoveAction through kdeglobals

ngraham added reviewers: VDG, davidedmundson. ngraham accepted this revision. ngraham added a subscriber: davidedmundson. ngraham added a comment. Fantastic. I'm approving this since my concerns have been resolved and it works as you've indended it, but don't land it as it needs more

D27951: Allow users to change dropAction to MoveAction through kdeglobals

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

D27951: Allow users to change dropAction to MoveAction through kdeglobals

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

D27951: Allow users to change dropAction to MoveAction through kdeglobals

ngraham added a comment. 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

D27951: Allow users to change dropAction to MoveAction through kdeglobals

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

D27951: Allow users to change dropAction to MoveAction through kdeglobals

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

D27951: Allow users to change dropAction to MoveAction through kdeglobals

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

D27951: Allow users to change dropAction to MoveAction through kdeglobals

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

D27951: Allow users to change dropAction to MoveAction through kdeglobals

ngraham edited the summary of this revision. 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

ngraham added a comment. Nice, drag-and-drop now correctly shows the menu for the case of dragging an icon from Kickoff or the task manager. 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

D27951: Allow users to change dropAction to MoveAction through kdeglobals

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

D27951: Allow users to change dropAction to MoveAction through kdeglobals

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:

D27951: Allow users to change dropAction to MoveAction through kdeglobals

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