This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.

Bug 76722 - Safe delete should be integrated with regular delete
Summary: Safe delete should be integrated with regular delete
Status: RESOLVED FIXED
Alias: None
Product: editor
Classification: Unclassified
Component: Refactoring (show other bugs)
Version: 6.x
Hardware: All All
: P2 blocker (vote)
Assignee: Jan Becicka
URL:
Keywords: API
: 91871 (view as bug list)
Depends on:
Blocks: 54811 76635 77310 83220
  Show dependency tree
 
Reported: 2006-05-23 09:43 UTC by jrojcek
Modified: 2007-06-15 16:29 UTC (History)
7 users (show)

See Also:
Issue Type: ENHANCEMENT
Exception Reporter:


Attachments
Proposed solution (5.50 KB, application/octet-stream)
2007-01-17 18:51 UTC, Jan Becicka
Details
Proposed API change (4.52 KB, text/plain)
2007-06-07 16:11 UTC, Petr Nejedly
Details

Note You need to log in before you can comment on or make changes to this bug.
Description jrojcek 2006-05-23 09:43:15 UTC
Currently, the regular delete action is not connected with the safe delete action. We integrated the 
regular rename, copy/move/paste, DnD with refactoring, but not delete.

I think the regular delete should be better integrated with refactoring (safe delete) and offer the user to 
preview usages of deleted class. Something like:

[x] Preview class usages before delete
(probably unchecked by default)

BTW, we have the Find Usages menu item in Edit menu. I think the Safe Delete also belongs to the Edit 
menu and maybe right below the Delete action, and similarly in contextual menu. If we integrate the 
safe delete action with regular delete, I think we can just get rid of the safe delete menu item 
altogether.
Comment 1 Jan Becicka 2007-01-05 12:00:23 UTC
*** Issue 91871 has been marked as a duplicate of this issue. ***
Comment 2 Petr Jiricka 2007-01-05 12:58:38 UTC
Thanks for pointing me to this issue, good to see that it is already considered.
Although due to the inconsistency with Rename, I tend to think this is a
usability bug, rather than an enhancement request.
Comment 3 Jan Becicka 2007-01-16 14:34:30 UTC
Jardo, Milosi, please propose some solution. It looks like there is now easy way
how to implement it with current explorer API..
Thanks
Comment 4 Jaroslav Tulach 2007-01-17 11:08:18 UTC
I guess we should improve the standard DeleteAction to support custom delete 
dialogs. My simplified proposal is to:

1. create org.openide.cookies.ExtendedDelete interface (don't implement cookie 
anymore!)
2. modify ExplorerUtils.actionDelete to search for that interface and handle 
it especially

The usecase is that nodes representing the Java sources will have the same(!) 
instance of ExtendedDelete in their lookup. The delete action finds that out 
and asks the instance to handle delete of the nodes instead of calling 
Node.destroy itself.

This solution may or may not support selection of multiple types of nodes 
(java+c+fortran+text+etc) - this depends on what will be put into the 
ExtendedDelete interface. As far as I know, Jan prefers to not solve this 
multiple type selection case and just fallback into our current 
implementation. Imho, this is ok, we can create ExtendedDelete2 if we need 
richer semantics later.
Comment 5 Jan Becicka 2007-01-17 15:31:38 UTC
I tried to implement it, but I fail.
Problem is, that ExplorerUtils.actionDelete is static context free method, which
returns singleton instance of DeleteAction. And I don't have any node, from
which I can get cookie.
I try to put ExtendedDeleteAction interface into org.openide.util.actions and
then put implementation into global Lookup.
Comment 6 Jan Becicka 2007-01-17 18:51:55 UTC
Created attachment 37463 [details]
Proposed solution
Comment 7 Jan Becicka 2007-01-17 18:54:52 UTC
I implemented prototype. I introduced new interface ExtendedDelete, which is
registered into meta-inf lookup.
Comment 8 Jaroslav Tulach 2007-01-18 08:48:05 UTC
Re. "context less method" - the method takes ExplorerManager as first 
argument, so just call getSelectedNodes() and you have the right nodes that 
you can query for the interface.

I would prefer to get the interface from node.getLookup() than your current 
patch with META-INF/services.
Comment 9 Jan Becicka 2007-01-18 09:04:55 UTC
Re. Re. "context less method" - the method takes ExplorerManager as first 
argument, so just call getSelectedNodes() and you have the right nodes that 
you can query for the interface.

OK. Someone want instance of DeleteAction - no nodes selected - he gets default
impl, because there is no nodes selected. And then the selection change - but
API user already have DeleteAction for his manager.
---

Another problem is, that for instance Java Refactoring module cannot add cookie
into JavaNode. Or am I wrong?
Comment 10 Jan Becicka 2007-01-26 09:31:55 UTC
Anyone working on it?
Comment 11 Petr Nejedly 2007-01-26 10:05:18 UTC
No, at least I'm not working on it.
So, I've got an enhancement, tagged with API_REVIEW_FAST, and with attached
solution with some disagreements. What do you expect me to do with it?
I also don't have any time allocated for this yet.

  
Comment 12 Jan Becicka 2007-01-26 11:02:05 UTC
I expect, that developer in "assigned to" field will evaluate this issue.

And yes, there are some objections...but the attached solution is the best we
*have*. If you have better I will appreciate it.
Thanks.
Comment 13 Petr Jiricka 2007-01-26 16:55:29 UTC
I would also like to see this done for NetBeans 6.0. Honza proposed a
satisfactory solution and there have not been any counterproposals, so I think
we should go with Honza's solution.
Comment 14 Jan Becicka 2007-02-13 10:06:52 UTC
Can you evaluate this issue (set target milestone)? Thanks.
Comment 15 Jaroslav Tulach 2007-02-13 12:49:28 UTC
If you keep the patch provided by jbecicka, then please note that his "cookie" 
is not in fact a "cookie", but more a "query". A query that extends behaviour 
of ExplorerUtils. As such I guess it should be in openide/explorer...
Comment 16 Jan Becicka 2007-04-18 20:55:14 UTC
Is this patch still planned for M9?
Comment 17 Petr Nejedly 2007-04-23 10:36:59 UTC
Sorry, but no.
I'll pursue it harder for M10.
Comment 18 Petr Nejedly 2007-06-07 14:17:00 UTC
jbecicka, I still don't understand your concerns re. context:
> OK. Someone want instance of DeleteAction - no nodes selected - he gets default
> impl, because there is no nodes selected. And then the selection change - but
> API user already have DeleteAction for his manager.

ExplorerUtils.deleteAction() so not something "someone" may "want" - it is to be
used only by given explorer instance and is always sensitive to the selection in
given explorer instance. If someone asks of an instance and calls
actionPerformed() on it, it will try to delete currently selected nodes in given
explorer instance (resolving current context-selection on the fly).

I'll try to provide an implementation that would serve your use case: whenever
somebody press [Del] in explorer, ExtendedDelete will be looked up in selected
nodes lookups and if the nodes agree on an instance, it will get called before
usual delete (fallbacking to usual delete if there is no common ExtendedDelete
or if it returns false.)
Comment 19 Petr Nejedly 2007-06-07 14:23:42 UTC
Oh, you can't still push something into java node's Lookup, right? Then the
globally registered query interface is the only way to go, it seems.
Comment 20 Jan Becicka 2007-06-07 15:03:35 UTC
> Oh, you can't still push something into java node's Lookup, right? 
right
Comment 21 Jaroslav Tulach 2007-06-07 15:25:57 UTC
It is not the only way: There can be API between Java and Refactoring that 
will place additional instance of some interface into Java nodes...
Comment 22 Jan Becicka 2007-06-07 15:37:41 UTC
We are not talking about Java Refactoring, but about general Refactoring
Framework, suitable for java, xml, jsp, js, cpp, etc..
Comment 23 Petr Nejedly 2007-06-07 16:11:53 UTC
Created attachment 43394 [details]
Proposed API change
Comment 24 Petr Nejedly 2007-06-07 16:13:58 UTC
Reviewers,
please review the proposed change.
It is similar to jbecicka's original proposal, but moves the handler interface
where it belongs, to the oo.explorer package.
Comment 25 Jesse Glick 2007-06-07 17:26:52 UTC
Probably the delete method should throw IOException. Otherwise looks OK to me.
Comment 26 Petr Nejedly 2007-06-12 09:20:28 UTC
And the IOException would mean:
"I'm the instance of ExtendedDelete that should have handled this, but while I
was trying to delete the nodes, I encountered this problem. No further attempts
to delete the selection (using subsequent instances or default implementation)
should be made."

So let's summarize the use cases:
o) User invokes delete on a java* node, safe delete pops up the dialog and say
   it's unsafe to delete the nodes. User cancels.
   -> The handler doesn't delete but returns true, no further processing takes
      place.

o) User invokes delete on a java* node, safe delete pops up the dialog and say
   the status. User confirms delete.
   -> The handler tries to delete and either return true, or fails with IOE
      (e.g. readonly FS). In either case, no further processing takes place.

o) User invokes delete on other* node.
   -> All registered handlers return false (quickly, please!), default
      implementation asks for confirmation (if enabled) and deletes the nodes. 

*: Generally, any node that refactorings (or other API users) cares/don't care
about.
Comment 27 Petr Nejedly 2007-06-15 08:55:40 UTC
jbecicka, the openide/explorer API part is in:
openide/explorer/apichanges.xml,v1.13
openide/explorer/manifest.mf,v1.12
openide/explorer/src/org/openide/explorer/ExplorerActionsImpl.java,v1.11
openide/explorer/src/org/openide/explorer/ExtendedDelete.java,v1.1

I'm sorry for doing the change this late in the release cycle, but the API is very close to your original proposal, so
it should be easy for you to adapt your safe delete integration with the final version.
Comment 28 Jan Becicka 2007-06-15 16:29:52 UTC
Thanks for openide part.
Here is refactoring impl.:
Checking in refactoring/java/src/org/netbeans/modules/refactoring/java/ui/SafeDeletePanel.form;
/cvs/refactoring/java/src/org/netbeans/modules/refactoring/java/ui/SafeDeletePanel.form,v  <--  SafeDeletePanel.form
new revision: 1.3; previous revision: 1.2
done
Checking in refactoring/java/src/org/netbeans/modules/refactoring/java/ui/SafeDeleteUI.java;
/cvs/refactoring/java/src/org/netbeans/modules/refactoring/java/ui/SafeDeleteUI.java,v  <--  SafeDeleteUI.java
new revision: 1.7; previous revision: 1.6
done
Checking in refactoring/java/src/org/netbeans/modules/refactoring/java/ui/SafeDeletePanel.java;
/cvs/refactoring/java/src/org/netbeans/modules/refactoring/java/ui/SafeDeletePanel.java,v  <--  SafeDeletePanel.java
new revision: 1.6; previous revision: 1.5
done
Checking in refactoring/java/src/org/netbeans/modules/refactoring/java/ui/RefactoringActionsProvider.java;
/cvs/refactoring/java/src/org/netbeans/modules/refactoring/java/ui/RefactoringActionsProvider.java,v  <-- 
RefactoringActionsProvider.java
new revision: 1.36; previous revision: 1.35
done
Checking in refactoring/java/src/org/netbeans/modules/refactoring/java/ui/Bundle.properties;
/cvs/refactoring/java/src/org/netbeans/modules/refactoring/java/ui/Bundle.properties,v  <--  Bundle.properties
new revision: 1.12; previous revision: 1.11
done
RCS file: /cvs/refactoring/api/src/META-INF/services/org.openide.explorer.ExtendedDelete,v
done
Checking in refactoring/api/src/META-INF/services/org.openide.explorer.ExtendedDelete;
/cvs/refactoring/api/src/META-INF/services/org.openide.explorer.ExtendedDelete,v  <--  org.openide.explorer.ExtendedDelete
initial revision: 1.1
done
Checking in refactoring/api/nbproject/project.xml;
/cvs/refactoring/api/nbproject/project.xml,v  <--  project.xml
new revision: 1.11; previous revision: 1.10
done
Checking in refactoring/api/src/org/netbeans/modules/refactoring/spi/impl/SafeDeleteAction.java;
/cvs/refactoring/api/src/org/netbeans/modules/refactoring/spi/impl/SafeDeleteAction.java,v  <--  SafeDeleteAction.java
new revision: 1.6; previous revision: 1.5
done
Checking in ide/test/qa-functional/src/validation/IDEValidation.java;
/cvs/ide/test/qa-functional/src/validation/IDEValidation.java,v  <--  IDEValidation.java
new revision: 1.198; previous revision: 1.197
done