search

eclipse-jdt / eclipse.jdt.debug

Eclipse Public License 2.0
16 stars 50 forks source link

Fixes #290 - complete the removal of com.ibm.icu dependency #291

Closed robstryker closed 1 year ago

robstryker commented 1 year ago

What it does

Completes the removal of dependency on com.ibm.icu

How to test

I am not sure how to test this, however, reading the code for java.text and com.ibm.icu.text shows that getLevelAt should be a reasonable replacement for getLevels().

Author checklist

rgrunber commented 1 year ago

CC'ing @akurtakov as the comment on https://bugs.eclipse.org/bugs/show_bug.cgi?id=561910#c9 seemed to imply maybe this wasn't possible (did getLevelAt not exist then?). Looking at it the change seems very reasonable.

akurtakov commented 1 year ago

I think I didn't manage to find it out :) . Back then so many changes went in and in so many places that I have probably overlooked this and probably a good number of other similar cases. I'm pretty sure that a second look will manage to complete far more from the conversion.

robstryker commented 1 year ago

I've triggered the bidi code path by modifying java code templates (in window -> preferences) to display the JavaSourceViewer, and then pasting hebrew or arabic text inside.

I haven't seen any exceptions yet, so I'm going to assume this works as expected.

robstryker commented 1 year ago

@robstryker can you just confirm how you tested with the code templates, because I didn't see how that code path would trigger the JDISourceViewer.

I'm sorry if was less than accurate. What I meant is that I tested the same logic that is in this patch by way of another patch on jdt.ui. In jdt.ui, I tested via the JavaSourceViewer viewer using the method described.

I did not know how to trigger the JDISourceViewer in this plugin so I tried to at least verify that the logic itself was sound by testing it in the other repository.