GNOME Bugzilla – Bug 766447
Can create a project for a directory that already exists and overwrites files!
Last modified: 2016-08-28 19:48:38 UTC
If a directory already exists, the user should be prompted if he/she wishes to overwrite the existing directory or not. If yes then continue with current code, else change directory name.
@hergertme: Any advice how i should proceed with this?
Created attachment 328071 [details] [review] User can now choose whether to overwrite or not @hergertme : Pls review. I havent removed the comments. I'll refactor if all's ok. Also the dialog alignment is not centered
Review of attachment 328071 [details] [review]: ::: plugins/create-project/gbp-create-project-widget.c @@ -674,3 +676,3 @@ g_task_set_task_data (task, g_file_new_for_path (path), g_object_unref); - ide_project_template_expand_async (template, + if( access( path, F_OK ) != -1 ) if (g_file_test (path, G_FILE_TEST_IS_DIR)) @@ -676,1 +678,5 @@ - ide_project_template_expand_async (template, + if( access( path, F_OK ) != -1 ) + { + g_print("Path is already present : %s\n",path); ... 2 more ... No need to assign NULL. @@ +681,3 @@ + + GtkWidget *dialog = NULL; + gint response; Remove spaces between gint and response. I applaud the effort, but it would just take too much time for us to use this convention everywhere. @@ -676,1 +678,8 @@ - ide_project_template_expand_async (template, + if( access( path, F_OK ) != -1 ) + { + g_print("Path is already present : %s\n",path); ... 5 more ... We don't use , when declaring variables. @@ -676,1 +678,10 @@ - ide_project_template_expand_async (template, + if( access( path, F_OK ) != -1 ) + { + g_print("Path is already present : %s\n",path); ... 7 more ... Use something like: g_autofree gchar *name = g_path_get_basename (path); dialog = gtk_message_dialog_new (GTK_WINDOW (window), GTK_DIALOG_MODAL, GTK_MESSAGE_QUESTION, GTK_BUTTONS_NONE, _("The directory “%s” already exists. Would you like to overwrite it?"), name); gtk_dialog_add_buttons (GTK_DIALOG (dialog), _("Cancel"), GTK_RESPONSE_CANCEL, _("Overwrite directory"), GTK_RESPONSE_OK, NULL); @@ -676,1 +678,44 @@ - ide_project_template_expand_async (template, + if( access( path, F_OK ) != -1 ) + { + g_print("Path is already present : %s\n",path); ... 41 more ... Maybe just focus the entry (which should also select all the text) @@ -676,1 +678,47 @@ - ide_project_template_expand_async (template, + if( access( path, F_OK ) != -1 ) + { + g_print("Path is already present : %s\n",path); ... 44 more ... Use gtk_widget_destroy(), otherwise we leak the dialog.
Created attachment 328112 [details] [review] Bug 766447 - Can create a project for a directory that already exists User is now shown a dialog box and he/she can choose whether to overwrite existing directory or change directory name.
Created attachment 328115 [details] [review] Bug 766447 - Can create a project for a directory that already exists User is now shown a dialog box and he/she can choose whether to overwrite existing directory or change directory name.
Created attachment 328116 [details] [review] Bug 766447 - Can create a project for a directory that already exists User is now shown a dialog box and he/she can choose whether to overwrite existing directory or change directory name. https://bugzilla.gnome.org/show_bug.cgi?id=766447 Bug 766447 - Can create a project for a directory that already exists User is now shown a dialog box and he/she can choose whether to overwrite existing directory or change directory name. Removeda redundant debug statement.
Created attachment 328119 [details] [review] Bug 766447 - Can create a project for a directory that already exists User is now shown a dialog box and he/she can choose whether to overwrite existing directory or change directory name. https://bugzilla.gnome.org/show_bug.cgi?id=766447 Bug 766447 - Can create a project for a directory that already exists User is now shown a dialog box and he/she can choose whether to overwrite existing directory or change directory name. Removeda redundant debug statement. https://bugzilla.gnome.org/show_bug.cgi?id=766447 Bug 766447 - Can create a project for a directory that already exists User is now shown a dialog box and he/she can choose whether to overwrite existing directory or change directory name. Added a space after typecasting.
sorry for the multiple patches..! Kindly review :)
Created attachment 328300 [details] [review] Bug 766447 Trying to pass struct to callback function. gint val is used to check if struct is passed correctly. However, getting an error in the callback function. sys:1: Warning: g_object_ref: assertion 'G_IS_OBJECT (object)' failed Segmentation fault (core dumped)
Review of attachment 328300 [details] [review]: Glad to see you making progress, keep working at it! ::: plugins/create-project/gbp-create-project-widget.c @@ -600,3 +645,3 @@ { - g_autoptr(GTask) task = NULL; - g_autoptr(GHashTable) params = NULL; + struct raunaq *r; + r->task = NULL; You can't dereference `r` here until you've allocated memory for `r`. r = g_new0 (struct raunaq, 1); Using g_new0 will allocate space for a structure and zero all the memory for you. (See `man calloc` for a similar function from libc). @@ -671,3 +720,3 @@ } - task = g_task_new (self, cancellable, callback, user_data); + r->task = g_task_new (self, cancellable, callback, user_data); Instead of making the struct the "owner" of the task, I would make the task the owner of the struct. Add a cleanup function like: static void raunaq_free (gpointer ptr) { struct raunaq *data = ptr; g_clear_pointer (&data->params, g_hash_table_unref); g_clear_object (&data->template); g_clear_object (&data->file); g_free (data); } And then set the task data like: g_task_set_task_data(task, r, raunaq_free); instead of setting the project file as the task data. In the callback you can get the task data like: struct raunaq *r = g_task_get_task_data (task);
Created attachment 328362 [details] [review] Bug 766447 - Can create a project for a directory that already exists User is now shown a dialog box and he/she can choose whether to overwrite existing directory or change directory name. https://bugzilla.gnome.org/show_bug.cgi?id=766447 Bug 766447 - Can create a project for a directory that already exists User is now shown a dialog box and he/she can choose whether to overwrite existing directory or change directory name. Removeda redundant debug statement. https://bugzilla.gnome.org/show_bug.cgi?id=766447 Bug 766447 - Can create a project for a directory that already exists User is now shown a dialog box and he/she can choose whether to overwrite existing directory or change directory name. Added a space after typecasting. https://bugzilla.gnome.org/show_bug.cgi?id=766447 Bug 766447 Trying to pass struct to callback function. gint val is used to check if struct is passed correctly. However, getting an error in the callback function. sys:1: Warning: g_object_ref: assertion 'G_IS_OBJECT (object)' failed Segmentation fault (core dumped) https://bugzilla.gnome.org/show_bug.cgi?id=766447 Bug 766447 - Can create a project for a directory that already exists User is now shown a dialog box and he/she can choose whether to overwrite existing directory or change directory name. Added a space after typecasting. Struct is made the owner of task. Memory allocation was the trick. Failure is also added in case "dont overwrite" is pressed.
@hergertme : pls review.. It works now.. Added the failure part too. I guess mem allocation did the trick. The name of the struct needs to be changed. Right now it's "raunaq" Thanks.
Created attachment 328649 [details] [review] Bug 766447 - Can create a project for a directory that already exists User is now shown a dialog box and he/she can choose whether to overwrite existing directory or change directory name. https://bugzilla.gnome.org/show_bug.cgi?id=766447 Bug 766447 - Can create a project for a directory that already exists User is now shown a dialog box and he/she can choose whether to overwrite existing directory or change directory name. Removeda redundant debug statement. https://bugzilla.gnome.org/show_bug.cgi?id=766447 Bug 766447 - Can create a project for a directory that already exists User is now shown a dialog box and he/she can choose whether to overwrite existing directory or change directory name. Added a space after typecasting. https://bugzilla.gnome.org/show_bug.cgi?id=766447 Bug 766447 Trying to pass struct to callback function. gint val is used to check if struct is passed correctly. However, getting an error in the callback function. sys:1: Warning: g_object_ref: assertion 'G_IS_OBJECT (object)' failed Segmentation fault (core dumped) https://bugzilla.gnome.org/show_bug.cgi?id=766447 Bug 766447 - Can create a project for a directory that already exists User is now shown a dialog box and he/she can choose whether to overwrite existing directory or change directory name. Added a space after typecasting. Struct is made the owner of task. Memory allocation was the trick. Failure is also added in case "dont overwrite" is pressed. https://bugzilla.gnome.org/show_bug.cgi?id=766447 Bug 766447 - Can create a project for a directory that already exists and overwrites files Now the dialog is centered to the application window. Not the screen.
Comment on attachment 328649 [details] [review] Bug 766447 - Can create a project for a directory that already exists This was a bit obsolete since we went through so much revamp of the project creation stuff this cycle. So I've put together a much smaller patch to address the issue at hand. We can look at updating it to be asynchronous however if someone would like to do that.
Created attachment 334317 [details] [review] create-project: check that the directory is available This is a bit non-ideal since we do synchronous I/O to check if the directory exists, but it solves the problem at hand for now.
Attachment 334317 [details] pushed as c3199e8 - create-project: check that the directory is available