Bug 271757 - TextMergeViewer.createSourceViewer(...) should return SourceViewer
Summary: TextMergeViewer.createSourceViewer(...) should return SourceViewer
Status: RESOLVED FIXED
Alias: None
Product: Platform
Classification: Eclipse Project
Component: Compare (show other bugs)
Version: 3.5   Edit
Hardware: All All
: P3 normal (vote)
Target Milestone: 3.5 M7   Edit
Assignee: Tomasz Zarna CLA
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks: 259411
  Show dependency tree
 
Reported: 2009-04-09 08:55 EDT by Tomasz Zarna CLA
Modified: 2009-04-15 04:08 EDT (History)
3 users (show)

See Also:
Mike_Wilson: pmc_approved+

Attachments
Patch v01 (3.44 KB, patch)
2009-04-09 09:15 EDT, Tomasz Zarna CLA 		no flags Details | Diff
View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Tomasz Zarna CLA 2009-04-09 08:55:59 EDT 
This is one of the issues mentioned on bug 259411. It looks serious enough to make a separate bug out of it and not mix it with the others.

GOALS/BENEFITS:

org.eclipse.compare.contentmergeviewer.TextMergeViewer.createSourceViewer(Composite, int) should return concrete SourceViewer instead of ISourceViewer interface. The ISourceViewer type is not enough for the MergeSourceViewer to work properly, the MergeSourceViewer requires methods which are not part of the ISourceViewer interface. The concrete class of SourceViewer is the optimal return type.

ASSOCIATED BUGS:
https://bugs.eclipse.org/bugs/show_bug.cgi?id=259411

RISKS:
The bad news is that the method is part API, the good news is that is has been introduced in 3.5. So I guess the risk is low, especially given the fact that the API is marked as "experimental" and using it should be consulted with the Workspace team. Impact is limited to JDT which is the only client of the new API so far. Required changes are in the patch.

PERFORMANCE IMPACT:
None.
Comment 1 Tomasz Zarna CLA 2009-04-09 09:15:06 EDT 
Created attachment 131390 [details]
Patch v01
Comment 2 Tomasz Zarna CLA 2009-04-09 10:16:03 EDT 
The solution suggested here could still be a no go to Dani who recommended to use ISourceViewer interface[1]. Before I ask Mike for a PMC approval I would be very happy to hear Dani's opinion about it.

[1] bug 169386, comment 27
Comment 3 Dani Megert CLA 2009-04-14 09:07:58 EDT 
Tomasz I agree. Given that we introduced this in 3.5 and given that it must be at least a SourceViewer we should change this before we ship 3.5. It is very unlikely that someone outside JDT is overriding this method.
Comment 4 Szymon Brandys CLA 2009-04-14 10:15:51 EDT 
Setting the PMC flag.
Comment 5 Szymon Brandys CLA 2009-04-14 10:22:23 EDT 
Tomasz is off today, so to speed it up I sent the request for approval to pmc list too.
Comment 6 Dani Megert CLA 2009-04-14 11:40:37 EDT 
Tomasz, let me know when you're ready to make this change.
Comment 7 Tomasz Zarna CLA 2009-04-15 03:43:03 EDT 
Compare part committed.
Comment 8 Dani Megert CLA 2009-04-15 03:48:39 EDT 
JDT part committed but only changed the code which is related to this bug i.e. discarded all the changes made to the call chain.
Comment 9 Tomasz Zarna CLA 2009-04-15 04:08:45 EDT 
Both parts in HEAD, fix available in builds >N20090414-2200. Thanks Dani!