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.
Editor actions are currently hard-coded in EditorKit implementations. It would be beneficial to let modules register editor actions declaratively in a module layer. MimeLookup seems to be a good registration facility for doing that.
Here is the proposed solution - http://editor.netbeans.org/doc/editor-actions.html
I'd like to have this change reviewed, please. Thank you very much indeed.
Y01 What I'd like to do as part of issue 70280 is to make sure all actions are registered in the "global action pool" - e.g. config/Actions, and all the instances in Menu, Toolbar, Shortcuts just refer to them using .shadow. Maybe you should do the same in the case of editor actions. Y02 Shall not one be allowed to associate a key to each action? If so, shall not the syntax be same as in case of Shortcuts folder? If we had one infrastructure and style for registering keystrokes, we might simplify the options dialog that handles keystrokes associations with actions. Y03 If the registration of actions is supposed to be similar to the registration of actions in the IDE, I would expect them to often implement ContextAwareAction interface. In such case the BaseKit should very likely "clone" the actions with proper context. What will be in the context?
I'm wondering whether we should declare ALL the actions through the layer. There is about 105 editor actions (IIRC it corresponds to number of all other actions in the IDE from all the modules) and many of them (e.g. cursor moves etc.) will probably never be overriden. ad Y01) I agree. ad Y02) IMHO we need to clarify this in the proposal. Right now the keybindings are specified int an .xml file e.g. /editor/src/org/netbeans/modules/editor/resources/NetBeans-keybindings.xml and they use bind to the Action.NAME which relates to the important fact that every editor action uses Action.NAME property for its unique identification instead of localized name. It is also used as a value in the InputMap and thus also as a key in the ActionMap of the editor pane. IMHO prior to making any changes to this we should first find out whether Swing does not rely on this in its impl. And it seems that it does see e.g. DefaultCaret - it uses Action a = null; ActionMap map = getComponent().getActionMap(); if (map != null) { a = map.get(DefaultEditorKit.selectWordAction); } BTW IMHO the "Shortcuts" should not generally be used due to shortcut profiles existence - I have found "Keymaps" being mentioned in arch-core.xml but I did not find any real use of this. Personally I find the present editor's shortcut specification optimal both from the functionality and performance point of view, we just need to allow for more of these files to be present in the xml layer and read by editor's infrastructure. ad Y03) I don't know but please note that the execution of the actions upon keystrokes is hardcoded in the JComponent and as the editor only installs its kit it cannot influence the action execution. We could possibly do it in the openide/text in QuietEditorPane but I'm not sure whether that's a good idea to do that.
*** Issue 53500 has been marked as a duplicate of this issue. ***
*** Issue 58176 has been marked as a duplicate of this issue. ***
I'd like to resurrect this discussion and resume work on this issue. The reason for this, besides of irresistible cravings for doing The Good, is that I need to find a replacement for SettingsNames.CUSTOM_ACTION_LIST, which in turn I need in order to finish work on issue #90403 (killing BaseOptions and Settings). The SettingsNames.CUSTOM_ACTION_LIST allowed modules to supply additional actions for any arbitrary kitClass (approx. mime type) of their choice. Precisely what this issue will make possible when resolved. ad #1: The proposal http://editor.netbeans.org/doc/editor-actions.html permits registration in the actions pool and the use of .shadow files. However, I'm afraid that in general this approach is not very convenient for editor actions. There are several reasons - some actions are very low-level (eg. caret navigation), others make sense only in certain file types. Therefore I think that in general it makes only a little sense to expose editor actions in the global actions pool. But as I said in cases where it makes sense it can be done. ad #2: I'm sorry, but I'm very reluctant to change the registration of editor shortcuts. The way how it is has been around for several releases, has become a public API and IMHO is more flexible and convenient than the way based on 'Shortcuts' and 'Keymaps'. On the other hand I understand your call for unification, but it's totally out of scope of this issue. ad #3: Well, usually editor actions are based on javax.swing.text.TextAction, which provides JTextComponent that the action should act on. Typically this is enough for any editor action, because from JTC the action can get pretty much everything (ie. Document, mime type, FileObject). This way TextAction instances can be shared (and are in Netbeans) between text components without a necessity to clone them. I think that the concept of ContextAwareAction could easily be supported, in which case the context Lookup passed to the action will only contain the JTextComponent instance. If this support is requested I will implement it, but again I don't see it essential for resolving this issue. Thanks
I've implemented it in the editor module. The diff I'm going to attach shows all the changes, but in fact is not so important, because no public java API was affacted. The change is fully backwards compatible, which means that existing editor kits may remain unchanged. The real benefit is for modules that supply editor actions for someone else's editor kit and had to use Settings.Initializer and SettingsNames.CUSTOM_ACTION_LIST.
Created attachment 61692 [details] Implementation of editor actions from the 'Actions' subfolder
Created attachment 61693 [details] Better diff with updated arch.xml
I'd like to ask for review. Please use the second diff, which also shows updated architecture description (arch.xml). Thanks
Might be useful for the diff to show at least one module being converted to use the declarative registrations.
This will definitely be useful for GSF; since GSF provides the EditorKit implementation it had its own API for letting other modules add in actions to its own action map. I can now rip all of that out and just have modules that need editor actions provide their own direct registration. Thanks Vita!
Created attachment 61734 [details] Updated editor.bracesmatching module
Jesse, I attached the diff of updated editor.bracesmatching module. It's a good example of a module, which is adding two actions (brackets navigation and navigation with selecting text) to all editor types. This is a typical scenario where the declarative registration will help the most.
Thanks for bracesmatching.diff. [JG01] I presume project.xml should also get <dependency> <code-name-base>org.netbeans.modules.editor</code-name-base> <run-dependency> <release-version>3</release-version> <specification-version>1.39</specification-version> </run-dependency> </dependency> to declare that it wishes to use the new system? [JG02] I think <attr name="instanceOf" stringvalue="javax.swing.Action"/> <attr name="instanceClass" stringvalue="org.netbeans.modules.editor.bracesmatching.BracesMatchAction"/> are both unnecessary and should be deleted from the layer. There is no need for InstanceCookie.Of (no one should be searching this folder other than the editor module code which is just about the load the actual instances), and no need for InstanceCookie.instanceClass(). In the probably common case that an impl class defines just one action, it should suffice to write just <f n="Editors"><f n="Actions"> <file name="my-action-Class.instance"/> </></> While these extra attributes are technically harmless, they are also useless and contribute to the general misperception that you have to specify instanceOf and instanceClass all the time. (Generally instanceClass is completely useless, and instanceOf is only needed for items in the increasingly deprecated Services/ folder.) [JG03] In arch.xml, <folder name="Editors/text/x-java/Actions"> is syntactically invalid.
JG01: Oops, correct, I forgot this. JG02: Ok, I'll remove these attributes. I usually tend to use them, because it should (at least I think) prevent creating instances when searching through Lookup. I recall adding them for editor kit registrations in MimeLookup in order to prevent excessive creation of all kits from KitsTracker that scans folders under 'Editors' in order to translate kitClass to mime-type. Anyway, in this case the attributes are really not neccessary. JG03: I see, I'll fix it. I don't know why I thought this style is possible. Thanks for your review.
vstejskal, great to hear you work on this. I have use case for #3. I'm implementing "quick search" feature, see http://www.netbeans.org/issues/show_bug.cgi?id=134398 In this feature I need to invoke editor actions while focus is in search text field and it would be best for me if editor actions were working on current context, not focus based. Or is there any other workaround I could apply? Perhaps I can show you what I'm talking about on my machine next week...
Jarda mentioned that the other day. In general, all editor actions subclass TextAction, which provides getTextComponent(ActionEvent e) method for subclasses to get the JTextComponent that they should act on. The method basically uses e.getSource() and tries to narrow it down to JTextComponent, if this fails it returns the last focused JTextComponent. I'm not sure how exactly you call the editor actions in quick-search, but supplying the right JTextComponent through the ActionEvent should help. The other approach would be to make editor actions implement ContextAwareAction, so that you could provide the JTextComponent through the Lookup context. But that is IMO equivalent to calling the actions with ActionEvent that has the correct event source set. Let's have a chat next week if you want.
Created attachment 61819 [details] Updated diff with all changes
Please see the updated diff with all the changes (both editor and editor.bracesmatching). If there are no further comments I will push the changes on Monday next week. Thanks
I think the change is ok. However, as an advice, I'd really want to see implementation of "context sensitivity" - e.g. Y03 again: Change BaseAction to support ContextAwareAction interface, make sure it get the component to work on from the context and not from last selected text component. If it is impossible to implement this advice, skip it, however if there is a way to do it, then you will simplify the life of Dafe and Jan who implement "quick access toolbar", imho.
Thanks for review. I pushed the changes to main: http://hg.netbeans.org/main/rev/1d7655c4fb56 http://hg.netbeans.org/main/rev/b618b8a1c4be As for Y03 I'll talk to Dafe what exactly he needs and how we could help him.
.
Integrated into 'main-golden', available in NB_Trunk_Production #221 build Changeset: http://hg.netbeans.org/main/rev/1d7655c4fb56 User: Vita Stejskal <vstejskal@netbeans.org> Log: #85442 (fixed): editor actions can be registered through XML layers (Editors/<mime-type>/Actions)
Integrated into 'main-golden', available in NB_Trunk_Production #234 build Changeset: http://hg.netbeans.org/main/rev/3fc686aa1748 User: Tor Norbye <tor@netbeans.org> Log: Get rid of GSF's EditorAction class now that the editor allows modules to plug into another module's editor kit (see issue #85442). Also rewrite the various actions that were using EditorAction to use the declarative editor action approach instead. (The Run action in JavaScript is removed since it doesn't usually do what people want; it does not do client-side Java; it runs it in Rhino in the IDE)
*** Issue 138558 has been marked as a duplicate of this issue. ***