Community
Participate
Working Groups
Eclipse Editor preferences has 2 levels: * global preferences * project preferences But it's not possible to update preferences for just a given editor instance and it's hard for instance to support editorconfig. See my comments at https://bugs.eclipse.org/bugs/show_bug.cgi?id=457046#c5 It should be cool if org.eclipse.ui.texteditor.AbstractTextEditor could host this code: --------------------------------- AbstractTextEditor#handlePreferenceStoreChanged(PropertyChangeEvent) */ protected void handlePreferenceStoreChanged(PropertyChangeEvent event) { ... } @Override public Object getAdapter(@SuppressWarnings("rawtypes") Class key) { if (key.equals(IEclipsePreferences.class)) { if (editorPreferences == null) { editorPreferences = new EclipsePreferences(); editorPreferences.addPreferenceChangeListener( new org.eclipse.core.runtime.preferences.IEclipsePreferences.IPreferenceChangeListener() { @Override public void preferenceChange(PreferenceChangeEvent event) { handlePreferenceStoreChanged(new PropertyChangeEvent(event.getSource(), event.getKey(), event.getOldValue(), event.getNewValue())); } }); } } return editorPreferences; } return super.getAdapter(key); } And after an external action could update the editor preferences like this: --------------------------------- IEditorPart editor = ... IEclipsePreferences p = (IEclipsePreferences) editor.getAdapter(IEclipsePreferences.class); if (p != null) { p.put("spacesForTabs", "true") } --------------------------------- Hope you will like my idea.
This seems to be a blocker for bug 457046, and if we want to easily allow external sources (editor config or other) to contribute to improve behaviour of the text editor, it seems worthy to do it. I can see 2 ways of doing it: 1. Add an API in the AbstractTextEditor 2. Add an adapter in the AbstractTextEditor Both would actually be interesting. 1. Is clean and will help developers to use this feature, but consuming it would introduce a dependency to the very latest version 2. Is backward compatible so it can be consumed even by code targeting older versions of platform.text. However, it's not not easy to discover and drives to less clean code than 1. It seems to me that the adapter could be easily considered for Neon.1. About the API, I'm not sure. Are API additions allowed in maintenance branch?
(In reply to Mickael Istria from comment #1) > This seems to be a blocker for bug 457046, and if we want to easily allow > external sources (editor config or other) to contribute to improve behaviour > of the text editor, it seems worthy to do it. > > I can see 2 ways of doing it: > 1. Add an API in the AbstractTextEditor > 2. Add an adapter in the AbstractTextEditor > Both would actually be interesting. > 1. Is clean and will help developers to use this feature, but consuming it > would introduce a dependency to the very latest version > 2. Is backward compatible so it can be consumed even by code targeting older > versions of platform.text. However, it's not not easy to discover and drives > to less clean code than 1. Maybe an additional preference scope could be added. > > It seems to me that the adapter could be easily considered for Neon.1. > About the API, I'm not sure. Are API additions allowed in maintenance branch? It needs PMC approval. Also new features need PMC approval. At any rate, a target should only be set if someone takes the bug.
(In reply to Dani Megert from comment #2) > Maybe an additional preference scope could be added. Sounds good. However, it seems a bit harder to implement than simply exposing editor preferences via adapter/api. What would be the benefits of a scope? > At any rate, a target should only be set if someone takes the bug. I set the target at least to get this idea considered before 4.6.1, it wasn't a commitment to implement that in 4.6.1, So it's fine to remove the target version until it gets planned. I can take the bug if: * there are good chances to get PMC approval * the implementation remains simple enough (that's what I like with the adapter) Should I request PMC approval right now? Sending a request to eclipse-pmc@ is enough?
(In reply to Mickael Istria from comment #3) > Should I request PMC approval right now? Sending a request to eclipse-pmc@ > is enough? We have to see the API, and as always, there needs to be a client that consumes and validates the API. Otherwise we can't approve it.
@Angelo: would it work using the IPreferenceStore rather than EclipsePreferences? (In reply to Dani Megert from comment #4) > We have to see the API I guess it would be something like public IPreferenceStore getEditorPreferenceStore() > and as always, there needs to be a client that consumes and validates the API. Otherwise we can't approve it. Does the client need to be in Eclipse.org? Or would a project such as https://github.com/ncjones/editorconfig-eclipse work? And what about the adapter, as it's not API change, it could be considered as internal non-API thing for 4.6.1 so it's less "risky" ?
(In reply to Mickael Istria from comment #5) > I guess it would be something like > public IPreferenceStore getEditorPreferenceStore() By the way, this method already exists as protected, so it's just about making it public.
(In reply to Mickael Istria from comment #6) > (In reply to Mickael Istria from comment #5) > > I guess it would be something like > > public IPreferenceStore getEditorPreferenceStore() > > By the way, this method already exists as protected, so it's just about > making it public. You mean #getPreferenceStore ? This is not the store for the editor instance. Also, textual editors currently don't have project specific settings (except for the Java editor's Save Actions). Any solution would have to be implemented in a way, that we keep the freedom to add project specific settings for textual editors in the future.
> > public IPreferenceStore getEditorPreferenceStore() By the way, this method already exists as protected, so it's just about making it public. The method which exists is protected IPreferenceStore getPreferenceStore() But you cannot use it, because the most editor returns an instance of http://help.eclipse.org/juno/index.jsp?topic=%2Forg.eclipse.platform.doc.isv%2Freference%2Fapi%2Forg%2Feclipse%2Fui%2Ftexteditor%2FChainedPreferenceStore.html which is read-only If you will try to update the preference store (with putValue, you will have this error like: ------------------------------------------------ java.lang.UnsupportedOperationException at org.eclipse.ui.texteditor.ChainedPreferenceStore.putValue(ChainedPreferenceStore.java:268) ------------------------------------------------ Why do you want not to use IEclipsePreferences? I understand that it's hard to discover the new API which will return IEclipsePreferences/IPreferenceStore of the editor, but please add too getAdapter, because if a plugin (like editorconfig) which want to support several version of Eclipse should do awful code with reflection to know if IEclipsePreferences/IPreferenceStore getter exists or not. The code: --------------------------------- IEditorPart editor = ... IEclipsePreferences p = (IEclipsePreferences) editor.getAdapter(IEclipsePreferences.class); if (p != null) { p.put("spacesForTabs", "true") } --------------------------------- compile with any Eclipse version.
Actually, we don't need a whole EclipsePrefrences object. What we need is an IPropertyChangeListener that would dispatch your invocation of listener.handlePropertyChanged() to AbstractTextEditor#handlePreferenceStoreChanged(...). I'll try a few hacks in the editorconfig plugin and keep in touch either on this bug, or on https://github.com/ncjones/editorconfig-eclipse/issues/30 . I have an idea of a way to change those properties and call handlePreferenceStoreChanged(...) without changing the AbstractTextEditor, but I need to try it. Also, another approach would be to make the AbstractTextEditor#handlePreferenceStoreChanged(...) public and to have editor config plugin invoking it directly. Reflection woudn't be too bad as a last solution...
@Angelo: I tried to get your propsal to work but didn't manage to achieve it. I have a few questions to know what's to blame: me or your proposal. 1. In your initial proposal, I see an editorPreferences object. How do you retrieve it? Is it a field you added to the AbstractTextEditor? 2. In both AbstractTextEditor and AbstractDecoratedTextEditor handlePreferenceStoreChanged methods, the value of the property event is almost never read directly, instead it's always retrieved from the (global or project) preferenceStore. So I don't get how just calling this method can change the behavior of the editor if it doesn't modify the global preference store. Because of item 2, it seems to me that the code you've proposed here would work. Is there anything missing?
> 1. In your initial proposal, I see an editorPreferences object. How do you retrieve it? Is it a field you added to the AbstractTextEditor? Yes it is that. > 2. In both AbstractTextEditor and AbstractDecoratedTextEditor handlePreferenceStoreChanged methods, the value of the property event is almost never read directly, instead it's always retrieved from the (global or project) preferenceStore. Yes you have right -( When I do test with my TypeScript editor, I used the value coming from the PropertyChangeEvent. Is there a reason that this property value is not used directly? When 2) will be fixed (perhaps we must add an instance of IPreferenceStore for the editor?), I think we could have a new method like ------------------------------------------------------------- AbstractTextEditor#setPreference(String name, Object value) ------------------------------------------------------------- which call handlePreferenceStoreChanged(PropertyChangeEvent event)
Created attachment 262147 [details] Plugins which uses FileScope to update editor instance preferences @Mickael, I have continued to study more how to support with a clean mean editor instance preferences with JSDT JavaEditor editor and my conclusion is that it depdends how to the editor overrides handlePreferenceStoreChanged: 1) if handlePreferenceStoreChanged uses property or value of the event, it's OK (ex: in JavaEditor => if (PreferenceConstants.EDITOR_MARK_OCCURRENCES.equals(property)) {....). 2) if handlePreferenceStoreChanged read the preferences store by ignoring the event value, it's not possible today but I have attached a demo which uses my idea with FileScope. 3) cache the preferences coming from project (ProjectScope) like JDT, JSDT does, in this case it's impossible. This case is for instance for the tabWith for JSDT/JDT editor. For the 3) JSDT uses CodeFormatterUtil#getTabWidth(IJavaScriptProject project) to retrieve tabWidth and as you can see the signature waits an IJavaScriptProject although it should wait an IFile (coming from the editor) CodeFormatterUtil#getTabWidth(IFile file). For the 2), I have found a solution which looks like ProjectScope. Indeed for me, we have: * global preferences (by using InstanceScope.INSTANCE.getNode()) * project preferences (by using new ProjectScope(project).getNode()) * file preferences which misses today. So my idea is to provide FileScope: new FileScope(file).getNode()) I have attached a zip which contains 2 projects: * editor-preferences-sample which provides a XML Editor which is able to update tabWidth only for a given editor instance (by using FileScope) * editor-action-sample which provide in the Eclipse toolbar a button that you can click it to update the tabWidth of the activated editor. You can see thoses 2 plugins in action with my attached demo too. Here some explanation: * FileScope looks like ProjectScope but for an IFile (very basic implementation). It uses "file" as scope name. * FilePreferences (in internal package) looks like ProjectPreferences. It is registered with the key "file" in the extension point ---------------------------------------------------------------- <extension id="filePreferences" point="org.eclipse.core.runtime.preferences" name="%preferencesExtPtName"> <scope name="file" class="myorg.eclipse.core.internal.resources.FilePreferences"/> </extension> ---------------------------------------------------------------- The XMLEditor add FileScope in the preferences store (by using EclipsePreferencesAdapter which is a copy/paste of JDT project): ---------------------------------------------------------------- @Override protected void doSetInput(IEditorInput input) throws CoreException { super.doSetInput(input); List<IPreferenceStore> stores = new ArrayList<>(); // Add editor file preferences store IFile file = input.getAdapter(IFile.class); stores.add(new EclipsePreferencesAdapter(new FileScope(file), XMLPlugin.PLUGIN_ID)); // add other preferences store stores.add(EditorsUI.getPreferenceStore()); super.setPreferenceStore(new ChainedPreferenceStore(stores.toArray(new IPreferenceStore[stores.size()]))); } ---------------------------------------------------------------- The handler which update the tabWidh is like this: ---------------------------------------------------------------- public class SampleHandler extends AbstractHandler { @Override public Object execute(ExecutionEvent event) throws ExecutionException { IWorkbenchWindow window = HandlerUtil.getActiveWorkbenchWindowChecked(event); IEditorPart editor = window.getActivePage().getActiveEditor(); IFile file = editor.getEditorInput().getAdapter(IFile.class); InputDialog dlg = new InputDialog(window.getShell(), "", "Enter a tab width", "2", null); if (dlg.open() == Window.OK) { new FileScope(file).getNode(XMLPlugin.PLUGIN_ID) .put(AbstractDecoratedTextEditorPreferenceConstants.EDITOR_TAB_WIDTH, dlg.getValue()); } return null; } } ---------------------------------------------------------------- In other words, to update tabWidth of an editor instance, you need just an IFile instance: ---------------------------------------------------------------- IFile file = // here retrieve an IFile instance // Update tabWidth of the editor which opens IFile with "2" new FileScope(file).getNode(XMLPlugin.PLUGIN_ID).put(AbstractDecoratedTextEditorPreferenceConstants.EDITOR_TAB_WIDTH, "2"); ---------------------------------------------------------------- And that's all! It works, because AbstractDecoratedTextEditor read the store (and don't cache the tabWidh in an another cache like JDT/JSDT does because tabWidh depends on other properties) ---------------------------------------------------------------- if (AbstractDecoratedTextEditorPreferenceConstants.EDITOR_TAB_WIDTH.equals(property)) { IPreferenceStore store= getPreferenceStore(); if (store != null) sourceViewer.getTextWidget().setTabs(store.getInt(AbstractDecoratedTextEditorPreferenceConstants.EDITOR_TAB_WIDTH)); ---------------------------------------------------------------- Hope you will like my idea. > We have to see the API, and as always, there needs to be a client that consumes and validates the API. Otherwise we can't approve it. @Dani hope my 2 plugins will help you to validate the API.
Created attachment 262148 [details] A demo with dialog which updates tabWidh for a given editor instance (file) And here the demo that you can see.
@Angelo: do you think you could turn your API proposal as a Gerrit patch? It's not easy to review proposals without a good review tool. Introducing a FileScope sounds good, it's what Dani suggested on comment #2, and I also believe it's the right way to go.
> @Angelo: do you think you could turn your API proposal as a Gerrit patch? There is none API proposal. The only thhing that I have added is FileScope+FilePreferences which should be hosted in the "org.eclipse.core.resources" plugin and that's all. I don't think we could add the FileScope with a generic mean and with lazy mean: ---------------------------------------- stores.add(new EclipsePreferencesAdapter(new FileScope(file), XMLPlugin.PLUGIN_ID)); ---------------------------------------- As you can see, in my case I use ChainedPreferenceStore + XMLPlugin.PLUGIN_ID which is specific.
(In reply to Angelo ZERR from comment #15) > The only thhing that I have added is > FileScope+FilePreferences which should be hosted in the > "org.eclipse.core.resources" plugin and that's all. This is an API proposal ;) Anyway, it's quite tricky to look inside external bundles and easily compare with current state and find out everything that's proposed to change. It's really the purpose of Gerrit and review tools, it would *really* make it easier for us to understand and comment suggested changes if you propose those as Gerrit patches. If you need any assistance with using Gerrit for such proposals, feel free to ping me on IRC.
@Mickael do you think is there any chance to support .editorconfig with GenericEditor?
(In reply to Angelo ZERR from comment #17) > @Mickael do you think is there any chance to support .editorconfig with > GenericEditor? The Generic Editor won't include a dependency to editorconfig. However, it would be interesting to plug into it a mechanism for specifying formatting. There is no such extension to specify formatting in he generic editor at the moment. Please open an enhancement request for that.
> The Generic Editor won't include a dependency to editorconfig. Sorry I meant, provide an extension kind or preferences to give the capability to set a preference for a given instance of the editor (indent, encoding, etc).
Moving to Platform/UI as such Editor/File scope isn't specific to text. By the way, wouldn't make sense to actually attach preferences to individual files rather than editor instances? Especially in the case of editorconfig?
> By the way, wouldn't make sense to actually attach preferences to individual files rather than editor instances? Especially in the case of editorconfig? Today the only preferences that I know which is attached to a file is "encoding". It works well because IFile has encoding, but what about other properties like "indent" ? Each editor manages their "indent" strategies according the languages.
I just stumbled over this bug since I was wondering if editorconfig support could be added. I like the idea of FileScope/FilePreferences, I think it is worth a feature request. I'll open one.
(In reply to Karsten Thoms from comment #22) > I just stumbled over this bug since I was wondering if editorconfig support > could be added. I like the idea of FileScope/FilePreferences, I think it is > worth a feature request. I'll open one. Just thinking here whether there could be an `EditorConfigScope` that for a given file would just resolve the related editorconfig model and let the preference store for TextEditor use this editorconfig model.
