Bugzilla – Full Text Bug Listing
| Summary: | [xtend][ui][organize imports] accelerator bound in xtext editor scope | ||||||
|---|---|---|---|---|---|---|---|
| Product: | [Modeling] TMF | Reporter: | Knut Wannheden <knut.wannheden> | ||||
| Component: | Xtext | Assignee: | Project Inbox <tmf.xtext-inbox> | ||||
| Status: | CLOSED FIXED | QA Contact: | |||||
| Severity: | normal | ||||||
| Priority: | P3 | CC: | sebastian.zarnekow | ||||
| Version: | 2.1.0 | Flags: | sebastian.zarnekow:
juno+
|
||||
| Target Milestone: | M4 | ||||||
| Hardware: | PC | ||||||
| OS: | Mac OS X - Carbon (unsup.) | ||||||
| Whiteboard: | |||||||
| Attachments: |
|
||||||
Created attachment 206681 [details]
partial patch
The attached patch demonstrates the proposed solution except for the proposed changes to the generator for the plugin.xml and for the binding.
Note that the reason the code looks a bit funny with regards to the injection is that the method initializeKeyBindingScopes() is already called by the constructor AbstractDecoratedTextEditor() at which point Guice will not yet have injected the fields.
Thanks for the patch. It looks good to me (besides that the since tag should be updated to 2.2). Could you apply this one on master? I'm not sure about the proposed enhancement to the generator, though. OK. I will update the @since tags and apply it to the master. What about the maintenance branch? (In reply to comment #3) > OK. I will update the @since tags and apply it to the master. What about the > maintenance branch? New binding keys and the like should not be introduced in 2.1.1. If you think it's necessary to fix it for 2.1.1 we'd have to think about another impl. (In reply to comment #4) > New binding keys and the like should not be introduced in 2.1.1. > If you think it's necessary to fix it for 2.1.1 we'd have to think about > another impl. Not sure I follow you. The proposed solution does not change any of the key bindings. It just makes sure the bindings added in org.eclipse.xtext.xtend2.ui are only available to Xtend2 editors. (In reply to comment #5) > (In reply to comment #4) > > New binding keys and the like should not be introduced in 2.1.1. > > If you think it's necessary to fix it for 2.1.1 we'd have to think about > > another impl. > > Not sure I follow you. The proposed solution does not change any of the key > bindings. It just makes sure the bindings added in org.eclipse.xtext.xtend2.ui > are only available to Xtend2 editors. Sorry for the confusion. I was talking about the guice binding keys which were newly introduced. They should not appear in the 2.1.1 branch. (In reply to comment #6) > Sorry for the confusion. I was talking about the guice binding keys which were > newly introduced. They should not appear in the 2.1.1 branch. I see. Makes sense. My mistake. I found a simple solution which is compatible with the proposed solution: If I introduce a custom context for my own editor (which would be the recommended solution starting with Xtext 2.2) and register the binding in that context then it all works. Even though the custom context has the standard XtextEditorScope as its parent for which Xtend registers a command with an identical key binding. So in summary this means that we don't have to do anything for 2.1.1. So I will go ahead and push the master patch. Fix pushed to master. Closing all bugs that were set to RESOLVED before Neon.0 Closing all bugs that were set to RESOLVED before Neon.0 |
The key binding for the command org.eclipse.xtext.xbase.ui.organizeImports is bound to Ctrl-Shift-O in the plug-in org.eclipse.xtext.xtend2.ui as follows: <key contextId="org.eclipse.xtext.ui.XtextEditorScope" schemeId="org.eclipse.ui.defaultAcceleratorConfiguration" sequence="M1+M2+O" commandId="org.eclipse.xtext.xbase.ui.organizeImports"> </key> As it is bound in the context org.eclipse.xtext.ui.XtextEditorScope any other Xtext language (not based on Xtend2) binding a command like "Organize Imports" to Ctrl-Shift-O in the same way (i.e. using the same context) will result in a conflict. It would make sense if Xtend2 would introduce its own context and bind its accelerators in that context. In addition to registering the context using the org.eclipse.ui.contexts extension point it is also necessary to change XtextEditor#initializeKeyBindingScopes() to use that context: @Override protected void initializeKeyBindingScopes() { setKeyBindingScopes(new String[] { "org.eclipse.xtext.ui.XtextEditorScope" }); //$NON-NLS-1$ } To make sure this doesn't get out of sync (and it may be a good idea anyways) we could by default generate a language specific context extension into the UI plug-in's plugin.xml as well as generate a named binding which would be used by XtextEditor#initializeKeyBindingScopes().