Bug 513034 - [generic editor] Support content-type specific icons for generic editor
Summary: [generic editor] Support content-type specific icons for generic editor
Status: RESOLVED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: Text (show other bugs)
Version: 4.7   Edit
Hardware: All All
: P3 enhancement (vote)
Target Milestone: 4.13 M3   Edit
Assignee: Lakshminarayana CLA
QA Contact: Mickael Istria CLA
URL:
Whiteboard:
Keywords: Documentation, noteworthy, plan
: 527626 (view as bug list)
Depends on:
Blocks:
 
Reported: 2017-03-03 02:08 EST by Alexander Kurtakov CLA
Modified: 2019-08-09 10:04 EDT (History)
9 users (show)

See Also:


Attachments
Screenshot showing genericeditor with same icon for multiple contenttypes. (9.79 KB, image/png)
2017-03-03 02:10 EST, Alexander Kurtakov CLA
no flags Details
Hand crafted generic editor for java annotated icon. (1.07 KB, image/png)
2017-06-07 12:02 EDT, Alexander Kurtakov CLA
no flags Details

Note You need to log in before you can comment on or make changes to this bug.
Description Alexander Kurtakov CLA 2017-03-03 02:08:46 EST
With generic editor starting to be used by more and more content-types using the same icon for all of them becomes really inconvenient as one may end up with only this icon being shown making it useless clutter as it's not helping visually distinguish between editors.
We should find a way to either define icon for generic editor to use for given content-type or (preferably) extend the content type definiton to contain default icon which the generic editor to use after that.
Comment 1 Alexander Kurtakov CLA 2017-03-03 02:10:41 EST
Created attachment 267076 [details]
Screenshot showing genericeditor with same icon for multiple contenttypes.
Comment 2 Mickael Istria CLA 2017-03-06 04:29:53 EST
(In reply to Alexander Kurtakov from comment #0)
> (preferably) extend the content type definiton to
> contain default icon which the generic editor to use after that.

+1.
Comment 3 Dani Megert CLA 2017-04-14 04:05:34 EDT
The icon should comes with the generic editor extension. That way an extension to the generic editor is self contained. It allows to provide icons that differ but still indicate that the generic editor is used. In addition it prevents collisions, i.e. different editors using the same icon because they work on the same content type.
Comment 4 Alexander Kurtakov CLA 2017-05-02 07:09:23 EDT
(In reply to Dani Megert from comment #3)
> The icon should comes with the generic editor extension. That way an
> extension to the generic editor is self contained. It allows to provide
> icons that differ but still indicate that the generic editor is used. In
> addition it prevents collisions, i.e. different editors using the same icon
> because they work on the same content type.

Which extension do you have in mind Dani?
A new one? Mickael, wdyt?
Comment 5 Dani Megert CLA 2017-05-02 09:03:55 EDT
(In reply to Alexander Kurtakov from comment #4)
> (In reply to Dani Megert from comment #3)
> > The icon should comes with the generic editor extension. That way an
> > extension to the generic editor is self contained. It allows to provide
> > icons that differ but still indicate that the generic editor is used. In
> > addition it prevents collisions, i.e. different editors using the same icon
> > because they work on the same content type.
> 
> Which extension do you have in mind Dani?
> A new one?

Yes. I think adding a new one just for the icon would be overkill, but something like 
org.eclipse.ui.genericeditor.editors which would be able to provide information simular to org.eclipse.ui.editors could be handy.
Comment 6 Mickael Istria CLA 2017-05-02 09:34:53 EDT
And what about adding an optional "icon" attribute for the content-type extensions? Clients (such as generic editor) could use it at their own convenience.
Comment 7 Dani Megert CLA 2017-05-02 13:04:21 EDT
(In reply to Mickael Istria from comment #6)
> And what about adding an optional "icon" attribute for the content-type
> extensions? Clients (such as generic editor) could use it at their own
> convenience.

That would be a layer breaker. UI related stuff has nothing to do there.
Comment 8 Mickael Istria CLA 2017-06-07 05:52:39 EDT
(In reply to Dani Megert from comment #7)
> That would be a layer breaker. UI related stuff has nothing to do there.

Ok, and what about a new extension pint such as org.eclipse.ui.contentTypeImages similarly to the org.eclipse.ui.commandImages?
I'd rather avoid making content-type <-> Image association specific to the generic editor. This mapping seems more generic, may be profitable to others, so it should most likely be an extension in the ui bundle.
Comment 9 Alexander Kurtakov CLA 2017-06-07 06:27:08 EDT
(In reply to Mickael Istria from comment #8)
> (In reply to Dani Megert from comment #7)
> > That would be a layer breaker. UI related stuff has nothing to do there.
> 
> Ok, and what about a new extension pint such as
> org.eclipse.ui.contentTypeImages similarly to the
> org.eclipse.ui.commandImages?
> I'd rather avoid making content-type <-> Image association specific to the
> generic editor. This mapping seems more generic, may be profitable to
> others, so it should most likely be an extension in the ui bundle.

I really like that approach.
Comment 10 Dani Megert CLA 2017-06-07 10:33:47 EDT
(In reply to Alexander Kurtakov from comment #9)
> (In reply to Mickael Istria from comment #8)
> > (In reply to Dani Megert from comment #7)
> > > That would be a layer breaker. UI related stuff has nothing to do there.
> > 
> > Ok, and what about a new extension pint such as
> > org.eclipse.ui.contentTypeImages similarly to the
> > org.eclipse.ui.commandImages?
> > I'd rather avoid making content-type <-> Image association specific to the
> > generic editor. This mapping seems more generic, may be profitable to
> > others, so it should most likely be an extension in the ui bundle.
> 
> I really like that approach.

-1. It should be something specific to the Generic editor. Otherwise the user will not be able to differentiate which kind of editor is used. Assume besides the Java editor and the Generic editor we have Super editor. If they all use the same icon that would be bad for the user. Read again comment 3 and comment 5.
Comment 11 Alexander Kurtakov CLA 2017-06-07 10:52:54 EDT
(In reply to Dani Megert from comment #10)
> (In reply to Alexander Kurtakov from comment #9)
> > (In reply to Mickael Istria from comment #8)
> > > (In reply to Dani Megert from comment #7)
> > > > That would be a layer breaker. UI related stuff has nothing to do there.
> > > 
> > > Ok, and what about a new extension pint such as
> > > org.eclipse.ui.contentTypeImages similarly to the
> > > org.eclipse.ui.commandImages?
> > > I'd rather avoid making content-type <-> Image association specific to the
> > > generic editor. This mapping seems more generic, may be profitable to
> > > others, so it should most likely be an extension in the ui bundle.
> > 
> > I really like that approach.
> 
> -1. It should be something specific to the Generic editor. Otherwise the
> user will not be able to differentiate which kind of editor is used. Assume
> besides the Java editor and the Generic editor we have Super editor. If they
> all use the same icon that would be bad for the user. Read again comment 3
> and comment 5.

I see your point. On the other side providing a mapping between content-type and icon can be helpful even in this case if the editors use the content type icon to annotate the editor one or vice-versa. That could be the best of both worlds and not requiring the new "Super" editor to come up with their own extension point to register content type icons.
Comment 12 Dani Megert CLA 2017-06-07 11:18:22 EDT
(In reply to Alexander Kurtakov from comment #11)
> I see your point. On the other side providing a mapping between content-type
> and icon can be helpful even in this case if the editors use the content
> type icon to annotate the editor one or vice-versa. That could be the best
> of both worlds and not requiring the new "Super" editor to come up with
> their own extension point to register content type icons.

There's another argument for my solution. Currently if you use Open With on a Java file you see different icons for the Java and the Generic editor, which I think is very useful. With your proposed approach all editors for Java files would appear with the same icon. Again, -1 ;-)
Comment 13 Alexander Kurtakov CLA 2017-06-07 11:24:20 EDT
(In reply to Dani Megert from comment #12)
> (In reply to Alexander Kurtakov from comment #11)
> > I see your point. On the other side providing a mapping between content-type
> > and icon can be helpful even in this case if the editors use the content
> > type icon to annotate the editor one or vice-versa. That could be the best
> > of both worlds and not requiring the new "Super" editor to come up with
> > their own extension point to register content type icons.
> 
> There's another argument for my solution. Currently if you use Open With on
> a Java file you see different icons for the Java and the Generic editor,
> which I think is very useful. With your proposed approach all editors for
> Java files would appear with the same icon. Again, -1 ;-)

No, in my proposal they will use different icons. Java editor will use whatever icon Java editor specified and Generic editor will use generi editor icon annotated with the "java" content type icon in a similar way we annotate icons in Package explorer with warning/error/info icons. Thus it will be clear which editor is used and which content type is open. The visual mechanics might be slightly different but that's the idea.
Comment 14 Mickael Istria CLA 2017-06-07 11:26:49 EDT
@Dani: I didn't say we should replace all file or editor icons by the content-type icon.
I only mention that such a registry, could be independent from generic editor and might be useful for other editors or other UI elements than the generic editor, so it would make more sense as a more generic registry out of the generic editor. The generic editor would be one (and the only ATM) client of this registry. All existing editors would keep using their specific icons.
As the effort of having this registry in a common place is as much as having one in the generic editor, it seems like it's worth having it in the most shared place possible.
JDT editor icon and a .java content-type icon would coexist. Your worries of all editors with same icon is not part nor an immediate consequence of the story proposed in comment #6.
Comment 15 Dani Megert CLA 2017-06-07 11:34:22 EDT
(In reply to Alexander Kurtakov from comment #13)
> No, in my proposal they will use different icons. Java editor will use
> whatever icon Java editor specified and Generic editor will use generi
> editor icon annotated with the "java" content type icon in a similar way we
> annotate icons in Package explorer with warning/error/info icons.

So, the extension point would provide an option decorator and not the icon? Can you provide more details about what you actually propose? How would it look like, who would (be forced to) adopt this?
Comment 16 Dani Megert CLA 2017-06-07 11:35:54 EDT
(In reply to Mickael Istria from comment #14)
> @Dani: I didn't say we should replace all file or editor icons by the
> content-type icon.
> I only mention that such a registry, could be independent from generic
> editor and might be useful for other editors or other UI elements than the
> generic editor, so it would make more sense as a more generic registry out
> of the generic editor. The generic editor would be one (and the only ATM)
> client of this registry. All existing editors would keep using their
> specific icons.
> As the effort of having this registry in a common place is as much as having
> one in the generic editor, it seems like it's worth having it in the most
> shared place possible.
> JDT editor icon and a .java content-type icon would coexist. Your worries of
> all editors with same icon is not part nor an immediate consequence of the
> story proposed in comment #6.

It doesn't make sense to me to introduce a mapping that's then not used, or, as you would probably do, only used by the generic editor.
Comment 17 Alexander Kurtakov CLA 2017-06-07 12:02:46 EDT
Created attachment 268792 [details]
Hand crafted generic editor for java annotated icon.
Comment 18 Mickael Istria CLA 2017-06-07 12:07:55 EDT
(In reply to Dani Megert from comment #16)
> It doesn't make sense to me to introduce a mapping that's then not used, or,
> as you would probably do, only used by the generic editor.

This mapping could be used in the content-type preference page to decorate the node, in LSP4E and TM4E which rely on content-type for their association independently of the actual editor, in in about all places where the content-type is mentioned to have a picture additionally to the name.
Content-type is semantic standalone concept mentioned separately of the editor in several places already. This registry could be used by any of those to improve the UI by adding another recognizable picture in all those places.
Comment 19 Alexander Kurtakov CLA 2017-06-07 12:12:35 EDT
(In reply to Dani Megert from comment #15)
> (In reply to Alexander Kurtakov from comment #13)
> > No, in my proposal they will use different icons. Java editor will use
> > whatever icon Java editor specified and Generic editor will use generi
> > editor icon annotated with the "java" content type icon in a similar way we
> > annotate icons in Package explorer with warning/error/info icons.
> 
> So, the extension point would provide an option decorator and not the icon?

Well, technically decorator is just another icon which can be scaled and used as decorator/creating composed icon.

> Can you provide more details about what you actually propose?

See the attached icon (a minute work to apply one icon on top of the other). That could be made way more beautiful.

> How would it
> look like, who would (be forced to) adopt this?

For now I envision "Content types" preference page showing the icon and generic editor using it to create a composite icon with it.
Comment 20 Dani Megert CLA 2017-06-15 11:20:13 EDT
(In reply to Alexander Kurtakov from comment #19)
> Well, technically decorator is just another icon which can be scaled and
> used as decorator/creating composed icon.

Why not sue the projectNatureImages as a start?
 

> > How would it
> > look like, who would (be forced to) adopt this?
> 
> For now I envision "Content types" preference page showing the icon and
> generic editor using it to create a composite icon with it.

I'm -100 for that.
Comment 21 Dani Megert CLA 2017-06-15 11:22:33 EDT
(In reply to Dani Megert from comment #20)
> > For now I envision "Content types" preference page showing the icon and
> > generic editor using it to create a composite icon with it.
> 
> I'm -100 for that.

Reason: That page would show an icon for the Java source content type which would not be used by the primary editor. No go!

Please reconsider comment 5.
Comment 22 Dani Megert CLA 2017-06-15 11:30:06 EDT
(In reply to Mickael Istria from comment #18)
> (In reply to Dani Megert from comment #16)
> > It doesn't make sense to me to introduce a mapping that's then not used, or,
> > as you would probably do, only used by the generic editor.
> 
> This mapping could be used in the content-type preference page to decorate
> the node, in LSP4E and TM4E which rely on content-type for their association
> independently of the actual editor, in in about all places where the
> content-type is mentioned to have a picture additionally to the name.
> Content-type is semantic standalone concept mentioned separately of the
> editor in several places already. This registry could be used by any of
> those to improve the UI by adding another recognizable picture in all those
> places.

You would not gain anything with respect to this bug report! JDT owns the the JDT content types and JDT would provide that icon. It would be the one for the Java editors. That the generic editor would also use the same icon would be vetoed by JDT. ==> you'd still need a mechanism to provide an editor icon for the JDT specific generic editor.
Comment 23 Mickael Istria CLA 2017-06-15 13:21:00 EDT
h(In reply to Dani Megert from comment #22)
> You would not gain anything with respect to this bug report! JDT owns the
> the JDT content types and JDT would provide that icon.

If someone goes for a Java editor based on Generic Editor + some Java LSP, there would not likely not be JDT around to provide the content-type. Such plugin would define another content-type for .java files, and may or may not use the JDT icon...

> That the generic editor would also use the same icon would be vetoed by JDT.

I don't get how JDT perception on this feature changes anything to the value proposition and feasability. If JDT doesn't want to provide an icon for its Java content-type, no-one is forcing it to do it.
But if someone introduce a content-type and prefers to have an icon associated with it, there is no reason JDT prevents this from being implemented

> ==> you'd still need a mechanism to provide an editor icon for the JDT specific generic editor.

There is and will never be such thing as a "JDT-specific generic editor", don't you notice the oxymoron in it? ;)

Anyway, let's just avoid such fight for the moment. We'll go for an regular generic editor extension to associate an icon with a content-type in the generic editor namespace and let only the generic editor use it.
And if anyone wants an icon for content-types for some other use-case later, then I'll let you answer them that they should duplicate or refactor an existing extension-point :P
Comment 24 Dani Megert CLA 2017-06-16 05:18:41 EDT
(In reply to Mickael Istria from comment #23)
> h(In reply to Dani Megert from comment #22)
> > You would not gain anything with respect to this bug report! JDT owns the
> > the JDT content types and JDT would provide that icon.
> 
> If someone goes for a Java editor based on Generic Editor + some Java LSP,
> there would not likely not be JDT around to provide the content-type. Such
> plugin would define another content-type for .java files, and may or may not
> use the JDT icon...

You're missing the point that the Java editor and the generic editor are part of the Eclipse SDK and the generic editor can be used to open Java files.



> > That the generic editor would also use the same icon would be vetoed by JDT.
> 
> I don't get how JDT perception on this feature changes anything to the value
> proposition and feasability. If JDT doesn't want to provide an icon for its
> Java content-type, no-one is forcing it to do it.
> But if someone introduce a content-type and prefers to have an icon
> associated with it, there is no reason JDT prevents this from being
> implemented
> 
> > ==> you'd still need a mechanism to provide an editor icon for the JDT specific generic editor.
> 
> There is and will never be such thing as a "JDT-specific generic editor",
> don't you notice the oxymoron in it? ;)

Let's assume for a moment that we add that extension point and JDT UI would play nice citizen and contribute the image for the content type and of course uses it in the editor. As per this bug report the generic editor would do the same. Therefore we would end up with the same icon for both editors in the Eclipse SDK.


There's another confusion that would be introduced: all our navigators use the default editor icon when showing a file. This would either have to be changed, loosing the information which editor will be used to open the file, or leave the user in the dark why there's an icon for the content type.
Comment 25 Mickael Istria CLA 2017-07-05 06:33:08 EDT
It's relatively easy to make the generic editor show a specific icon on the editor tab.
However, at the moment, unless we allow alternative strategies for icon computation in WorkbenchFile.java, it seems almost impossible to have the Project Explorer reusing the dynamic icon provided by the content-type extension; it always ends up delegating to the EditorDescriptor.getImageDescriptor() which uses the static icon defined in for the editor declaration from plugin.xml.
So if we implement "dymanic" icon for Generic Editor and can't do it for Project Explorer, it means that we'd be stuck with the generic editor icon in Project Explroer and show a specific icon on the editor tab.
I don't know whether this would be perceived as an improvement.

If we really do want to have it working, it requires more investment in order to make File->Image resolution more flexible and powerful; for example:
1. by allowing content-type->image association extension point in the core bundle (not only generic editor), and to change the WorkbenchFile so that if editor is found and editor icon is null, it looks for a content-type icon, or
2. same as above, but add a "preferContentTypeIcon" to the editor definition to let WorkbenchFile prefer the contentTypeIcon over the editor one if editor says so, or
3. let editor extension point define an "IconStrategy" that would be accessible from WorkbenchFile without instantiating the whole editor so editor could plug dynamic icon resolution to WorkbenchFile.
(and probably other possible solutions)

Proposal 1. would be my favourite at the moment, as there is currently a default in case of null editor icon which is to show the IMG_OBJ_FILE. It seems consensual to plug something smarter here if we have a content-type->image registry available. It also require not modification of the editor extension point and very minimal adoption effort for Generic Editor or other editors who want it (simply remove icon).

I don't know how to move forward with this issue, unless everyone agrees on one of the proposal.
Comment 26 Dani Megert CLA 2017-11-22 10:18:17 EST
*** Bug 527626 has been marked as a duplicate of this bug. ***
Comment 27 Mickael Istria CLA 2017-11-24 12:57:49 EST
I'm giving another round on this issue and have a new proposal:
* we keep the generic editor icon as it (empty file, 2 purple curly braces), it's the one that would show up in the Project Explorer (as at the moment, we cannot map map a file to another icon than the one statically defined in the editor definition)
* we introduce an extension point "org.eclipse.ui.genericeditor.icons", which allows child elements "contentTypeIcon" to associate a content-type to a static icon.
* The generic editor, when instantiated, would lookup for an existing icon for the given content-type AND add the purple {} decorator to this icon. That way, the content-type icon doesn't have to be specific to the editor (cool!), and the editor takes care of making it more consistent with Project Explorer by adding its "touch".
Comment 28 Alexander Kurtakov CLA 2017-11-24 13:02:37 EST
Sounds good to me. I'm eager to see how it looks in reality.
Comment 29 Dani Megert CLA 2017-12-01 10:53:05 EST
(In reply to Mickael Istria from comment #27)
> I'm giving another round on this issue and have a new proposal:
> * we keep the generic editor icon as it (empty file, 2 purple curly braces),
> it's the one that would show up in the Project Explorer (as at the moment,
> we cannot map map a file to another icon than the one statically defined in
> the editor definition)
> * we introduce an extension point "org.eclipse.ui.genericeditor.icons",
> which allows child elements "contentTypeIcon" to associate a content-type to
> a static icon.

+1, but I would use "icon" and "contentType" as attributes.


> * The generic editor, when instantiated, would lookup for an existing icon
> for the given content-type AND add the purple {} decorator to this icon.
> That way, the content-type icon doesn't have to be specific to the editor
> (cool!), and the editor takes care of making it more consistent with Project
> Explorer by adding its "touch".

The icon must also be used in the navigators if the Generic Editor is the one to edit the file.
Comment 30 Lars Vogel CLA 2018-09-10 09:39:58 EDT
Mickael, would it be possible to target this bug for 4.10? Now that more editors use the generic editor a content-type specific icon would be very handy.
Comment 31 Mickael Istria CLA 2018-09-10 09:55:41 EDT
(In reply to Lars Vogel from comment #30)
> Mickael, would it be possible to target this bug for 4.10? Now that more
> editors use the generic editor a content-type specific icon would be very
> handy.

I don't have a proper solution to suggest at the moment, and this is too far away in my list of priority for me to really fix it by December.
But if anyone is willing to do the work, that'd be welcome!
Comment 32 Mickael Istria CLA 2019-07-02 15:26:45 EDT
The suggest patch is in a good shape. While it's tricky to get it in by the end of this week for M1, it seems realistic to have it in for M2/M3.
Comment 33 Pierre-Yves Bigourdan CLA 2019-07-03 16:38:05 EDT
Thanks for the patch Lakshminarayana, I was looking forward to this feature!
Comment 35 Mickael Istria CLA 2019-07-18 10:21:25 EDT
@Lakshminarayana Thanks for the patch!
Can you please add
1. a note in the noteworthy doc
2. a reference to the extension point in the org.eclipse.platform.isv.doc bundle?
Comment 36 Eclipse Genie CLA 2019-07-19 06:29:25 EDT
New Gerrit change created: https://git.eclipse.org/r/146350
Comment 37 Eclipse Genie CLA 2019-07-19 07:21:07 EDT
New Gerrit change created: https://git.eclipse.org/r/146353
Comment 39 Eclipse Genie CLA 2019-07-22 10:01:48 EDT
New Gerrit change created: https://git.eclipse.org/r/146458
Comment 40 Eclipse Genie CLA 2019-07-22 11:24:45 EDT
New Gerrit change created: https://git.eclipse.org/r/146463
Comment 41 Mickael Istria CLA 2019-07-22 11:33:32 EDT
Thanks for this good addition Lakshminarayana!
Comment 42 Lakshminarayana CLA 2019-07-22 11:57:00 EDT
(In reply to Mickael Istria from comment #41)
> Thanks for this good addition Lakshminarayana!

No Problem. Without your support, it won't be completed. :)
Comment 43 Pierre-Yves Bigourdan CLA 2019-07-27 17:21:55 EDT
Am I correct in thinking that this will only change the icon in the editor tabs, but not the icon in the project explorer?
Comment 44 Lakshminarayana CLA 2019-07-27 21:26:38 EDT
(In reply to Pierre-Yves B. from comment #43)
> Am I correct in thinking that this will only change the icon in the editor
> tabs, but not the icon in the project explorer?

Yes, Only in the editor tabs.Not in the Project or Package Explorer. I will create a new bug for it.

There is one more commit that not yet merged. But, marked as fixed.
https://git.eclipse.org/r/#/c/146353/

@Mickael Can you check?
Comment 46 Lakshminarayana CLA 2019-07-30 11:32:27 EDT
Created new bug for Package or Project explorer issue.
Bug 549665
Comment 47 Eclipse Genie CLA 2019-08-05 08:56:27 EDT
New Gerrit change created: https://git.eclipse.org/r/147057
Comment 49 Eclipse Genie CLA 2019-08-05 10:45:33 EDT
New Gerrit change created: https://git.eclipse.org/r/147069
Comment 50 Lakshminarayana CLA 2019-08-05 10:46:50 EDT
(In reply to Eclipse Genie from comment #49)
> New Gerrit change created: https://git.eclipse.org/r/147069

Updated the image with PE file contents and context menu.
Comment 51 Lakshminarayana CLA 2019-08-09 02:08:11 EDT
(In reply to Lakshminarayana from comment #50)
> (In reply to Eclipse Genie from comment #49)
> > New Gerrit change created: https://git.eclipse.org/r/147069
> 
> Updated the image with PE file contents and context menu.

Mickael< Can you review and merge the change?
Comment 53 Eclipse Genie CLA 2019-08-09 09:46:52 EDT
New Gerrit change created: https://git.eclipse.org/r/147376