Get IPreferenceStore from AbstractTextEditor with adapt mechanism

[angelozerr]

Get IPreferenceStore from AbstractTextEditor with adapt mechanism

Signed-off-by: azerr azerr@redhat.com

[angelozerr]

This simple PR provides the capabaility to get IPreferenceStore from an AbstractTextEditor with adapt mechanism:

ITextEditor = textEditor = ...
IPreferenceStore store = textEditor.getAdapter(IPreferenceStore.class);

It avoids using Java Reflection to access it:

Field field = AbstractTextEditor.class.getDeclaredField("fPreferenceStore");
field.setAccessible(true);
IPreferenceStore store = (IPreferenceStore) field.get(textEditor);

I have 2 usecases where I will need to get IPreferenceStore from a ITextEditor:

• .editorconfig support : the apply of indent_style = tab for a given editor must update the preference store of the editor:

ITextEditor textEditor = ...
IPreferenceStore store = textEditor.getAdapter(IPreferenceStore.class);
boolean spacesForTabs = (indent_style != tab)
if (oldSpacesForTabs != spacesForTabs) {
	store.firePropertyChangeEvent(EDITOR_SPACES_FOR_TABS, oldSpacesForTabs, spacesForTabs);
}

See:

• Provide .editorconfig support eclipse.platform.ui#877

• manage custom preferences for spaces according to a language for LSP4E (ex : having a xml spaces preferences for XML)

See:

• [XML] Preferences page for XML eclipse-wildwebdeveloper/wildwebdeveloper#139 (comment)
• Read from the editor preference store, not the global preference store, to retrieve formatting settings eclipse/lsp4e#245

LSP4e hard code teh Editors UI preference store for format:

IPreferenceStore store = EditorsUI.getPreferenceStore();
int tabWidth = store.getInt(AbstractDecoratedTextEditorPreferenceConstants.EDITOR_TAB_WIDTH);

See https://github.com/eclipse/lsp4e/blob/b5055d9afcc65dded8ce0ed9f15673e453e138c3/org.eclipse.lsp4e/src/org/eclipse/lsp4e/operations/format/LSPFormatter.java#L72

With this PR we will able to write:

ITextEditor textEditor = ...
IPreferenceStore store = textEditor.getAdapter(IPreferenceStore.class);
int tabWidth = store.getInt(AbstractDecoratedTextEditorPreferenceConstants.EDITOR_TAB_WIDTH);

The IPreferenceStore from editor could contains a custom IPreferenceStore for the XML language (XMLLanguagePreferenceStore which will return the value of custom spaces preferences for AbstractDecoratedTextEditorPreferenceConstants.EDITOR_TAB_WIDTH).

[laeubi]

That's a good approach here!

[laeubi]

@angelozerr the build failure is not related to your change, I'll take a look into it!

[mickaelistria]

I still have concerns with the idea of reusing the editor preference store while most of the possible operations may not require an editor but could instead start from an IDocument (think about JDT-LS).
We're discussing possible alternatives (or lack of) with @angelozerr and possible increments towards the bigger goal. I'd rather wait for some former conclusions to be reached before merging this.

[angelozerr]

@mickaelistria here a concrete usecase for this PR. LSP4e formatter needs to get the preference store of the generic editor to get tabWidth / insert spaces for a given language

https://github.com/eclipse/lsp4e/pull/271/files#diff-32bb8d5387f288529fd4b881ee69dbdd3282c84cc9c452605ab770d968732478R74

[angelozerr]

@mickaelistria here an new concrete sample with TM4e language configuration to get insert spaces / tab width when on enter rule (when user type Enter,it indent correctly and should get the tab with / insert spaces from the editor preference store

See * and access to the editor preference store https://github.com/eclipse/tm4e/pull/461/files#diff-2961979865d7499073f384bc8a71472fb1d5298dd54d3c4713c8f6ed94405801R295

[angelozerr]

Is there any chance to merge this PR? I tried to show you several usecases when IPreferenceStore must be get from an editor, and today we need to use Java reflection to access it, which is very bad.

[laeubi]

As it is about editor-config I think it is fair to adapt an editor but have no idea about the IDocument approach @mickaelistria maybe you can outline a bit more how this is supposed to work?

I also checked the code-base a bit and I don't see that only one approach will work, for a full solution one would most probably need different approaches, e.g IDocument, Editors and even IFile as one can format source files also without any editor (and thus possibly any document).

@angelozerr can you rebase the PR and make sure the build works?

[laeubi]

I think the change is not harmful and do not add any special maintenance burden.
Even if there are better ways to archive this it could be a good first step.

[angelozerr]

As it is about editor-config I think it is fair to adapt an editor but have no idea about the IDocument approach @mickaelistria maybe you can outline a bit more how this is supposed to work?

It is not only for .editorconfig support. The main idea is that an ITextEditor is created with preference store and some external component need to access to those preferences.

Take a sample without .editorconfig:. In WWD we have more and more users request who want to have the capability to customize spaces for XML (and don't use the global EditorUI preferences, like WTP XML editor). An external component created outside the WWD (like LSP4E, TM4E) needs to access to this preference to support format, auto editing strategies, etc, and they don't know WWD and which preferences store which is used to manages spaces.

This PR gives the capability to get a preference store registered in the editor without using Java Reflection because AbstractTextEditor#getPreferenceStore is protected.

I have several usescase to use this PR:

• LSP4E : https://github.com/eclipse/lsp4e/pull/271/files#diff-32bb8d5387f288529fd4b881ee69dbdd3282c84cc9c452605ab770d968732478R74
• TM4E : On enter rule don't use the proper insert spaces / tab width when those settings are changed eclipse/tm4e#460
• .editorconfig support

[angelozerr]

@angelozerr can you rebase the PR and make sure the build works?

rebased

[laeubi]

All checks are green, please @mickaelistria either adjust your review or give guidance how we can proceed here. My vote would be to merge this and if we later find better alternative it even is not harmful. Maybe also @vogella likes to give another review on this.

[vogella]

LGTM

[mickaelistria]

What about making getPreferenceStore() public?

[angelozerr]

What about making getPreferenceStore() public?

• it will change the API
• it will require to cast the ITextEditor into AbstractTextEditor to get the preference store
• it is not consistent with ITextViewer (you can get ITextViewer with getAdapter and getSourceViewer is protected)

[mickaelistria]

While I don't think those arguments are really strong; and that not publishing an API and instead relying on unspecified behavior is ugly; I looked at the code, and it really seems like there is a real effort to not expose such implementation details.
So let's stick to that (anti-)pattern for the moment.

ok