This does not include the workspace-option kcm setting.
So I would title this "Allow users to change dropAction to MoveAction through a dndToMove kdeglobal setting"

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 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?

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.

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?

Done.

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?

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
• 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.

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.

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 by drag&drop in this case. How do you think about this?

Maybe something like holding Shift while dnd to show the menu?

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&drop 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.

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&drop 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 the move action to others (copy, link, extract, set as wallpaper...)
If he wants to see the menu, he could hold the Shift modifier key.

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 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.

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 quite likely he likes that the most, and he dislikes/ doesn't care about others.
I think the current implementation is balanced.

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.

With dndToMove=true in kdeglobals, drag&drop files will move them without showing the menu. (holding Shift shows it)

Are we going to expose this setting in Plasma?

In D27951#627849, @elvisangelaccio wrote:

With dndToMove=true in kdeglobals, drag&drop 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

Friendly ping.

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 justifications for doing anything are "windows does it" or "many users want". It's our job to do thinking and provide something that's actually better.

This little menu was one of the first things I saw when I first used Konqueror and I remember it being one of the things that made me love KDE.
It's excellent thought through usability, drag and drop is inherently ambiguous. For something so important, always prompting is always safe. It's consistent and not pseudo-random and the little UI follows fitt's law and is scarcely slower. I'm aware this is an option, but IMHO an option for something bad.

I fully agree with the other David, FWIW.

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 our job to give then what they really need, not necessarily what they ask for. However when different users consistently complain about the same thing over an extended period of time, I think we'd be remiss to ignore that and say the users don't know what they're talking about and we know better. Maybe sometimes we do, but that should be something we continuously challenge and verify rather than assuming it.

I see this as a "powerful when needed" feature. The determinism of always showing a menu is indeed highly usable for novice or normal users, but eventually I think it starts to feel annoying for power users. I think it's pretty likely for the most common drag-and-drop operation to be a regular move from one location on the same partition to another one, and if you realize this, I think it's nice to have that be a default rather than getting asked every time.

Speaking personally, every time I drag-and-drop from one local location to another on the same partition after forgetting to hold down the shift key, a little voice in my head says, "darn it, why can it just move by default?"

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 paths and use the driver letters to see if it's the same partition or not, that's not possible on Unix). But I see how we can assume that a power user knows where his partitions are. And they can turn off the advanced feature again if they get it wrong too many times.

So I won't veto this.

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?

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"

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.

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"="/run/media/trmdi/7E7A4CDA7A4C9137/Data" "UDS_SIZE"=4607182418800017408 "UDS_DEVICE_ID"=0 "UDS_INODE"=0 "UDS_FILE_TYPE"=4096 "UDS_ACCESS"=1568 "UDS_MODIFICATION_TIME"=140636170510627 "UDS_ACCESS_TIME"=0 "UDS_USER"="" "UDS_GROUP"="" ]
"/run/media/trmdi/7E7A4CDA7A4C9137/Data"  -  "/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/trmdi/Downloads"  -  "/dev/sda1"

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

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?

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.

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.

Indeed. Bug in kio_file, fixed in D28388.

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.

In D27951#637118, @trmdi wrote:

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.

Look into kde connect kioslave.
That does not concern this diff much though.

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.

Another question, will UDSEntry::UDS_DEVICE_ID be available on Windows?

In D27951#637792, @trmdi wrote:

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.

Maybe you don't much about KIO.

Anything that uses KIO:: calls is treated by ioslave which are programs to handle protocols.
For instance file:/ (that handles /) or kdeconnect:/
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.

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 this should be possible not that we will implement, feel free to give it a try in file_win.cpp ;) ).

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 this should be possible not that we will implement, feel free to give it a try in file_win.cpp ;) ).

dwVolumeSerialNumber of https://docs.microsoft.com/fr-fr/windows/win32/api/fileapi/ns-fileapi-by_handle_file_information seems promising
https://docs.microsoft.com/fr-fr/windows/win32/api/fileapi/nf-fileapi-getfileinformationbyhandle?redirectedfrom=MSDN

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.

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 Windows specialist but I imagine we could hack together an equivalent solution (I am saying this should be possible not that we will implement, feel free to give it a try in file_win.cpp ;) ).

dwVolumeSerialNumber of https://docs.microsoft.com/fr-fr/windows/win32/api/fileapi/ns-fileapi-by_handle_file_information seems promising
https://docs.microsoft.com/fr-fr/windows/win32/api/fileapi/nf-fileapi-getfileinformationbyhandle?redirectedfrom=MSDN

Or https://docs.microsoft.com/en-us/windows/win32/api/winbase/ns-winbase-file_id_info. May be a little bit cheapers, as it retrieves less information.

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.

In D27951#641207, @trmdi wrote:

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 have local urls.

I thought their file scheme would be kdeconnect. They would be on a fuse mount, either way it is detectable.

Fixing every ioslave is too difficult.
So I should drop this idea.

It does not rest on your shoulder only, fear not.
I did not expect you to do all of this, but this could be a very shorltist as we don't that many writable filesystem-like ioslave that will need actual fixing.
I meant this is not your concern for this patch.

This is a nice feature some of our users will appreciate.

So I would encourage you to reconsider abandoning this work.

I might take over to finish it eventually.

In D27951#645212, @meven wrote:

I might take over to finish it eventually.

Yes, please. I'm very glad that you support this. :)