GNOME Bugzilla – Bug 769602
files-view: use GFile utilities
Last modified: 2016-08-21 16:45:26 UTC
set_up_scripts_directory_global() currently uses the POSIX file interface, which is inconsistent with the rest of the code. This commit rewrites the parts of the function to use the GFile API.
Created attachment 332880 [details] [review] files-view: use GFile utilities
Review of attachment 332880 [details] [review]: Thanks for doing this and sorry for a long time to review! Still needs a bit of work though. Also, it'd be cool to see the other utility functions like "g_build_filename" be replaced as well. ::: src/nautilus-files-view.c @@ +2589,3 @@ message = _("Nautilus 3.6 deprecated this directory and tried migrating " "this configuration to ~/.local/share/nautilus"); if (!g_file_test (updated, G_FILE_TEST_EXISTS)) { I would suggest replacing this and and the other "Utility Functions" with GFile API. In this case it'd be g_file_query_exists. @@ +2598,1 @@ + if (!g_file_make_directory_with_parents (directory, NULL, NULL)) { I think this condition is wrong. g_mkdir_with_parents will return 0 if the directory was created OR if it already exists. g_file_make_directory_with_parents will fail if the directory exists, won't it?. You could add an error parameter and also check for IO_ERROR_EXISTS. @@ +2605,3 @@ + NULL, NULL); + + if (g_file_info_has_attribute (info, G_FILE_ATTRIBUTE_UNIX_MODE)) { You should replace info querying and attribute checking / setting with g_file_set_attribute_uint32.
Created attachment 333595 [details] [review] files-view: use GFile utilities set_up_scripts_directory_global() currently uses the POSIX file interface, which is inconsistent with the rest of the code. This commit rewrites the parts of the function to use the GFile API.
Created attachment 333614 [details] [review] files-view: use GFile utilities set_up_scripts_directory_global() currently uses the POSIX file interface, which is inconsistent with the rest of the code. This commit rewrites the parts of the function to use the GFile API.
Review of attachment 333614 [details] [review]: ::: src/nautilus-files-view.c @@ +2586,3 @@ + scripts_directory = g_file_new_for_path (scripts_directory_path); + + if (g_file_query_file_type (old_scripts_directory, use variables, the if is ugly with this identation @@ +2601,3 @@ + g_autoptr (GError) error = NULL; + + if (g_file_make_directory_with_parents (parent, NULL, &error) || put this outside and only check for no error or IO_EXISTS. As a rule of thumb don't put too many calls or weight inside if() conditions, rather use variables to explain what is going on. @@ +2609,3 @@ + NULL, NULL); + + g_error_matches (error, G_IO_ERROR, G_IO_ERROR_EXISTS)) { same as before, and check for errors. @@ +2636,3 @@ } + return (scripts_directory_uri != NULL); not need for parenteses. Also omg to the previous condition here :D
Created attachment 333771 [details] [review] files-view: use GFile utilities set_up_scripts_directory_global() currently uses the POSIX file interface, which is inconsistent with the rest of the code. This commit rewrites the parts of the function to use the GFile API.
Review of attachment 333771 [details] [review]: excellent, thanks a lot!
Attachment 333771 [details] pushed as 672ccbc - files-view: use GFile utilities