cockpit-project / cockpit-files

A Featureful File Browser for Cockpit (Modernized and tested version of https://github.com/45Drives/cockpit-navigator)
GNU Lesser General Public License v2.1
25 stars 23 forks source link

Copy, cut & paste issues #467

Open jelly opened 1 month ago

jelly commented 1 month ago

Collection of issues with cut, copy and paste.

We currently have copy & paste in the UI but there are several issues with it:

Conflicts:

Preserving file/directory attributes while copying:

As user:

pcmanfm (using gvfs) copies group and mtime, but creates a new ctime

As superuser (administrative access)

Ux:

briansmith0 commented 1 month ago

GNOME Files and KDE Dolphin also provide the opportunity to rename the file in this type of scenario.

This other issue seems to be related to this as well: https://github.com/cockpit-project/cockpit-files/issues/296

I think it is a common workflow to go in to a directory, copy a file, and paste it in the same directory to make a backup of the file.

jelly commented 1 month ago

Testing this in GNOME files:

jelly commented 1 month ago

It is not trivial to handle cp --interactive from Cockpit and for renaming we went with a Python script to run renameat2. So for copying we likely want to implement something ourselves as well some considerations:

For copying one option is:

https://docs.python.org/3/library/shutil.html#shutil.copytree

As passing dirs_exist_ok will throw an exception on an existing file it seems the only way we can handle resolutions well is overriding the copy_function . But this will be tricky to handle from the UI as it is callback based.

Some wild ideas:

Moving

As moving is relevant I wonder if we can do this atomic by:

mv foo /tmp/bar/foo

fp = openat("/tmp/bar/foo")
renameat2("foo", fp);

Except when moving between two different filesystem, an easy test case:

source = b'/tmp/foo'
dest = '/foo'
>>> libc6.renameat2(AT_FDCWD, source, AT_FDCWD, dest, RENAME_NOREPLACE)
-1
>>> ctypes.get_errno()
18
>>> os.strerror(ctypes.get_errno())
'Invalid cross-device link'
jelly commented 1 month ago

Alternative suggestion from @allisonkarlitskaya is to not support pasting when the directory already exists. (As already implemented in https://github.com/cockpit-project/cockpit-files/pull/457

Only paste in directory is then harder to support but that is maybe a bit nieche.

martinpitt commented 1 month ago

We allow copying multiple files at once and from different directories

Why do we do this? I don't know any file manager which supports this kind of "multiple clipboard". I see the use case for marking a bunch of files (or subdirs) in a directory view and moving/copying them someplace else -- but building a "additive persistent clipboard" with marking files from different dirs sounds like making our lives really hard..

jelly commented 1 month ago

We allow copying multiple files at once and from different directories

Why do we do this? I don't know any file manager which supports this kind of "multiple clipboard". I see the use case for marking a bunch of files (or subdirs) in a directory view and moving/copying them someplace else -- but building a "additive persistent clipboard" with marking files from different dirs sounds like making our lives really hard..

I agree, it is valid to copy something from dir A and then browse to / and paste it but picking things up on the way is imo making life hard. So maybe the clipboard state should note be:

When in /tmp/foo and then copying "a, b" you'll get in the "clipboard":

"/tmp/foo/a" /tmp/foo/b"

Maybe it should be

{ files: [], dirs: [], base_dir: "/tmp/foo" }

And when copying in a different directory we compare $CWD with the clipboard.base_dir state and reset files, dirs.

This whole feature needs more thought.