Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ITextEditorAware API to inject ITextEditor. #108

Closed

Conversation

angelozerr
Copy link
Contributor

ITextEditorAware API to inject ITextEditor.

Signed-off-by: azerr [email protected]

@angelozerr
Copy link
Contributor Author

angelozerr commented Oct 27, 2022

ITextEditorAware API allows to inject an instance of ITextEditor.

I create this API because I need it for eclipse-platform/eclipse.platform.ui#877

More I think this API can be very helpfull for generic editor. My idea is when generic editor creates a custom reconciler for instance, it check that the instanciated class implemented ITextEditorAware and in this case it set the editor to the contribution here

With this code:

@SuppressWarnings("unchecked")
public T createDelegate(ITextEditor editor) {
try {
	T delegateInstance = (T) extension.createExecutableExtension(CLASS_ATTRIBUTE);
	if (delegateInstance instanceof ITextEditorAware) {
		((ITextEditorAware) delegateInstance).setEditor(editor);
	}
	return delegateInstance;
} catch (CoreException e) {
	GenericEditorPlugin.getDefault().getLog()
			.log(new Status(IStatus.ERROR, GenericEditorPlugin.BUNDLE_ID, e.getMessage(), e));
}
return null;
}

A good usecase is in LSP4e https://github.com/eclipse/lsp4e/blob/c6859853bd37c5ae70e9ee9ba43495ff28d668d3/org.eclipse.lsp4e/src/org/eclipse/lsp4e/operations/completion/LSContentAssistProcessor.java#L197

Here the required editor is get with a static method, by getting the current activate editor. It should be cleaner to do like this:

public class LSContentAssistProcessor implements IContentAssistProcessor, ITextEditorAware {

    private TextEditor editor;

    @Override
    public void setEditor(ITextEditor editor) {
       this.editor = editor;
   }
  ...
}

@mickaelistria do you like this idea?

@angelozerr
Copy link
Contributor Author

Ok I have updated my PR to use ITextEditorAware with generic editor. It will opens the door to access to the generic editor in any contribution.

The main idea is to avoid getting the activated editor to get the editor in each contribution. I see that:

@angelozerr
Copy link
Contributor Author

@mickaelistria here a concrete sample in the PR eclipse-tm4e/tm4e#461 which uses ITextEditorAware to have cleaner code in TM4e language configuration

try {
return (T) extension.createExecutableExtension(CLASS_ATTRIBUTE);
T delegateInstance = (T) extension.createExecutableExtension(CLASS_ATTRIBUTE);
if (delegateInstance instanceof ITextEditorAware) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since the API ITextEditorAware was added in ui.workbench.texteditor, I'd expect it to be respected there, too. Right now, it looks as if something was added in ui.workbench just for the sake of ui.genericeditor. This has a bad smell to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with you @szarnekow I think more and more that this API should be hosted in generic editor.

I think too ItextViewerLyfecicle #107 should be hosted in generic editor

@mickaelistria are you OK with that.

package org.eclipse.ui.texteditor;

/**
* API to inject {@link ITextEditor} instance.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please try to add documentation what an implementor can expect to happen if his random class implements the "API to inject an ITextEditor".

@mickaelistria
Copy link
Contributor

eclipse-pde/eclipse.pde#402 should fix further issues.

@vogella
Copy link
Contributor

vogella commented Mar 23, 2023

Is this PR still relevant?

@laeubi
Copy link
Contributor

laeubi commented Jun 28, 2023

This repository was merged with https://github.com/eclipse-platform/eclipse.platform.ui if your pull-request is still relevant please rebase the code and push it to the new repository.

@laeubi laeubi closed this Jun 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants