use .get_file() instead of basename from libc

[omar-polo]

This fixes #48938

[theraot]

For what I've found basename and get_basename are not equivalent:

• basename returns the last component of the path, which could be a file name or a folder name. See http://man.openbsd.org/basename.3
• get_basename returns the path without the extension. See https://docs.godotengine.org/en/latest/classes/class_string.html#class-string-method-get-basename

Given the name file_name, the fact that it will be used like this:

	String dest_path = trash_path + "/files/" + file_name;

And as far as I can tell, move_to_trash does not work for folders.

I think you should use get_file instead of get_basename. See https://docs.godotengine.org/en/latest/classes/class_string.html#class-string-method-get-file

---

The behavior of get_file differs from the description of basename in that it will return empty string if the input is empty string or consist only of /. I quote from http://man.openbsd.org/basename.3:

If path consists entirely of ‘/’ characters, a pointer to the string "/" is returned. If path is a null pointer or the empty string, a pointer to the string "." is returned.

---

The description of basename at https://www.man7.org/linux/man-pages/man3/basename.3.html (which that is not BSD) matches the description at http://man.openbsd.org/basename.3 and also provides some test cases:

              path       dirname   basename
              /usr/lib   /usr      lib
              /usr/      /         usr
              usr        .         usr
              /          /         /
              .          .         .
              ..         .         ..

Adding get_basename and get_file there for comparison, looks like this:

              path       dirname   basename   get_basename     get_file
              /usr/lib   /usr      lib        /usr/lib         lib
              /usr/      /         usr        /usr/            <empty string>
              usr        .         usr        usr              usr
              /          /         /          /                <empty string>
              .          .         .          <empty string>   .
              ..         .         ..         .                ..

Where <empty string> denotes the empty string.

You can see that if we assume we have a file path, then get_file is what we want.

[omar-polo]

Thanks for checking! I've updated the PR.

I was confused because the similar naming, apologize.

[omar-polo]

I forgot to update the commit message, here's another try

[theraot]

Sorry.

I had a look for usages of move_to_trash, and there is one that seems to use it on folders:

godot/editor/dependency_editor.cpp

Lines 528 to 537 in 77721b3

	for (int i = 0; i < dirs_to_delete.size(); ++i) {
	String path = OS::get_singleton()->get_resource_dir() + dirs_to_delete[i].replace_first("res://", "/");
	print_verbose("Moving to trash: " + path);
	Error err = OS::get_singleton()->move_to_trash(path);
	if (err != OK) {
	EditorNode::get_singleton()->add_io_error(TTR("Cannot remove:") + "\n" + dirs_to_delete[i] + "\n");
	} else {
	emit_signal(SNAME("folder_removed"), dirs_to_delete[i]);
	}
	}

If move_to_trash needs to support this, we need something better than get_file.

[theraot]

I wish there was a better API for this. However, let us take advantage of this pattern: get_base_dir().get_file()

I found example of it already in the source:

godot/editor/plugins/script_editor_plugin.cpp

Line 1976 in 58aa020

if (!path.get_base_dir().get_file().is_empty()) {

That would give us the name of the folder (but not of the file).

[theraot]

This is what I suggest:

	String file_name = p_path.get_file();
	if (file_name.length() == 0) {
		file_name = p_path.get_base_dir().get_file();
	}

That way, if p_path points to a file, p_path.get_file() gives us the name of the file. Otherwise, we get empty string and enter the conditional, where p_path.get_base_dir().get_file() gives us the name of the folder.

[omar-polo]

Thanks for the suggestion! I'm not really comfortable writing C++ yet.

Yep, I wish there were better API too. (but this applies also to libc, basename(3) alter the passed string :/)

I've also improved the commit message, hope it's not too long ^^

[theraot]

I, of course, approve this. Because of course.

[akien-mga]

Thanks!

[akien-mga]

Cherry-picked for 3.4.