In D28370#676352, @elvisangelaccio wrote:

This caused issues so I reverted it for now.

https://bugs.kde.org/show_bug.cgi?id=425757
https://bugs.kde.org/show_bug.cgi?id=426196

In D27733#674689, @stikonas wrote:

OK, the patch looks fine. I would suggest pushing it.

It does not deal with the case of non-plasma wayland compositor but I just tested yakuake on Gnome Wayland and it horribly broken anyway.

In D27733#674638, @stikonas wrote:

Is there any reason why this is abandonned? It seems to work fine on my system.

Can this be moved to krunner? In that case it can be used even when the user doesn't use global menu?

Just a question, SierraBreezeEnhanced has a feature called "Unison hovering". Does this support it?

In D29395#662576, @ngraham wrote:

Now the inner image has a greater top margin than bottom margin. Also, could the artist and album be allowed to become two-line strings before eliding, maybe?

In D29260#659172, @ngraham wrote:

GridUnit * 40 works out to be 720 px high with the default 10pt Noto Sans font, which is enough space to not interfere with the default 36px bottom panel.

But there's no way around the fact that this window greatly benefits from more space. On such a small screen resolution, I would expect people to want to just maximize the window.

Please keep in mind that not everyone uses a high res screen. 1366x768 is a very popular one, please don't make it too big on those screens. :(

In D28985#652097, @ngraham wrote:

Excellent!

Please land on the Plasma/5.18 branch and then merge to master. Let me know if you need a hand with that.

In D27951#645212, @meven wrote:

I might take over to finish it eventually.

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.

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.

In D27951#637690, @meven wrote:

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

In D27951#637038, @dfaure wrote:

Indeed. Bug in kio_file, fixed in D28388.

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.

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

Improve the logic a bit.

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.

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?

@ngraham
It seems that everyone accepted this, could you help me to land this to whatever branch you want ?

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

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

Friendly ping.

The patch now only updates the cursor while dragging.

In D28017#627858, @elvisangelaccio wrote:

With Qt 5.15 I get this debug output when i click that "drag" button:

Qt::CopyAction QFlags<Qt::DropAction>(CopyAction|MoveAction)

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?

In D28017#627845, @elvisangelaccio wrote:

As far as I can see, bug #388259 has been fixed upstream (either in Qt or in the platform integration plugin).

Set accepted state and dropAction of event more correctly.

In D28017#627729, @davidedmundson wrote:

Is this a Qt bug or an intention

which bit specifically?

Is this a Qt bug or an intention? Because QDrag is used in many places.

Nate's comment

In D27998#627623, @ngraham wrote:

Where? Being wrong in multiple places doesn't make it right. ;)

Nate's comment

In D27998#627605, @ngraham wrote:

Right.

@bugseforuns's objection is that you used the word "device" to mean "partition" in the UI.

clean

In D27998#627603, @ngraham wrote:

What does?

Yes, it means "partition".

Address @meven and @ngraham's comments.
Now you can change the modifier key without having to move the mouse.

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.

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.

In D28017#626772, @meven wrote:

This works only once the mouse starts moving after the key modifier has been pressed.

Minor improvement

Move to bottom, add hint

Allow to explicitly show the menu when holding Shift

In D27951#626135, @ngraham wrote:

it needs more discussion 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

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

Code style

Code style

Apply only when all items are local.

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?

Move comment

Address meven's comment

Move fileItem back to the local scope

• Check possible actions in determineDropAction

In D27925#624066, @ryanmccoskrie wrote:

In D27925#624022, @trmdi wrote:

Look weird. Don't you misunderstand it?

Apparently my titlebar resize patch shouldn't have a space between "if" and "(" so I changed the rest of Yakuake to match.

Look weird. Don't you misunderstand it?

Could we close this now?

Rename m_realVisible to m_requestedVisible

Small fix.

Never show before position is set