Community
Participate
Working Groups
I'm implementing some kind of viewer and found out that default comparator is sorting case SENSITIVE while JavaDoc says it's case INSENSITIVE: JavaDoc of method int compare(viewer, object, object) form ViewerComparator: * The default implementation of this method is based on * comparing the elements' categories as computed by the <code> category </code> * framework method. Elements within the same category are further * subjected to a case insensitive compare of their label strings, either * as computed by the content viewer's label provider, or their * <code>toString</code> values in other cases. Subclasses may override.</p> but when you'll go deeply to get the source code of the default comparator, which you'll find in Policy.java, there will be: private static Comparator getDefaultComparator(){ return new Comparator(){ /** * Compares string s1 to string s2. * * @param s1 string 1 * @param s2 string 2 * @return Returns an integer value. Value is less than zero if source is * less than target, value is zero if source and target are equal, * value is greater than zero if source is greater than target. * @exception ClassCastException the arguments cannot be cast to Strings. */ public int compare(Object s1, Object s2) { return ((String)s1).compareTo((String)s2); } }; }
(copied from the newsgroup)
Hitesh is now responsible for watching bugs in the [Viewers] component area.
*** Bug 384061 has been marked as a duplicate of this bug. ***
PDE has its own set of comparators to work around this (See ListUtil in PDE or Bug 414172). If the default behaviour was fixed to follow the javadoc, PDE could remove a lot of this code. Policy is an internal class, though I see it is used in the java project wizards, the log view and the patch comparator. Does anyone object to changing the behaviour?
To be clear, this means that anyone using the string comparator from Policy would get a case-insensitive search going forward.
(In reply to comment #5) > To be clear, this means that anyone using the string comparator from Policy > would get a case-insensitive search going forward. Sounds too dangerous as it affects existing clients outside Platform UI.
> Sounds too dangerous as it affects existing clients outside Platform UI. The fix would correct the implementation which seems currently incorrect. AFAIK the contract is that platform implements according to the Javadoc. Suggested fix: https://git.eclipse.org/r/#/c/15087/
(In reply to comment #7) > > Sounds too dangerous as it affects existing clients outside Platform UI. > > The fix would correct the implementation which seems currently incorrect. > AFAIK the contract is that platform implements according to the Javadoc. > Suggested fix: https://git.eclipse.org/r/#/c/15087/ Well, the Javadoc was wrong and is still wrong (even the good old ViewerSorter has the same issue since day one), especially now, that a policy is used. Even with your fix, most people will get the workbench's comparator, which correctly uses a Collator instance (which is case-sensitive for most locales) as it did since day one where we used the ViewerSorter. ==> Update the Javadoc to say that a policy is used and that the workbench uses a Collator instance and don't touch the getDefaultComparator() which was like that for years.
https://git.eclipse.org/r/#/c/16734/ Proposed javadoc update
(In reply to Curtis Windatt from comment #9) > https://git.eclipse.org/r/#/c/16734/ > Proposed javadoc update Released spec update: http://git.eclipse.org/c/platform/eclipse.platform.ui.git/commit/?id=5dbded47a1c2497b55218571ea57ed75454d836f PW
Verified in master.
