enhanced xpect editor override feature

[mehmet-karaman]

Added the ability to disable editor override at all or exclude override by extension.

[cdietrich]

@iloveeclipse can you please review this functionality and code whise....
wont have time to setup and env and try

[trancexpress]

OK, I checked the functionality, works as expected.

[cdietrich]

thx @trancexpress @mehmet-karaman

[trancexpress]

Please remove the full stop symbol here, its unusual to see it in a preference page.

[trancexpress]

I think we can skip the "will not be used if override is disabled", there is an overlap of the 2 preferences but I doubt the user cares.

Maybe you can disable the entire group with the settings per preference, if the checkbox for the global disable is ticket. A nice to have though, I don't think we need much fancy UI work here.

[trancexpress]

"Skip content check for XPECT for extensions" doesn't seem like optimal wording.

"Disable Xpect editor override for the following file extensions"? Not ideal but maybe better.

Would not hurt to explain what the editor override is, maybe with a hint hover... But again I don't think we need to invest much UI effort here. In the end its unlikely most users are aware the editor override even exists. Though that is a good reason to add an explanation...

[trancexpress]

thx @trancexpress @mehmet-karaman

Thank you for merging Christian, but please wait a bit for reviews.

[cdietrich]

@trancexpress sry. follow up pr is welcome

[iloveeclipse]

All involved: I wonder why do we have provided ability to skip the override for some file types but not the ability to enable override for some types?

Background: we have a fixed set of Xpect language related file extensions and an open set of all other file types.

For me, it makes more sense to restrict the list to the fixed known set of file types, as the set will not change/grow in same way as the unlimited set of all possible file types that shouldn't be touched fy Xpect at all.

[trancexpress]

Sounds fine to me. Should be simple to reverse the logic?

[cdietrich]

you mean by extending the extension point(s)?

[iloveeclipse]

I mean the code XpectEditorAssociationOverride.overrideDefaultEditor() currently "detects" xpect content type also by looking into the file itself which means the file is always read and if it contains XPECT xpect assumes it should "own" the editor.

So we added a check for files that we don't want to handle with Xpect before. Fine:

Xpect/org.eclipse.xpect.ui/src/org/eclipse/xpect/ui/editor/XpectEditorAssociationOverride.java

Lines 79 to 85 in b37f5dd

	public IEditorDescriptor overrideDefaultEditor(IEditorInput editorInput, IContentType contentType, IEditorDescriptor editorDescriptor) {
	if (XpectRootPreferencePage.isEditorOverrideDisabled()) {
	return editorDescriptor;
	}
	IFile file = getFile(editorInput);
	if (file == null || hasFavoriteEditor(file) || XpectRootPreferencePage.getSkipExtensionsList().contains(file.getFileExtension()))
	return editorDescriptor;

Instead (or additionally) we could add check if the file in the list of "known" Xpect files, because this is a smaller & fixed set, something like:

IFile file = getFile(editorInput);
		if (file == null || hasFavoriteEditor(file) || XpectRootPreferencePage.getSkipExtensionsList().contains(file.getFileExtension()))
			return editorDescriptor;

Set<String> knownXpectTypes = XpectRootPreferencePage.getKnownXpectExtensionsList();
if( !knownXpectTypes.isEmpty() && !knownXpectTypes.contains(file.getFileExtension())) {
			return editorDescriptor;
}

[cdietrich]

i am not sure what side effects that would have and what a
"known" xpect file would mean here. so how to populates knownXpectTypes.

this is why i pointed to the extension point. that registers file extensions to xpect.
i also dont know if xpect really needs the extension point it uses
or if it also reads the one from xtext
see FileExtensionInfoRegistry
unfortunately i dont know this part of xpect at all

[iloveeclipse]

so how to populates knownXpectTypes.

Same was we populate getSkipExtensionsList()?

The FileExtensionInfoRegistry registry doesn't help here I guess if it works on the file extensions contributed by Xtext in the current IDE, as the Xtext languages in the target platform not necessarily same we have in the IDE.

The product has 10 "Xpect" file types, IDE has zero.

[mehmet-karaman]

The Extension point is not usable, because the file extensions are necessary in the IDE where the DSL (and of course its tests) is/are developed, that's why I've introduced the new settings in the preferences page. But I am ok with the idea that we should reverse the logic. To not break anything else we should disable this by default.