Bug 325799 - UI: Support styled text in CUILabelProvider
Summary: UI: Support styled text in CUILabelProvider
Status: RESOLVED FIXED
Alias: None
Product: CDT
Classification: Tools
Component: cdt-source-nav (show other bugs)
Version: 7.0   Edit
Hardware: All All
: P3 enhancement (vote)
Target Milestone: 8.0   Edit
Assignee: Markus Schorn CLA
QA Contact: Markus Schorn CLA
URL:
Whiteboard:
Keywords: contributed
Depends on:
Blocks:
 
Reported: 2010-09-20 16:47 EDT by Patrick Hofer CLA
Modified: 2010-09-30 08:23 EDT (History)
0 users

See Also:


Attachments
proposed patch (117.07 KB, patch)
2010-09-26 07:29 EDT, Patrick Hofer CLA
no flags Details | Diff
updated patch (117.97 KB, patch)
2010-09-30 03:10 EDT, Patrick Hofer CLA
mschorn.eclipse: iplog+
Details | Diff
Updates the automated tests. (89.47 KB, patch)
2010-09-30 07:05 EDT, Markus Schorn CLA
mschorn.eclipse: iplog-
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Patrick Hofer CLA 2010-09-20 16:47:40 EDT
Styled text, with its coloring, makes it much easier to capture the relevant information in lots of the richer labels (the longer, more complex ones). Enhance the CDT views to support them. 
This will touch quite a lot of places in the UI, like the Outline View, Call Hierarchy and Type Hierarchy, just to name a few.

I'm preparing a patch to do this.
Comment 1 Patrick Hofer CLA 2010-09-24 00:16:13 EDT
Does anyone recall why CElementBaseLabels was moved to org.eclipse.cdt.core.model.util? 

Bug 176229 says "CElementLabels is in a UI package, but may be used in non-UI code, and outside CDT." Did the 'may be used' lead to any known real world usage?
  
I need StyledString from org.eclipse.jface.viewers for this enhancement in CElementLabels.
But I don't like to add a dependency from org.eclipse.cdt.core to org.eclipse.jface and think this wouldn't be well received.

Can we 'revert' the refactoring of  Bug 176229? Meaning putting a copy back to ui and deprecate the one in org.eclipse.cdt.core.
Comment 2 Patrick Hofer CLA 2010-09-26 07:29:48 EDT
Created attachment 179584 [details]
proposed patch

First draft version for the enhancement. 
I'm already pretty happy with this. Most views work as wanted. Mylyn is fine as well.

I tried to stay close to JDT on how I implemented this feature.

I'd like to get some feedback now.

There are still some todo's left.
 - The core needs some small extensions. I'm going to file a new bug for this 
   (Call hierarchy doesn't show return types yet)
 - The quick outline doesn't show the labels in colored style. 
   Need to investigate further.
 - How do I have to handle copyright issues (author, year) when I copy/move 
   large portions of code from another file or eclipse project (like JDT).
 - haven't checked all javadoc since tags
 - need to run the unittests
 - fixed some minor issues like typos within this patch. Do I have to put those 
   in a seperate patch?
Comment 3 Markus Schorn CLA 2010-09-29 09:33:32 EDT
(In reply to comment #1)
> ...
> Can we 'revert' the refactoring of  Bug 176229? Meaning putting a copy back to
> ui and deprecate the one in org.eclipse.cdt.core.
This is a reasonable approach.
Comment 4 Markus Schorn CLA 2010-09-29 09:43:06 EDT
(In reply to comment #2)
> Created an attachment (id=179584) [details]
> proposed patch
> First draft version for the enhancement. 
Great, the colored return types look really good!

> I'm already pretty happy with this. 
> ...
> There are still some todo's left.
> ...
>  - How do I have to handle copyright issues (author, year) when I copy/move 
>    large portions of code from another file or eclipse project (like JDT).
The way you do this in the patch is fine. If you drop an author or just mention
the current year, it is not really a problem, either. In the comment of the class it makes sense to document from where the code has been cloned from.

>  - haven't checked all javadoc since tags
In non-API code, there is no need for since tags.

>  - need to run the unittests
>  - fixed some minor issues like typos within this patch. Do I have to put 
>    those in a seperate patch?
It is best if you create a new patch containing all the changes and replace the
current proposal.
Comment 5 Patrick Hofer CLA 2010-09-29 18:01:39 EDT
> Great, the colored return types look really good!
Glad you like it. One of my favourite spots is the call hierarchy. If you enable 'show file names' and get multiple matches you can see all four colors in one label. Looks even better ;-)

Thanks for the other clarifications. I hope to have an updated patch soon.
Comment 6 Patrick Hofer CLA 2010-09-30 03:10:20 EDT
Created attachment 179922 [details]
updated patch

Here we go. I've addressed most of the issues mentioned in comment #2.

Bug 326372 helped to get rid of the TODO markers.

My todo list narrows down to the following:
 - The quick outline doesn't show the labels in colored style. 
   -> Any clues? I'm still puzzled because the outline view does it right. (Common super class there) 
       I might have overlooked something. Didn't find the time to investigate further until now.
 - need to run the unittests
   -> If anybody else did, I would be interested in the result (As green as possible if you managed to do so).
Comment 7 Markus Schorn CLA 2010-09-30 07:05:15 EDT
Created attachment 179943 [details]
Updates the automated tests.
Comment 8 Patrick Hofer CLA 2010-09-30 07:24:56 EDT
(In reply to comment #7)
> Updates the automated tests.
Thanks!
Comment 9 Markus Schorn CLA 2010-09-30 07:46:09 EDT
(In reply to comment #6)
> My todo list narrows down to the following:
>  - The quick outline doesn't show the labels in colored style. 
>    -> Any clues? I'm still puzzled because the outline view does it right.
When you use a label-provider that is not a CellLableProvider, than it gets
wrapped somewhere within the tree-widget. This can be fixed (worked around) 
by using a DecoratingCLabelProvider for the InformationControl.

Fixed in 8.0 > 20100930.
Comment 10 CDT Genie CLA 2010-09-30 08:23:04 EDT
*** cdt cvs genie on behalf of mschorn ***
Bug 325799: Support styled text in CUILabelProvider, by Patrick Hofer.

[*] CElementBaseLabels.java 1.12 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt-core/org.eclipse.cdt.core/model/org/eclipse/cdt/core/model/util/CElementBaseLabels.java?root=Tools_Project&r1=1.11&r2=1.12

[*] THHistoryListAction.java 1.6 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt-core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/typehierarchy/THHistoryListAction.java?root=Tools_Project&r1=1.5&r2=1.6
[*] THHistoryAction.java 1.6 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt-core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/typehierarchy/THHistoryAction.java?root=Tools_Project&r1=1.5&r2=1.6
[*] THViewPart.java 1.22 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt-core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/typehierarchy/THViewPart.java?root=Tools_Project&r1=1.21&r2=1.22
[*] THLabelProvider.java 1.8 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt-core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/typehierarchy/THLabelProvider.java?root=Tools_Project&r1=1.7&r2=1.8

[*] DecoratingCLabelProvider.java 1.9 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt-core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/viewsupport/DecoratingCLabelProvider.java?root=Tools_Project&r1=1.8&r2=1.9
[+] CElementLabelComposer.java  http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt-core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/viewsupport/CElementLabelComposer.java?root=Tools_Project&revision=1.1&view=markup
[*] AppearanceAwareLabelProvider.java 1.13 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt-core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/viewsupport/AppearanceAwareLabelProvider.java?root=Tools_Project&r1=1.12&r2=1.13
[*] CElementLabels.java 1.21 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt-core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/viewsupport/CElementLabels.java?root=Tools_Project&r1=1.20&r2=1.21
[*] CElementImageProvider.java 1.20 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt-core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/viewsupport/CElementImageProvider.java?root=Tools_Project&r1=1.19&r2=1.20
[*] CUILabelProvider.java 1.15 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt-core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/viewsupport/CUILabelProvider.java?root=Tools_Project&r1=1.14&r2=1.15
[*] StatusBarUpdater.java 1.10 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt-core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/viewsupport/StatusBarUpdater.java?root=Tools_Project&r1=1.9&r2=1.10

[*] IBHistoryListAction.java 1.8 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt-core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/includebrowser/IBHistoryListAction.java?root=Tools_Project&r1=1.7&r2=1.8
[*] IBHistoryAction.java 1.6 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt-core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/includebrowser/IBHistoryAction.java?root=Tools_Project&r1=1.5&r2=1.6

[*] COutlineInformationControl.java 1.15 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt-core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/text/COutlineInformationControl.java?root=Tools_Project&r1=1.14&r2=1.15

[*] OpenDeclarationsJob.java 1.21 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt-core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/search/actions/OpenDeclarationsJob.java?root=Tools_Project&r1=1.20&r2=1.21

[*] CHLabelProvider.java 1.12 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt-core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/callhierarchy/CHLabelProvider.java?root=Tools_Project&r1=1.11&r2=1.12
[*] CallHierarchyUI.java 1.25 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt-core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/callhierarchy/CallHierarchyUI.java?root=Tools_Project&r1=1.24&r2=1.25
[*] CHHistoryListAction.java 1.6 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt-core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/callhierarchy/CHHistoryListAction.java?root=Tools_Project&r1=1.5&r2=1.6
[*] CHViewPart.java 1.29 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt-core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/callhierarchy/CHViewPart.java?root=Tools_Project&r1=1.28&r2=1.29
[*] CHHistoryAction.java 1.8 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt-core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/callhierarchy/CHHistoryAction.java?root=Tools_Project&r1=1.7&r2=1.8

[*] CViewLabelProvider.java 1.24 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt-core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/cview/CViewLabelProvider.java?root=Tools_Project&r1=1.23&r2=1.24

[*] AbstractCModelOutlinePage.java 1.16 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt-core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/editor/AbstractCModelOutlinePage.java?root=Tools_Project&r1=1.15&r2=1.16

[*] Strings.java 1.5 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt-core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/corext/util/Strings.java?root=Tools_Project&r1=1.4&r2=1.5

[*] PDOMSearchQuery.java 1.30 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt-core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/search/PDOMSearchQuery.java?root=Tools_Project&r1=1.29&r2=1.30

[*] OpenActionUtil.java 1.10 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt-core/org.eclipse.cdt.ui/src/org/eclipse/cdt/internal/ui/actions/OpenActionUtil.java?root=Tools_Project&r1=1.9&r2=1.10

[*] CElementLabelProvider.java 1.34 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt-core/org.eclipse.cdt.ui/src/org/eclipse/cdt/ui/CElementLabelProvider.java?root=Tools_Project&r1=1.33&r2=1.34

[*] CTypeHierarchyTest.java 1.7 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt-core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/typehierarchy/CTypeHierarchyTest.java?root=Tools_Project&r1=1.6&r2=1.7
[*] TypeHierarchyAcrossProjectsTest.java 1.7 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt-core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/typehierarchy/TypeHierarchyAcrossProjectsTest.java?root=Tools_Project&r1=1.6&r2=1.7
[*] CppTypeHierarchyTest.java 1.7 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt-core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/typehierarchy/CppTypeHierarchyTest.java?root=Tools_Project&r1=1.6&r2=1.7
[*] QuickTypeHierarchyTest.java 1.4 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt-core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/typehierarchy/QuickTypeHierarchyTest.java?root=Tools_Project&r1=1.3&r2=1.4

[*] CallHierarchyAcrossProjectsTest.java 1.12 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt-core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/callhierarchy/CallHierarchyAcrossProjectsTest.java?root=Tools_Project&r1=1.11&r2=1.12
[*] BasicCppCallHierarchyTest.java 1.14 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt-core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/callhierarchy/BasicCppCallHierarchyTest.java?root=Tools_Project&r1=1.13&r2=1.14
[*] InitializersInCallHierarchyTest.java 1.7 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt-core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/callhierarchy/InitializersInCallHierarchyTest.java?root=Tools_Project&r1=1.6&r2=1.7
[*] BasicCallHierarchyTest.java 1.24 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt-core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/callhierarchy/BasicCallHierarchyTest.java?root=Tools_Project&r1=1.23&r2=1.24
[*] CppCallHierarchyTest.java 1.15 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt-core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/callhierarchy/CppCallHierarchyTest.java?root=Tools_Project&r1=1.14&r2=1.15
[*] CallHierarchyBugs.java 1.20 http://dev.eclipse.org/viewcvs/index.cgi/org.eclipse.cdt-core/org.eclipse.cdt.ui.tests/ui/org/eclipse/cdt/ui/tests/callhierarchy/CallHierarchyBugs.java?root=Tools_Project&r1=1.19&r2=1.20