Bug 214900 - GEF should be using ICU instead of java.text
Summary: GEF should be using ICU instead of java.text
Status: RESOLVED FIXED
Alias: None
Product: GEF
Classification: Tools
Component: GEF-Legacy GEF (MVC) (show other bugs)
Version: 3.4   Edit
Hardware: PC Windows XP
: P3 normal (vote)
Target Milestone: 3.4.0 (Ganymede) M6   Edit
Assignee: Pierre Carlson CLA
QA Contact:
URL:
Whiteboard:
Keywords: contributed
Depends on: 214406
Blocks:
  Show dependency tree
 
Reported: 2008-01-10 10:22 EST by Pierre Carlson CLA
Modified: 2008-09-18 13:48 EDT (History)
4 users (show)

See Also:

Attachments
Patches org.eclipse.gef and org.eclipse.draw2d to use icu classes (6.28 KB, application/octet-stream)
2008-01-10 10:23 EST, Pierre Carlson CLA 		ahunter.eclipse: iplog+
Details
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 Pierre Carlson CLA 2008-01-10 10:22:04 EST 
GEF should be using ICU instead of java.text per http://wiki.eclipse.org/ICU4J.

org.eclipse.gef is using java.text.MessageFormat and draw2d is using java.text.Bidi.  I'll attach a patch that changes the imports to com.ibm.icu.  The draw2d piece of the patch won't work until Eclipse upgrades to ICU 3.8.  This is supposed to happen in M5.  See bug 214406.
Comment 1 Pierre Carlson CLA 2008-01-10 10:23:00 EST 
Created attachment 86560 [details]
Patches org.eclipse.gef and org.eclipse.draw2d to use icu classes
Comment 2 Anthony Hunter CLA 2008-01-10 10:31:35 EST 
We were supposed to already be ICU4J compliant, must have missed a few places.
Comment 3 Pratik Shah CLA 2008-01-11 03:30:28 EST 
We should be careful here.  It's not always a simple case of changing the import statement.  I know for a fact that the word iterators in JDK and ICU work quite differently, so changing the word iterator in the text example (TextFlowPart) to the ICU4J version introduced bugs in the word navigation behaviour.  Similarly, changing the line iterator in FlowUtilities might have introduced a bug.

Looking at http://wiki.eclipse.org/ICU4J, I don't see Bidi mentioned in there as a class that needs to be changed.  If we do change it, we'd have to do sufficient testing to ensure that it didn't introduce any problems.
Comment 4 Pierre Carlson CLA 2008-01-11 09:34:31 EST 
The change that is important to my product is the Bidi change.  The only usage is the static method requiresBidi() which determines if a string needs to be Bidi processed.  The Bidi class was not added to ICU until 3.8, which Eclipse is just adopting now.  I suspect that is why it is not on the Eclipse/ICU page.  It seems like we should be able to rely on the ICU testing of this single static method.
Comment 5 Pierre Carlson CLA 2008-02-04 12:25:17 EST 
I verified this morning that the attached patch still works against what is in HEAD.  the patch should just need to be reviewed and committed to CVS.
Comment 6 Anthony Hunter CLA 2008-02-08 15:32:58 EST 
Missed M5, so moving to M6
Comment 7 Anthony Hunter CLA 2008-03-26 15:55:20 EDT 
Committed to HEAD,

Also added the explicit bundle version dependency to draw2d and gef.

 com.ibm.icu;bundle-version="[3.8.0,4.0.0)"
Comment 8 Anthony Hunter CLA 2008-03-31 17:33:48 EDT 
(In reply to comment #7)
> Also added the explicit bundle version dependency to draw2d and gef.
> 
>  com.ibm.icu;bundle-version="[3.8.0,4.0.0)"
> 

I guess this was a bad idea. GEF originally used 
Import-Package: com.ibm.icu.text

Given we fixed this bug, I changed this to explicitly require the 3.8.0 or newer bundle, since it depends on the new ICU.

For whatever reason, this change broken the GMF build.

For now I have put the import package back in the Manifest.MF