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.
F2 on Node invokes inplace rename action. Unfortunately there is no way how to override this behavior. And, unfortunately, we are required to override F2 behavior in our ui spec http://ui.netbeans.org/docs/hi/javamdr/refactoring_design.html , section UI specification | Rename: Possible ways of invocation: ... * By pressing F2 key The requirement is, that F2 should not invoke inplace rename, but refactoring rename. AFAIK it is not possible to do it with current APIs. Can you provide such API, please?
I don't think I can provide such API for 4.1, so setting TM to future. If you need to fix it in the mean time, it may be possible to intercept projects view's JTree and modify its action map, replacing "startEditing" mapping with your own action.
We appreciate some API, which allows to plugin into setName() method. Adding Jarda to CC.
I guess you need to intercept renames only on DataFolder nodes. If so, let's add interface like HandleFolderRename { public void rename(String, DataFolder) } into org.openide.loader, find it in lookup and call it from FolderNode.setName. I guess that would solve your problems.
Yes, I do exactly the same thing in java module (JavaNode). I introduced a new interface org.netbeans.modules.java.RenameHandler. There will probably arise a new requirement to override F2 behavior also for e.g. JavaBeans node in the future. Introduction HandleFolderRename is sufficient solution for now, but I would appreciate more general interface HandleRename, which would be applicable for nodes in general...
I introduced 3 similar interfaces RenameHandler in java module, FolderRenameHandler in openide/loaders, and PackageRenameHandler in java/project module. Jardo and Tomasi, please take a look at attached diff, and if you don't have objections, let's switch this issue to api_review mode.
Created attachment 21904 [details] Proposed patch
Several comments: 1) Why the FolderRenameHandler interface takes a DataObject? Shouldn't a FileObject be enough? And why it is in openide.loaders package? 2) PackageViewChildren.getRenameHandlers() should listen on the Lookup.Result (some modules can be turned on/off) 3) I don't understand to the semantics of the FolderRenameHandler. Does it the actual renaming? If so, is it correct to allow multiple FolderRenameHandlers?
> 1) Why the FolderRenameHandler interface takes a DataObject? Shouldn't a > FileObject be enough? And why it is in openide.loaders package? This question is for Jarda. He suggested it. But it looks natural to me, that FolderRenameHandler is in the same folder, where DataFolder and it's node resides. > 2) PackageViewChildren.getRenameHandlers() should listen on the Lookup.Result > (some modules can be turned on/off) Yes it should. The same applies for other Handlers. 3) I don't understand to the semantics of the FolderRenameHandler. Does it the actual renaming? If so, is it correct to allow multiple FolderRenameHandlers? Yes it does renaming (java and package rename handlers as well). Currently we don't have requirement for plugging into setName() besides the one from refactoring. If you both prefer to have only one Handler, I'll change it.
I vote for having single FolderRenameHandler in the Lookup. In case when there are more FolderRenameHandlers it is unclear what is the result of renaming. To the point #1. If you are sure that it will not be needed after datasystems removal then OK.
Created attachment 21956 [details] Modified patch including above mentioned suggestions + tests
Get rid of all renameHandlersCache vars; they make the code more complicated and they are very likely useless. How often is setName called on a node? Rarely. If and when it is, ask Lookup for the handler(s). And do not use assertions to check that there is <= 1 instance; you do not control the code that might be adding additional instances, so you cannot guarantee that this will really be true. Report such an error properly using ErrorManager.
Created attachment 21961 [details] hopefully final patch
Suggest a more helpful warning message (with no stack trace, since it is not useful in this context): ErrorManager.getDefault().notify(ErrorManager.WARNING, "Multiple instances of FolderRenameHandler found in Lookup; only using first one: " + handlers); In the test, public static junit.framework.Test suite () { junit.framework.TestSuite suite = new junit.framework.TestSuite(FolderRenameHandlerTest.class); return suite; } is useless and can just be deleted. setUp() and tearDown() should always call the super method in their body, as a matter of style. Use System.setProperty ("org.openide.util.Lookup", Lkp.class.getName()); for type safety and ease of Find Usages etc. Lkp could more cleanly be written as public static class Lkp extends ProxyLookup { public Lkp() { super(new Lookup[0]); } public void register(Object[] instances) { setLookups(new Lookup[] {Lookups.fixed(instances)}); } } then lookupAdded can be removed and the test methods can be written more clearly (and not depend on order of their execution!). If necessary, you can also include Lookups.metaInfServices(Lkp.class.getClassLoader()) among the list of lookups to proxy to.
Tests were copy/pasted from other org.openide.loaders tests. Please remove redundant suite() methods, call super in setUp() and tearDown() methods, and use Lkp.class.getName() instead of hardcoded Strings in openide tests. Otherwise people like me will use the same invalid patterns. Thanks.
Jesse, there is no method ErrorManager.getDefault().notify(int, String); Did you mean log(int, String)? Thanks
Yes I meant log(int,String), sorry.
> Use > System.setProperty ("org.openide.util.Lookup", Lkp.class.getName()); I have a feeling that I've seen this fail as the Lkp.class may trigger resolution of Lookup which can called Lookup.getDefault(). that is why I use full string name, it is safer. > ProxyLookup vs. AbstractLookup I like AbstractLookup more, but I do not think any such preference is important for the tested case.
Implemented
fixed.
Verified.