fm.delete not working as expected

[dskrad]

Runtime Environment

• Operating system and version: Elementary OS 5.1.7
• Terminal emulator and version: tmux 2.6
• Python version: 3.6.9
• Ranger version/commit: 1.9.3
• Locale: en_US.UTF-8

Current Behavior

self.fm.delete(files) deletes the file and the containing folder, even when there are remaining files that were not selected in my custom function.

I created a custom function in commands.py in which user selects two video files and merges them with mkvmerge using self.fm.execute_command. After merging the files and further processing and moving the result, I want to delete the file-merged.mkv to cleanup the working folder.

I think this has something to do with the fact that I use self.fm.thistab.get_selection, but then I use some concatenation of filenames to process the files and call self.fm.delete on this new filename. I am not sure why this results in the containing folder to be deleted.

Expected Behavior

I expect only the arguments in files to be deleted. The containing folder and any files it contains should remain intact.

[markus-bauer]

Try passing the filename in a list, not as a string: self.fm.delete([file_to_delete]).

---

Without seeing your code, it's hard to know. But I think what's happening is this:

If you look at the code for delete, you see that it expects a list of files, not a string. https://github.com/ranger/ranger/blob/391f061cb0b0cfa8266c0651f2a6d948f22e01dd/ranger/core/actions.py#L1626-L1633

The files it deletes are this:

files = [os.path.abspath(path) for path in files]

If you call fm.delete("filename") and your current dir is "/current/", it will try to delete: /current/f /current/i /current/l /current/e /current/n ...

That's the python magic, where it just silently (miss)interprets your string as a list.

[markus-bauer]

@toonn By the way. This shouldn't happen. Even if this is a user error, it still deletes files that it obviously shouldn't. There should be a check if files is a list.

I haven't looked closely, but couldn't this be dangerous:

import os
os.chdir("/home/")
files_to_delete = [os.path.abspath(path) for path in "path/to/file"]

for f in files_to_delete:
    print(f)

This would delete this:

/home/p
/home/a
/home/t
/home/h
/
/home/t
/home/o
/
/home/f
/home/i
/home/l
/home/e

Notice that "/" is in there.

[dskrad]

Thanks for the tip. Since I am only trying to delete one file, I just wrapped the argument in braces so it is a list containing one item.

As @markus-bauer says, the default method can be dangerous. I checked the source code, and the delete method is doing some complicated checking of the files before deleting, which may save you from deleting root or any other mount point. But it will apparently delete the parent folder in my case at least.

This could be solved with a quick check to make sure it is a list before treating it as a list?

if not isinstance(files, list):
  raise(TypeError())

[markus-bauer]

Was this the problem with your custom command? Does it work now?

---

the delete method is doing some complicated checking of the files before deleting,

I don't know if that's the case. I think it more or less says:

files = [os.path.abspath(path) for path in files]
# ... untag and remove from copy_buffer
# delete:
for path in files:
    if dir and not link:
        rmtree(path)
    else:
        remove(path)

[dskrad]

I was passing a single string to the delete function, and the result was to delete the containing folder, then ranger displayed an error about the file not existing anymore. Once I changed the code to pass a list with one item, the delete function works as expected: the single file is deleted and the containing folder remains intact.

I was speculating about the delete function, as I'm not sure what all the if statements are doing with tags and copy_buffer and so forth.

Correct me if I'm wrong, but shouldn't a function which expects a list, and does unexpected things when passed a string, have a built-in check to make sure a list was passed? I know python does not care what arguments are passed, but the code should be more explicit. If a string is passed, throw an error, or change the function to be able to delete a single file if a string is passed.

[markus-bauer]

Good, I just wanted to confirm, that your problem is actually solved.

---

Correct me if I'm wrong, but shouldn't a function which expects a list, and does unexpected things when passed a string, have a built-in check to make sure a list was passed? I know python does not care what arguments are passed, but the code should be more explicit. If a string is passed, throw an error, or change the function to be able to delete a single file if a string is passed.

I totally agree with this. I'm not a ranger dev, and don't know why the code is like this.