{"action":"created","comment":{"url":"https://api.github.com/repos/godotengine/godot/pulls/comments/168415723","pull_request_review_id":96775978,"id":168415723,"diff_hunk":"@@ -2499,48 +2499,66 @@ static String get_mountpoint(const String &p_path) {\n }\n \n Error OS_X11::move_to_trash(const String &p_path) {\n-\tString trashcan = \"\";\n+\tString trash_can = \"\";\n \tString mnt = get_mountpoint(p_path);\n \n+\t// If there is a directory \"[Mountpuint]/.Trash-[UID]/files\", use it as the trash can.\n \tif (mnt != \"\") {\n \t\tString path(mnt + \"/.Trash-\" + itos(getuid()) + \"/files\");\n \t\tstruct stat s;\n \t\tif (!stat(path.utf8().get_data(), &s)) {\n-\t\t\ttrashcan = path;\n+\t\t\ttrash_can = path;\n \t\t}\n \t}\n \n-\tif (trashcan == \"\") {\n+\t// Otherwise, if ${XDG_DATA_HOME} is defined, use \"${XDG_DATA_HOME}/Trash/files\" as the trash can.\n+\tif (trash_can == \"\") {\n \t\tchar *dhome = getenv(\"XDG_DATA_HOME\");\n \t\tif (dhome) {\n-\t\t\ttrashcan = String(dhome) + \"/Trash/files\";\n+\t\t\ttrash_can = String(dhome) + \"/Trash/files\";\n \t\t}\n \t}\n \n-\tif (trashcan == \"\") {\n+\t// Otherwise, if ${HOME} is defined, use \"${HOME}/.local/share/Trash/files\" as the trash can.\n+\tif (trash_can == \"\") {\n \t\tchar *home = getenv(\"HOME\");\n \t\tif (home) {\n-\t\t\ttrashcan = String(home) + \"/.local/share/Trash/files\";\n+\t\t\ttrash_can = String(home) + \"/.local/share/Trash/files\";\n \t\t}\n \t}\n \n-\tif (trashcan == \"\") {\n-\t\tERR_PRINTS(\"move_to_trash: Could not determine trashcan location\");\n+\t// Issue an error if none of the previous locations is appropriate for the trash can.\n+\tif (trash_can == \"\") {\n+\t\tERR_PRINTS(\"move_to_trash: Could not determine the trash can location\");\n \t\treturn FAILED;\n \t}\n \n+\t// Create needed directories for decided trash can location.\n \tList<String> args;\n \targs.push_back(\"-p\");\n-\targs.push_back(trashcan);\n-\tError err = execute(\"mkdir\", args, true);\n-\tif (err == OK) {\n-\t\tList<String> args2;\n-\t\targs2.push_back(p_path);\n-\t\targs2.push_back(trashcan);\n-\t\terr = execute(\"mv\", args2, true);\n+\targs.push_back(trash_can);\n+\tint retval;\n+\tError err = execute(\"mkdir\", args, true, NULL, NULL, &retval);","path":"platform/x11/os_x11.cpp","position":58,"original_position":58,"commit_id":"a4a534f505e65038dd165fe54ce1e82368cc7818","original_commit_id":"a4a534f505e65038dd165fe54ce1e82368cc7818","user":{"login":"akien-mga","id":4701338,"avatar_url":"https://avatars0.githubusercontent.com/u/4701338?v=4","gravatar_id":"","url":"https://api.github.com/users/akien-mga","html_url":"https://github.com/akien-mga","followers_url":"https://api.github.com/users/akien-mga/followers","following_url":"https://api.github.com/users/akien-mga/following{/other_user}","gists_url":"https://api.github.com/users/akien-mga/gists{/gist_id}","starred_url":"https://api.github.com/users/akien-mga/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/akien-mga/subscriptions","organizations_url":"https://api.github.com/users/akien-mga/orgs","repos_url":"https://api.github.com/users/akien-mga/repos","events_url":"https://api.github.com/users/akien-mga/events{/privacy}","received_events_url":"https://api.github.com/users/akien-mga/received_events","type":"User","site_admin":false},"body":"The `::` in `::rename` is used to access the outer scope, and thus not call `DirAccessUnix::rename` recursively but instead call the stdio `rename` method: http://www.cplusplus.com/reference/cstdio/rename/","created_at":"2018-02-15T09:17:32Z","updated_at":"2018-02-15T09:17:32Z","html_url":"https://github.com/godotengine/godot/pull/16675#discussion_r168415723","pull_request_url":"https://api.github.com/repos/godotengine/godot/pulls/16675","author_association":"OWNER","_links":{"self":{"href":"https://api.github.com/repos/godotengine/godot/pulls/comments/168415723"},"html":{"href":"https://github.com/godotengine/godot/pull/16675#discussion_r168415723"},"pull_request":{"href":"https://api.github.com/repos/godotengine/godot/pulls/16675"}},"in_reply_to_id":168123231},"pull_request":{"url":"https://api.github.com/repos/godotengine/godot/pulls/16675","id":168992147,"html_url":"https://github.com/godotengine/godot/pull/16675","diff_url":"https://github.com/godotengine/godot/pull/16675.diff","patch_url":"https://github.com/godotengine/godot/pull/16675.patch","issue_url":"https://api.github.com/repos/godotengine/godot/issues/16675","number":16675,"state":"open","locked":false,"title":"[X11] Improving error detection in move_to_trash","user":{"login":"Iskustvo","id":23140889,"avatar_url":"https://avatars3.githubusercontent.com/u/23140889?v=4","gravatar_id":"","url":"https://api.github.com/users/Iskustvo","html_url":"https://github.com/Iskustvo","followers_url":"https://api.github.com/users/Iskustvo/followers","following_url":"https://api.github.com/users/Iskustvo/following{/other_user}","gists_url":"https://api.github.com/users/Iskustvo/gists{/gist_id}","starred_url":"https://api.github.com/users/Iskustvo/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Iskustvo/subscriptions","organizations_url":"https://api.github.com/users/Iskustvo/orgs","repos_url":"https://api.github.com/users/Iskustvo/repos","events_url":"https://api.github.com/users/Iskustvo/events{/privacy}","received_events_url":"https://api.github.com/users/Iskustvo/received_events","type":"User","site_admin":false},"body":"Hi, this is my first PR here, so I would appreciate any advice or criticism.\r\nBeside correctness of the code, that includes the coding guidelines, branch naming, commit message and pretty much anything you think I could've done better :D\r\n\r\nOS: Arch Linux\r\n(Other info about the system isn't necessary for this issue)\r\n\r\nI have noticed a small issue in x11's implementation of **move_to_trash** function.\r\n\r\nWhen deleting the asset from the editor, I have noticed that the asset wasn't actually deleted but just moved to some other directory which should represent the \"Trash can\". In my case, that happened to be \"${HOME}/.local/share/Trash/files\". This behavior was undesirable to me and I wanted to actually delete the asset. My idea was to create symbolic(soft) link to /dev/null that will be named \"Trash\", so that when Godot tries to move the asset to the \"Trash\" directory, it will actually be deleted. After creation of that symbolic link, I've noticed that Godot now couldn't delete the assets anymore. In this case, the engine acted like it did the job well, no errors were reported, but the asset remained untouched. After that I've analyzed the code and noticed that in the **move_to_trash** function, execution of both **mkdir** and **mv** are succeeding, but their return values were never checked to see if they did the job well.\r\nMy PR will include the checking of returned values from **mkdir** and **mv** in order to give the user correct error report in similar cases. Beside that, I have added a few comments in the function, mainly for myself but I think it can be useful for others.","created_at":"2018-02-14T00:16:40Z","updated_at":"2018-02-15T09:17:32Z","closed_at":null,"merged_at":null,"merge_commit_sha":"657c1e154ebf7c8c8858ab1c6beefa4f75a09c8e","assignee":null,"assignees":[],"requested_reviewers":[],"requested_teams":[],"labels":[{"id":73330066,"url":"https://api.github.com/repos/godotengine/godot/labels/enhancement","name":"enhancement","color":"44bbff","default":true},{"id":279097326,"url":"https://api.github.com/repos/godotengine/godot/labels/platform:linuxbsd","name":"platform:linuxbsd","color":"dddddd","default":false},{"id":279097966,"url":"https://api.github.com/repos/godotengine/godot/labels/topic:core","name":"topic:core","color":"ffdd44","default":false}],"milestone":{"url":"https://api.github.com/repos/godotengine/godot/milestones/7","html_url":"https://github.com/godotengine/godot/milestone/7","labels_url":"https://api.github.com/repos/godotengine/godot/milestones/7/labels","id":2529217,"number":7,"title":"3.1","description":"","creator":{"login":"akien-mga","id":4701338,"avatar_url":"https://avatars0.githubusercontent.com/u/4701338?v=4","gravatar_id":"","url":"https://api.github.com/users/akien-mga","html_url":"https://github.com/akien-mga","followers_url":"https://api.github.com/users/akien-mga/followers","following_url":"https://api.github.com/users/akien-mga/following{/other_user}","gists_url":"https://api.github.com/users/akien-mga/gists{/gist_id}","starred_url":"https://api.github.com/users/akien-mga/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/akien-mga/subscriptions","organizations_url":"https://api.github.com/users/akien-mga/orgs","repos_url":"https://api.github.com/users/akien-mga/repos","events_url":"https://api.github.com/users/akien-mga/events{/privacy}","received_events_url":"https://api.github.com/users/akien-mga/received_events","type":"User","site_admin":false},"open_issues":744,"closed_issues":179,"state":"open","created_at":"2017-05-18T13:33:56Z","updated_at":"2018-02-15T09:01:04Z","due_on":null,"closed_at":null},"commits_url":"https://api.github.com/repos/godotengine/godot/pulls/16675/commits","review_comments_url":"https://api.github.com/repos/godotengine/godot/pulls/16675/comments","review_comment_url":"https://api.github.com/repos/godotengine/godot/pulls/comments{/number}","comments_url":"https://api.github.com/repos/godotengine/godot/issues/16675/comments","statuses_url":"https://api.github.com/repos/godotengine/godot/statuses/a4a534f505e65038dd165fe54ce1e82368cc7818","head":{"label":"Iskustvo:improved_error_detectioin_in_move-to-trash_for_Linux","ref":"improved_error_detectioin_in_move-to-trash_for_Linux","sha":"a4a534f505e65038dd165fe54ce1e82368cc7818","user":{"login":"Iskustvo","id":23140889,"avatar_url":"https://avatars3.githubusercontent.com/u/23140889?v=4","gravatar_id":"","url":"https://api.github.com/users/Iskustvo","html_url":"https://github.com/Iskustvo","followers_url":"https://api.github.com/users/Iskustvo/followers","following_url":"https://api.github.com/users/Iskustvo/following{/other_user}","gists_url":"https://api.github.com/users/Iskustvo/gists{/gist_id}","starred_url":"https://api.github.com/users/Iskustvo/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Iskustvo/subscriptions","organizations_url":"https://api.github.com/users/Iskustvo/orgs","repos_url":"https://api.github.com/users/Iskustvo/repos","events_url":"https://api.github.com/users/Iskustvo/events{/privacy}","received_events_url":"https://api.github.com/users/Iskustvo/received_events","type":"User","site_admin":false},"repo":{"id":121436028,"name":"godot","full_name":"Iskustvo/godot","owner":{"login":"Iskustvo","id":23140889,"avatar_url":"https://avatars3.githubusercontent.com/u/23140889?v=4","gravatar_id":"","url":"https://api.github.com/users/Iskustvo","html_url":"https://github.com/Iskustvo","followers_url":"https://api.github.com/users/Iskustvo/followers","following_url":"https://api.github.com/users/Iskustvo/following{/other_user}","gists_url":"https://api.github.com/users/Iskustvo/gists{/gist_id}","starred_url":"https://api.github.com/users/Iskustvo/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/Iskustvo/subscriptions","organizations_url":"https://api.github.com/users/Iskustvo/orgs","repos_url":"https://api.github.com/users/Iskustvo/repos","events_url":"https://api.github.com/users/Iskustvo/events{/privacy}","received_events_url":"https://api.github.com/users/Iskustvo/received_events","type":"User","site_admin":false},"private":false,"html_url":"https://github.com/Iskustvo/godot","description":"Godot Engine – Multi-platform 2D and 3D game engine","fork":true,"url":"https://api.github.com/repos/Iskustvo/godot","forks_url":"https://api.github.com/repos/Iskustvo/godot/forks","keys_url":"https://api.github.com/repos/Iskustvo/godot/keys{/key_id}","collaborators_url":"https://api.github.com/repos/Iskustvo/godot/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/Iskustvo/godot/teams","hooks_url":"https://api.github.com/repos/Iskustvo/godot/hooks","issue_events_url":"https://api.github.com/repos/Iskustvo/godot/issues/events{/number}","events_url":"https://api.github.com/repos/Iskustvo/godot/events","assignees_url":"https://api.github.com/repos/Iskustvo/godot/assignees{/user}","branches_url":"https://api.github.com/repos/Iskustvo/godot/branches{/branch}","tags_url":"https://api.github.com/repos/Iskustvo/godot/tags","blobs_url":"https://api.github.com/repos/Iskustvo/godot/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/Iskustvo/godot/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/Iskustvo/godot/git/refs{/sha}","trees_url":"https://api.github.com/repos/Iskustvo/godot/git/trees{/sha}","statuses_url":"https://api.github.com/repos/Iskustvo/godot/statuses/{sha}","languages_url":"https://api.github.com/repos/Iskustvo/godot/languages","stargazers_url":"https://api.github.com/repos/Iskustvo/godot/stargazers","contributors_url":"https://api.github.com/repos/Iskustvo/godot/contributors","subscribers_url":"https://api.github.com/repos/Iskustvo/godot/subscribers","subscription_url":"https://api.github.com/repos/Iskustvo/godot/subscription","commits_url":"https://api.github.com/repos/Iskustvo/godot/commits{/sha}","git_commits_url":"https://api.github.com/repos/Iskustvo/godot/git/commits{/sha}","comments_url":"https://api.github.com/repos/Iskustvo/godot/comments{/number}","issue_comment_url":"https://api.github.com/repos/Iskustvo/godot/issues/comments{/number}","contents_url":"https://api.github.com/repos/Iskustvo/godot/contents/{+path}","compare_url":"https://api.github.com/repos/Iskustvo/godot/compare/{base}...{head}","merges_url":"https://api.github.com/repos/Iskustvo/godot/merges","archive_url":"https://api.github.com/repos/Iskustvo/godot/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/Iskustvo/godot/downloads","issues_url":"https://api.github.com/repos/Iskustvo/godot/issues{/number}","pulls_url":"https://api.github.com/repos/Iskustvo/godot/pulls{/number}","milestones_url":"https://api.github.com/repos/Iskustvo/godot/milestones{/number}","notifications_url":"https://api.github.com/repos/Iskustvo/godot/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/Iskustvo/godot/labels{/name}","releases_url":"https://api.github.com/repos/Iskustvo/godot/releases{/id}","deployments_url":"https://api.github.com/repos/Iskustvo/godot/deployments","created_at":"2018-02-13T20:59:40Z","updated_at":"2018-02-13T20:59:49Z","pushed_at":"2018-02-13T23:08:41Z","git_url":"git://github.com/Iskustvo/godot.git","ssh_url":"git@github.com:Iskustvo/godot.git","clone_url":"https://github.com/Iskustvo/godot.git","svn_url":"https://github.com/Iskustvo/godot","homepage":"https://godotengine.org","size":225671,"stargazers_count":0,"watchers_count":0,"language":"C++","has_issues":false,"has_projects":true,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":0,"mirror_url":null,"archived":false,"open_issues_count":0,"license":null,"forks":0,"open_issues":0,"watchers":0,"default_branch":"master"}},"base":{"label":"godotengine:master","ref":"master","sha":"e7351ecdf5f14f1b6a8733f91fd90646406d77a5","user":{"login":"godotengine","id":6318500,"avatar_url":"https://avatars2.githubusercontent.com/u/6318500?v=4","gravatar_id":"","url":"https://api.github.com/users/godotengine","html_url":"https://github.com/godotengine","followers_url":"https://api.github.com/users/godotengine/followers","following_url":"https://api.github.com/users/godotengine/following{/other_user}","gists_url":"https://api.github.com/users/godotengine/gists{/gist_id}","starred_url":"https://api.github.com/users/godotengine/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/godotengine/subscriptions","organizations_url":"https://api.github.com/users/godotengine/orgs","repos_url":"https://api.github.com/users/godotengine/repos","events_url":"https://api.github.com/users/godotengine/events{/privacy}","received_events_url":"https://api.github.com/users/godotengine/received_events","type":"Organization","site_admin":false},"repo":{"id":15634981,"name":"godot","full_name":"godotengine/godot","owner":{"login":"godotengine","id":6318500,"avatar_url":"https://avatars2.githubusercontent.com/u/6318500?v=4","gravatar_id":"","url":"https://api.github.com/users/godotengine","html_url":"https://github.com/godotengine","followers_url":"https://api.github.com/users/godotengine/followers","following_url":"https://api.github.com/users/godotengine/following{/other_user}","gists_url":"https://api.github.com/users/godotengine/gists{/gist_id}","starred_url":"https://api.github.com/users/godotengine/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/godotengine/subscriptions","organizations_url":"https://api.github.com/users/godotengine/orgs","repos_url":"https://api.github.com/users/godotengine/repos","events_url":"https://api.github.com/users/godotengine/events{/privacy}","received_events_url":"https://api.github.com/users/godotengine/received_events","type":"Organization","site_admin":false},"private":false,"html_url":"https://github.com/godotengine/godot","description":"Godot Engine – Multi-platform 2D and 3D game engine","fork":false,"url":"https://api.github.com/repos/godotengine/godot","forks_url":"https://api.github.com/repos/godotengine/godot/forks","keys_url":"https://api.github.com/repos/godotengine/godot/keys{/key_id}","collaborators_url":"https://api.github.com/repos/godotengine/godot/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/godotengine/godot/teams","hooks_url":"https://api.github.com/repos/godotengine/godot/hooks","issue_events_url":"https://api.github.com/repos/godotengine/godot/issues/events{/number}","events_url":"https://api.github.com/repos/godotengine/godot/events","assignees_url":"https://api.github.com/repos/godotengine/godot/assignees{/user}","branches_url":"https://api.github.com/repos/godotengine/godot/branches{/branch}","tags_url":"https://api.github.com/repos/godotengine/godot/tags","blobs_url":"https://api.github.com/repos/godotengine/godot/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/godotengine/godot/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/godotengine/godot/git/refs{/sha}","trees_url":"https://api.github.com/repos/godotengine/godot/git/trees{/sha}","statuses_url":"https://api.github.com/repos/godotengine/godot/statuses/{sha}","languages_url":"https://api.github.com/repos/godotengine/godot/languages","stargazers_url":"https://api.github.com/repos/godotengine/godot/stargazers","contributors_url":"https://api.github.com/repos/godotengine/godot/contributors","subscribers_url":"https://api.github.com/repos/godotengine/godot/subscribers","subscription_url":"https://api.github.com/repos/godotengine/godot/subscription","commits_url":"https://api.github.com/repos/godotengine/godot/commits{/sha}","git_commits_url":"https://api.github.com/repos/godotengine/godot/git/commits{/sha}","comments_url":"https://api.github.com/repos/godotengine/godot/comments{/number}","issue_comment_url":"https://api.github.com/repos/godotengine/godot/issues/comments{/number}","contents_url":"https://api.github.com/repos/godotengine/godot/contents/{+path}","compare_url":"https://api.github.com/repos/godotengine/godot/compare/{base}...{head}","merges_url":"https://api.github.com/repos/godotengine/godot/merges","archive_url":"https://api.github.com/repos/godotengine/godot/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/godotengine/godot/downloads","issues_url":"https://api.github.com/repos/godotengine/godot/issues{/number}","pulls_url":"https://api.github.com/repos/godotengine/godot/pulls{/number}","milestones_url":"https://api.github.com/repos/godotengine/godot/milestones{/number}","notifications_url":"https://api.github.com/repos/godotengine/godot/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/godotengine/godot/labels{/name}","releases_url":"https://api.github.com/repos/godotengine/godot/releases{/id}","deployments_url":"https://api.github.com/repos/godotengine/godot/deployments","created_at":"2014-01-04T16:05:36Z","updated_at":"2018-02-15T09:10:42Z","pushed_at":"2018-02-15T09:01:11Z","git_url":"git://github.com/godotengine/godot.git","ssh_url":"git@github.com:godotengine/godot.git","clone_url":"https://github.com/godotengine/godot.git","svn_url":"https://github.com/godotengine/godot","homepage":"https://godotengine.org","size":226798,"stargazers_count":13252,"watchers_count":13252,"language":"C++","has_issues":true,"has_projects":true,"has_downloads":true,"has_wiki":false,"has_pages":false,"forks_count":2868,"mirror_url":null,"archived":false,"open_issues_count":3010,"license":{"key":"other","name":"Other","spdx_id":null,"url":null},"forks":2868,"open_issues":3010,"watchers":13252,"default_branch":"master"}},"_links":{"self":{"href":"https://api.github.com/repos/godotengine/godot/pulls/16675"},"html":{"href":"https://github.com/godotengine/godot/pull/16675"},"issue":{"href":"https://api.github.com/repos/godotengine/godot/issues/16675"},"comments":{"href":"https://api.github.com/repos/godotengine/godot/issues/16675/comments"},"review_comments":{"href":"https://api.github.com/repos/godotengine/godot/pulls/16675/comments"},"review_comment":{"href":"https://api.github.com/repos/godotengine/godot/pulls/comments{/number}"},"commits":{"href":"https://api.github.com/repos/godotengine/godot/pulls/16675/commits"},"statuses":{"href":"https://api.github.com/repos/godotengine/godot/statuses/a4a534f505e65038dd165fe54ce1e82368cc7818"}},"author_association":"NONE"}}
{"actor":{"display_login":"akien-mga"}}