Corrected component

Corrected component

I'm adding a dialog box that asks if the user wants to delete the file from 1. Disk; 2. Disk and Project; 3. Disk, Project and Version Control. I will be interfacing it with proper methods then. Would ianjuta_vcs_remove be useful for removing the file from the vcs repository?

(In reply to comment #3)
> I'm adding a dialog box that asks if the user wants to delete the file from 1.
> Disk; 2. Disk and Project; 3. Disk, Project and Version Control. I will be
> interfacing it with proper methods then. Would ianjuta_vcs_remove be useful for
> removing the file from the vcs repository?

I think we can perhaps add a confirmation dialog before deleting the file with by example a check boxes (checked by default) to remove the file from the project and the version control system.

The necessary interface function is missing from the project manager interface.

I'm using g_remove; git_ivcs_remove/subversion_ivcs_remove and am adding a function to the project manager to remove the file. Is that alright?

While g_remove() would work it only works only for local files and doesn't have a good error handling. Better us g_file_trash() or g_file_remove() though I would prefer g_file_trash() in that case. It should also be easier because you a GFile* from the file-manager anyway.

For removing from version control you should first query the loaded ivcs plugin (that is done in the file-manager somewhere already when it loads the icons for the status) and call ivcs_remove() for that plugin. Mind that you call the interface and not the actual plugin because there might be more version control plugins than just git and svn and you don't have access to the implementation of the plugins. See the reference documentation for details.

I am wondering why there is no ianjuta_project_manager_remove_source() but you should be able to add it. The interfaces are defined in libanjuta/interface/libanjuta.idl.

(In reply to comment #6)
> I am wondering why there is no ianjuta_project_manager_remove_source() but you
> should be able to add it. The interfaces are defined in
> libanjuta/interface/libanjuta.idl.

I can add this ianjuta_project_manager_remove_source. Indeed it can be useful, I suppose it's not here because nobody has needed it up to now.

It's not completely obvious though because a source file could belong to several targets, in this case we have to remove it from all targets. This is not straight forward because you get only the source file belonging to the first target.

(In reply to comment #6)
> For removing from version control you should first query the loaded ivcs plugin
> (that is done in the file-manager somewhere already when it loads the icons for
> the status) and call ivcs_remove() for that plugin. Mind that you call the
> interface and not the actual plugin because there might be more version control
> plugins than just git and svn and you don't have access to the implementation
> of the plugins. See the reference documentation for details.

I'm able to figure out the vcs being used. Does calling ianjuta_vcs_remove() with the current vcs as parameter automatically call the proper remove() function for the vcs? And, if yes, it requires a GList* of files. So does it mean I must put the current (selected) GFile into a GList and then pass it?

(In reply to comment #8)
> I'm able to figure out the vcs being used. Does calling ianjuta_vcs_remove()
> with the current vcs as parameter automatically call the proper remove()
> function for the vcs?

Yes.

> And, if yes, it requires a GList* of files. So does it
> mean I must put the current (selected) GFile into a GList and then pass it?

Yes, I think so.

I have just committed a change adding a new function ianjuta_project_manager_remove_file to remove a file from the project. It should be enough to fully implement this feature.

Created attachment 248914 [details] [review]
Added a dialog box to delete files

Thanks for the addition, Sébastien. I'm putting up an incomplete patch; I need some suggestions.
1. I have used a global variable ivcs_global in file-manager/plugins.c to store the currently loaded vcs plugin. Instead of doing this, I want to use the get_vcs_plugin() function, but it requires me to pass a root_uri. How do I get the root_uri?

2. Is there a way to store the currently loaded project manager? To use the ianjuta_project_manager_remove_file() function, I have to pass it an IAnjutaProjectManager, but I don't know how to get it.

Review of attachment 248914 [details] [review]:

Thanks for the patches - see comments below.

::: plugins/file-manager/plugin.c
@@ +59,2 @@
 static gpointer parent_class;
+static IAnjutaVcs* get_vcs_plugin(AnjutaFileManager* file_manager, const gchar* root_uri);

You can simply add a file_model_get_ivcs() method and receive the current ivcs from the file-model so no need to receice it twice from the shell.

@@ +70,3 @@
 };
 
+struct _DeleteDialog

I don't really think you need to have an extra struct here - just declare the variables locally in the on_file_manager_delete() method.

@@ +117,3 @@
+	{
+		GError** err;
+		AnjutaFileView* fv = file_manager->fv;

Please move selected_file to the top_level of this method and avoid duplicating it for the three cases.

@@ +119,3 @@
+		AnjutaFileView* fv = file_manager->fv;
+		GFile* selected_file = file_view_get_selected (fv);
+		g_file_trash (selected_file, NULL, err);

You need to add a g_object_unref(selected_file) to avoid a memory leak.

@@ +136,3 @@
+		IAnjutaVcs* current_vcs;
+		GFile* selected_file = file_view_get_selected (fv);
+		GList* selected_file_list;

Please add a = NULL to avoid segfaults. An empty GList* must be NULL.

@@ +149,3 @@
+
+static void
+on_file_manager_delete_init (GtkAction* action, AnjutaFileManager* file_manager)

Please merge this with on_file_manager_delete()

> 2. Is there a way to store the currently loaded project manager? To use the
> ianjuta_project_manager_remove_file() function, I have to pass it an
> IAnjutaProjectManager, but I don't know how to get it.

ianjuta_shell_get_interface (anjuta_plugin_get_shell (plugin), IAnjutaProjectManager, NULL) will return the current Project Manager or NULL.

Created attachment 248927 [details] [review]
Added a dialog box to delete files from file-manager

I've added the modifications suggested by Johannes.

Review of attachment 248927 [details] [review]:

Looks good Overall - see comments.

::: plugins/file-manager/plugin.c
@@ +60,2 @@
 static gpointer parent_class;
+static IAnjutaVcs* get_vcs_plugin(AnjutaFileManager* file_manager, const gchar* root_uri);

This line is obsolete, right?

@@ +141,3 @@
+		{
+			g_file_trash (selected_file, NULL, err);
+			g_object_unref(selected_file);

If you unref the file here you won't have it in the vcs part anymore. So move this at the end of the method (checking that the file isn't NULL sounds like a good idea, too).

Created attachment 248930 [details] [review]
Added a dialog box to delete files from file-manager

Added the NULL check, removed the unwanted declaration.

Review of attachment 248930 [details] [review]:

Looks very good now:
 * Error handling should be added
 * The dialog should show something like "Are you sure you want to delete <file.xy>?" (g_file_get_display_name() and g_strdup_printf() are your friends...)
 * The checkbox for "delete from disk" should be removed. The file is always deleted from disk and just project and vcs should be options.
 * The checkbox for project should be insensitive when no project is open
 * The checkbox for vcs should be insensitive when there is no vcs plugin loaded AND when the vcs does not track the file. Therefore you might have to add another method to the file-model to be able to receive the vcs status.

Thanks for your work until now - sorry that I come up with all this things now but they caught my eye when I first really tried this patch in anjuta.

::: plugins/file-manager/plugin.c
@@ +114,3 @@
+	gint response;
+	GError* error = NULL;
+	GError** err = NULL;

You should move the declaration of second error down it and should also be GError* err = NULL, not GError**

@@ +139,3 @@
+		if(gtk_toggle_button_get_active (delete_from_disk))
+		{
+			g_file_trash (selected_file, NULL, err);

Please add error handling here, e.g. show a dialog when the file cannot be deleted.

@@ +147,3 @@
+			                                                        IAnjutaProjectManager,
+			                                                        NULL);
+			ianjuta_project_manager_remove_file (pm, selected_file, err);

Same for the error handling here

@@ +165,3 @@
+			selected_file_list = g_list_append (selected_file_list, selected_file);
+
+			ianjuta_vcs_remove (IANJUTA_VCS (current_vcs), selected_file_list, notify, err);

And here...

bugzilla.gnome.org is being replaced by gitlab.gnome.org. We are closing all
old feature requests in Bugzilla which have not seen updates for many years.

If you are still requesting this feature in a currently supported version of GNOME (currently that would be 3.38), then please feel free to report it at https://gitlab.gnome.org/GNOME/anjuta/-/issues/

Thank you for reporting this issue and we are sorry it could not be implemented.