eclipse-platform / eclipse.platform.ui

Eclipse Platform
https://projects.eclipse.org/projects/eclipse.platform
Eclipse Public License 2.0
78 stars 163 forks source link

Replace "Default" text editor with "Generic" text editor #1532

Open iloveeclipse opened 8 months ago

iloveeclipse commented 8 months ago

Recently I've seen few cases where current "Generic Text Editor" (org.eclipse.ui.genericeditor.GenericEditor) was competing with "Text Editor" (org.eclipse.ui.DefaultTextEditor) and IDE decided to use later one, even if "Generic" is almost always a better choice, especially together with tm4e. For example, because IDE doesn't allow to contribute "default" editor for content types, simply first found editor contribution is used to open "unknown" text files (the one from simple Text editor), and not the "Generic" one. Also having two "similarly named" editors in the IDE confuses users and leads to lengthy discussions which name should one use for these editors.

That all seem to be a problem of a parallel co-existence of two "plain text editors" in same IDE - old "simple" text editor and the "generic" one. This co-existence is caused by the IDE evolution, where the "generic" text editor (with more functionality under the hood) was added later but the old & simple one was not removed because it was (and still is) widely used.

What I would propose is to evaluate if we can simply replace editor contribution of the "simple" text editor with "generic" one, without removing implementation class / editor id, to stay compatible as much as possible.

Note: I know there is a GenericEditorWithIconAssociationOverride that can be easily changed to open org.eclipse.ui.genericeditor.GenericEditor instead of org.eclipse.ui.DefaultTextEditor, but that one doesn't allow to hide all the default text editor contributions from the UI and keeps it in the file associations/content types preferences, so it is not quite what needed here.

What I'm thinking about is to extend the org.eclipse.ui.editors extension point with alias or override element that would internally redirect all uses of one editor id with another one and so "hide" all contributions of the old one, eliminating duplicated and confusing editor contributions appearing in the UI.

So the org.eclipse.ui.genericeditor.GenericEditor would say "I'm an alias/override for org.eclipse.ui.DefaultTextEditor, so use me if anyone refers to org.eclipse.ui.DefaultTextEditor".

However, since there are also additional other extensions allowed that use editor id's as target (like org.eclipse.ui.editorActions, org.eclipse.ui.workbench.texteditor.hyperlinkDetectorTargets and so on), I'm not quite sure how far that "aliasing" should go. For sure we don't want to contribute same actions twice to the "aliased" editor...

So I tend to think the "alias/override" extension for an editor would only allow to replace all the IDE uses of editor ID to open an "aliased" editor and respective "content type / editor association" preferences used by IDE to find an editor to open.

Given that, one would be able to keep (for compatibility reasons) org.eclipse.ui.DefaultTextEditor contribution but leave only one "user visible" Text Editor in the IDE (contributed by org.eclipse.ui.genericeditor.GenericEditor).

What is also not yet clear to me, how a "legacy" product would be able to keep using the old, "aliased" org.eclipse.ui.DefaultTextEditor after updating the platform, if the "generic" alias/override shouldn't be used by the product for whatever reason. Since the compatibility is very important part of Eclipse SDK, I would assume a custom product could "disable" specific editor alias in some way, may be by aliasing the alias with the old editor id, or a system property or preference.

So WDYT about this idea?

vogella commented 8 months ago

+1 for only one text editor in the IDE. This will reduce the confusion for end users with the Eclipse IDE

Nice to see suggestions for simplifying the UX in the IDE from you Andrey.

mickaelistria commented 8 months ago

I'm a bit worried about overriding the ids of editors. I imagine some RCP products would like to use the DefaultEditor (plain text) to edit plain text, without highlighting nor anything. I suspect the ids to be considered as stable as API, so "org.eclipse.ui.DefaultTextEditor".equals(getEditor().getId()) && !(getEditor() instanceof ExtensionBasedTextEditor) should remain true to ensure we don't break people treating id as API. What I think would be more interesting would be to relabel the editors (as it is being discussed in https://github.com/eclipse-platform/eclipse.platform.ui/issues/1385 ) and just make that the default behavior of IDE.open(...) methods and the default return value for editor associations uses the Generic Editor, or whichever other editor with something based on a preference. Then we can flip the default editor in the IDE by changing the default preference, and consumers who want to stick to another editor can override the preference. This can also serve past or future stories where some better-than-default editor was existing (eg LicClipse) and it was impossible to really make it default.

So I would suggest we reorganize this proposal as "Define and use a preference to choose what is the default editor"

iloveeclipse commented 8 months ago

What I think would be more interesting would be to relabel the editors (as it is being discussed in #1385 )

Please not yet another discussion how to rename two text editors! It doesn't matter how we name these two editors, someone will be always upset by the chosen name. Better to fix the root cause and have one "Text" editor.

and just make that the default behavior of IDE.open(...) methods and the default return value for editor associations uses the Generic Editor, or whichever other editor with something based on a preference.

As described in the proposal, currently IDE doesn't allow to choose "preferred" editor if there are few editors registered for same content type. That must be implemented in your proposal, but the problem with two "text editors" appearing in UI will still remain and confuse everyone.

I'm a bit worried about overriding the ids of editors.

That's IMO the main issue here, the idea is to be able to switch "overriding" off for a RCP product that don't want to have it. I believe that is possible.

I also believe this will be a very small amount of products that won't have the new feature, and most of downstream clients would be happy to get rid of "schizophrenia" we have with two text editors.

mickaelistria commented 8 months ago

So maybe we can extract the extension point that define the Default editor in a separate bundle, and not including anymore in Platform? As a preliminary work, we can also try to set the the default attribute of the Generic Editor definition in plugin.xml to true and see what happens from there.

iloveeclipse commented 8 months ago

So maybe we can extract the extension point that define the Default editor in a separate bundle, and not including anymore in Platform?

We can't move extension point that defines any editor in question here to other bundle as this will again break compatibility with existing products that would need to change their code/imports.

As a preliminary work, we can also try to set the the default attribute of the Generic Editor definition in plugin.xml to true and see what happens from there.

Once again, that will not fix content types association problem and will not "hide" the twin contributions in UI.

mickaelistria commented 8 months ago

existing products that would need to change their code/imports.

Existing products would "only" have to add the legacyTextEditor fragment if they want it in their RCP; but by default it would be removed. In general, anything that changes the "default" behavior can break some clients, and every such change should ideally come with instructions of how to revert to legacy behavior (whether it is about adding a fragment, setting of preference, or whatever switch... is equivalent). If you want that products keep the legacy editor by default, then it means we need 1 more switches in Platform products to enable "hiding" the DefaultEditor (I suspect we can remove an entry from the editor registry based on some flag). But let's also discuss what people expect from a Platform: they expect the platform to also provide some innovations/fixes that they don't provide themselves. If Platform has established that the Generic Editor is best and that the Text Editor is worth being retired, then Platform has the responsibility of pushing this improvement to downstream consumers too. So changing the default by retiring the legacy text editor is IMO best here, as long as we provide a simple path to people who want it back (and with my proposal above, the migration path is just about adding a fragment).

Note that in the end, I don't really care about the legacy Text Editor personally; I just shared some potential concerns with overriding the id being more error-prone than retiring the editor; but in the end, I won't veto anything that is about making the Generic Editor the default one in the SDK. To be honest, that was one of our long-term goals with @sopotc when we introduced it. Having other community members actively requesting it and likely to make it happen is too good, and I'd enjoy seeing it happen even if it might hurt some products; because I think it's still in the best interest in the Platform itself.

nlisker commented 3 days ago

Can confirm that I got here because I was confused by having two seemingly indistinguishable text editors: image

Phasing out the default one in favor of the "Generic" one will be helpful.

mickaelistria commented 3 days ago

I don't remember about everything that was discussed, but I would now suggest

nlisker commented 2 days ago

I don't remember about everything that was discussed, but I would now suggest

  • Rename "Text editor" into "Plain Text editor"
  • Rename "Generic Text editor" into "Generic Code Editor"

What is the purpose of having 2 text editors, with one being inferior to the other? This is just confusing the users,

I completely agree with:

Please not yet another discussion how to rename two text editors! It doesn't matter how we name these two editors, someone will be always upset by the chosen name. Better to fix the root cause and have one "Text" editor.

merks commented 2 days ago

Sometimes the Generic Text Editor performs so poorly that I close it and use the crappy old editor without the pretty coloring, e.g., this file is difficult to edit at times:

https://github.com/eclipse-orbit/orbit-simrel/blob/main/maven-bnd/tp/MavenBND.target

The poor performance (freezing the UI for ~8 seconds) also happens in the target editor source tab, I expect for the same reason. Only the crappy old editor doesn't need to pay the prettiness penalty for this 3000 line XML file. Of course I really like the prettiness when the cost isn't intolerable...

nlisker commented 2 days ago

Aside from the investigation in #2276, having a bug in the Generic editor should not be a reason to keep an older editor just for that bug. The direction should be to solve the problem in the newer editor. Even if there is a problem with tm4e's syntax highlights in the generic editor for large files, it could be solved, for example, by a setting do disable text highlights for files larger than N lines.

Also, poor performance of syntax highlights is only relevant for the generic editor + tm4e, so it's not the editor itself that is buggy, but it's combination with tm4e.

mickaelistria commented 1 day ago

I think with #1385 resolved, most of the confusion between editors is removed and there is less reason to consider further changes.