Bug 565598 - Allow delete confirmation dialog to be disabled (DeleteResourceAction)
Summary: Allow delete confirmation dialog to be disabled (DeleteResourceAction)
Status: NEW
Alias: None
Product: Platform
Classification: Eclipse Project
Component: IDE (show other bugs)
Version: 4.17   Edit
Hardware: All All
: P3 enhancement (vote)
Target Milestone: ---   Edit
Assignee: Platform-UI-Inbox CLA
QA Contact:
URL:
Whiteboard:
Keywords: usability
Depends on:
Blocks:
 
Reported: 2020-07-28 09:03 EDT by Andrew Obuchowicz CLA
Modified: 2021-02-15 03:32 EST (History)
5 users (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Andrew Obuchowicz CLA 2020-07-28 09:03:06 EDT
I'd like to use some of platforms resource actions such as DeleteResourceAction in a plugin but with the confirmation dialog disabled.

Is there currently a way to do this, from a plugin developers perspective? I will also likely need to do the same for Copy, Cut, etc. 

Additionally, is there desire for a preference to disable the confirmation dialog when running the delete action? Similar to the project explorer inline rename, file deletion might feel a bit snappier without any dialogs.
Comment 1 Lars Vogel CLA 2020-07-28 09:08:57 EDT
Big +1 for the option to remove this confirmation dialog.
Comment 2 Andrey Loskutov CLA 2020-07-28 10:50:46 EDT
(In reply to Lars Vogel from comment #1)
> Big +1 for the option to remove this confirmation dialog.

Please keep the "confirmation" as default and please make sure the preference will read product defaults too (not just instance defaults).
Comment 3 Andrew Obuchowicz CLA 2020-07-28 12:01:23 EDT
(In reply to Andrey Loskutov from comment #2)
> (In reply to Lars Vogel from comment #1)
> > Big +1 for the option to remove this confirmation dialog.
> 
> Please keep the "confirmation" as default and please make sure the
> preference will read product defaults too (not just instance defaults).

+1 it should be entirely optional and customizable for RCP products.

Do you have any tips for making sure the preference reads the product defaults? (I'm not familiar with product defaultp references yet)

Could I just follow the way the inline rename preference is implemented?
Comment 4 Andrey Loskutov CLA 2020-07-29 08:08:30 EDT
(In reply to Andrew Obuchowicz from comment #3)
> Do you have any tips for making sure the preference reads the product
> defaults? (I'm not familiar with product defaultp references yet)

Something like this is needed: https://git.eclipse.org/r/c/platform/eclipse.platform.ui/+/166989

> Could I just follow the way the inline rename preference is implemented?

Haven't checked the code, so no idea.
Comment 5 Lars Vogel CLA 2020-07-31 05:29:07 EDT
Andrew, you can start with implementing this as preference and if it cannot be configured on the product level we can investigate.

From the UI, we should have a dialog similar to the Workspace selection dialog with a checkbox (setting this preference) "Always delete without confirmation".
Comment 6 Andrew Obuchowicz CLA 2020-08-07 13:23:03 EDT
So I started working on this.

There are 2 ways the delete can be performed: via LTK RenameResourcesHandler or as a UI Job that runs a DeleteResourcesOperation.

From what it seems, the LTK approach doesn't let you undo a delete change (easily at least) although I might be missing something. This also

What I propose:
Use LTK to check if a delete operation would cause multiple changes (other files are affected by this delete).
If no other resources are affected, then silently exit the LTK context and let the UI job delete resource operation run.

Alternatively, we could do everything with LTK and try to improve the LTK delete refactoring so that it adds the change to the undo stack? Not sure how difficult this would be.
Comment 7 Andrew Obuchowicz CLA 2020-08-07 13:33:24 EDT
(In reply to Andrew Obuchowicz from comment #6)
> So I started working on this.
> 
> There are 2 ways the delete can be performed: via LTK RenameResourcesHandler
> or as a UI Job that runs a DeleteResourcesOperation.

I meant LTK DeleteResourcesHandler

> 
> From what it seems, the LTK approach doesn't let you undo a delete change
> (easily at least) although I might be missing something. This also
> 

Meant to write: this also means that the inline rename does not currently support undo/redo
Comment 8 Andrew Obuchowicz CLA 2020-08-07 13:58:50 EDT
Update: it turns out you can easily add Change's to the undo stack.

Something like:

//Silently perform the delete without the dialog					RefactoringCore.getUndoManager().aboutToPerformChange(change);
Change undo = change.perform(new NullProgressMonitor());					RefactoringCore.getUndoManager().changePerformed(change, true);					RefactoringCore.getUndoManager().addUndo("Delete resources", undo);

Works well. 

All I need to work on now is the preference + UI change :)
Comment 9 Lars Vogel CLA 2020-10-01 10:55:16 EDT
(In reply to Andrew Obuchowicz from comment #8)
> Update: it turns out you can easily add Change's to the undo stack.
> 
> Something like:
> 
> //Silently perform the delete without the dialog				
> RefactoringCore.getUndoManager().aboutToPerformChange(change);
> Change undo = change.perform(new NullProgressMonitor());				
> RefactoringCore.getUndoManager().changePerformed(change, true);				
> RefactoringCore.getUndoManager().addUndo("Delete resources", undo);
> 
> Works well. 
> 
> All I need to work on now is the preference + UI change :)

Any progress here?
Comment 10 Andrew Obuchowicz CLA 2020-10-01 18:08:20 EDT
(In reply to Lars Vogel from comment #9)
> (In reply to Andrew Obuchowicz from comment #8)
> > Update: it turns out you can easily add Change's to the undo stack.
> > 
> > Something like:
> > 
> > //Silently perform the delete without the dialog				
> > RefactoringCore.getUndoManager().aboutToPerformChange(change);
> > Change undo = change.perform(new NullProgressMonitor());				
> > RefactoringCore.getUndoManager().changePerformed(change, true);				
> > RefactoringCore.getUndoManager().addUndo("Delete resources", undo);
> > 
> > Works well. 
> > 
> > All I need to work on now is the preference + UI change :)
> 
> Any progress here?

I haven't had a chance to make further progress on this topic, but all that I really needed to do was add a new preference & add a checkbox to the delete dialog to enable this preference. Not that hard work IMO but I've been quite busy with university schoolwork.

If someone could lend a hand there, I'll gladly guide & review the necessary gerrit's. 

Maybe this would be a good topic to advertise as a begginer bug? CC: Mickael Istria

Otherwise, I'll try to get this done when I have some more spare time :)
Comment 11 Lars Vogel CLA 2020-10-02 03:52:58 EDT
Chenhui, please have a look. Repo should be eclipse.platform.ui
Comment 12 Mickael Istria CLA 2020-10-02 04:06:36 EDT
(In reply to Lars Vogel from comment #5)
> Andrew, you can start with implementing this as preference and if it cannot
> be configured on the product level we can investigate.

Preferences are usually fine with product customization, with just some care to not directly use InstanceNode but instead use the PreferenceService if possible (as Andrey highlights in the other bug)

> From the UI, we should have a dialog similar to the Workspace selection
> dialog with a checkbox (setting this preference) "Always delete without
> confirmation".

That can be a 2nd step IMO, a preference in the Workspace preference page would be enough.

> From what it seems, the LTK approach doesn't let you undo a delete change (easily at least) although I might be missing something.

It would be nice to investigate what's preventing from undoing a delete here. But I see in comment #6 this can easily be done.


For the UI, I suggest that the popup shows anyway if there is no available undo operation. But if it is possible for users to undo deletion, there is no much point in showing the confirmation popup.
Comment 13 Chenhui Xu CLA 2020-10-31 10:23:48 EDT
Hey, I am new here and trying to start to work on this.

So far I have added a tick box to indicate the preference 'do not show again'.

And my thinking is put a tick box in the dialog 'do not show again' -> if ticked, the boolean variable set to true -> and every time for a delete action, the boolean variable is checked first to see if dialog should pop up.

As for undo delete, I am planning to handle that later.

But I don't know where to add the preference option 'show/hide delete dialog' in the eclipse preference, or where to add the preference boolean variable. And which file is related to this? (right now I am looking in eclipse.platform.ui).

And if anyone knows which Gerrit is similar or related with this, it would be great to put them here.

Thank you :)
Comment 14 Mickael Istria CLA 2020-11-01 10:25:25 EST
(In reply to Chenhui Xu from comment #13)
> As for undo delete, I am planning to handle that later.

IMO, undo delete is a prerequisite to have before we change anything else; and is more difficult so it can become a blocker. I would suggest to start by addressing it first before spending time on other tasks that do depend on it.
 
> But I don't know where to add the preference option 'show/hide delete
> dialog' in the eclipse preference, or where to add the preference boolean
> variable. And which file is related to this? (right now I am looking in
> eclipse.platform.ui).

See ChooseWorkspaceData.writePersistedData() for example.
Comment 15 Eclipse Genie CLA 2020-11-16 02:24:28 EST
New Gerrit change created: https://git.eclipse.org/r/c/platform/eclipse.platform.ui/+/172276
Comment 16 Lars Vogel CLA 2020-11-16 04:48:32 EST
Andrew, Chenhui did implement a preference for this. Can you guide in using it in the org.eclipse.ltk.internal.ui.refactoring.actions.DeleteResourcesHandler

The DeleteResourceAction from org.eclipse.ui.actions seem not to be used.
Comment 17 Andrew Obuchowicz CLA 2020-11-16 11:35:33 EST
(In reply to Lars Vogel from comment #16)
> Andrew, Chenhui did implement a preference for this. Can you guide in using
> it in the
> org.eclipse.ltk.internal.ui.refactoring.actions.DeleteResourcesHandler
> 
> The DeleteResourceAction from org.eclipse.ui.actions seem not to be used.

I would be glad to help give some guidance although I must admit I'm quite busy (especially today) with university work.

I quickly skimmed over the patch and it seems that the patch only deals in platform and a patch for org.eclipse.ltk.internal.ui.refactoring.actions.DeleteResourcesHandler is required.

Perhaps the DeleteResourceHandler needs to check if a boolean was sent in the delete command, the boolean will be something like "showDialog". The booleans value will be the preference that Chenhui implemented.

Then depending on whether this boolean is set to true or not, the following code will run in DeleteResourceHandler:

//Silently perform the delete without the dialog					RefactoringCore.getUndoManager().aboutToPerformChange(change);
Change undo = change.perform(new NullProgressMonitor());					RefactoringCore.getUndoManager().changePerformed(change, true);					RefactoringCore.getUndoManager().addUndo("Delete resources", undo);

There is a class that sends commands from Platform to org.eclipse.ltk to allow Platform to call JDT (LTK) without creating a circular dependency. I forget the name at the moment, but this class would probably have to be modified to take in the "showDialog" parameter in its delete method.
Comment 18 Andrew Obuchowicz CLA 2020-11-16 11:45:35 EST
> There is a class that sends commands from Platform to org.eclipse.ltk to
> allow Platform to call JDT (LTK) without creating a circular dependency. I
> forget the name at the moment, but this class would probably have to be
> modified to take in the "showDialog" parameter in its delete method.


The class I was reffering to is LTKLauncher, see https://git.eclipse.org/r/c/platform/eclipse.platform.ui/+/145320

It needs to be modified to receive a boolean in the delete method, which will be sent in the command object to org.eclipse.ltk.internal.ui.refactoring.actions.DeleteResourcesHandler which will then decide to perform the delete silently with the code I wrote previously (or show the dialog).

This is at least, how I would imagine this should be implemented. I could be wrong.
Comment 19 Alexander Kurtakov CLA 2021-01-07 03:07:02 EST
Mass move 4.19 M1 bugs to M3
Comment 20 Alexander Kurtakov CLA 2021-02-15 03:32:48 EST
Nothing happen for multiple milestones. Removing target milestone.