When searching for „jav“, Search Tags contains „Java Collections Framework“ followed by various java* command line tools. When using collation, the first item is „javac“, followed by „Java Collections Framework“ and then the remaining java* tools.
It just looks strange and it causes tests to fail, so I opted for keeping search tag order as it is. Hannes > Am 27.06.2018 um 18:05 schrieb Jonathan Gibbons <[email protected]>: > > Hannes, > > Can you explain more what you mean by ... > > "except search tags, where collation causes frameworks and commands to be > mixed up and tests to fail." > > -- Jon > > > On 6/27/18 8:57 AM, Hannes Wallnöfer wrote: >> Updated webrev with comparators moved to Utils. All index items use >> collation for primary comparison except search tags, where collation causes >> frameworks and commands to be mixed up and tests to fail. Comparators for >> index collections are created in HtmlConfiguration.initConfiguration, I >> think Locale should be set by then. >> >> http://cr.openjdk.java.net/~hannesw/8190876/webrev.02/ >> >> Thanks, >> Hannes >> >> >>> Am 22.06.2018 um 23:22 schrieb Jonathan Gibbons >>> <[email protected]>: >>> >>> While I agree that Utils is not a great place for the comparators, I will >>> note that the comparison routines there are better than the "simple" ones >>> you have placed in SearchIndexItem. In particular, the existing comparison >>> code in Utils uses java.text.Collator to provide locale-sensitive >>> comparison, which is conceptually more correct than your simple comparison >>> of the upper-cased strings. >>> >>> -- Jon >>> >>> >>> On 06/19/2018 03:03 AM, Hannes Wallnöfer wrote: >>>> Thanks for the review, Jon. >>>> >>>> I’ve uploaded a new webrev to address the issues you brought up: >>>> >>>> http://cr.openjdk.java.net/~hannesw/8190876/webrev.01/ >>>> >>>> Moving the index item comparators up to Utils didn’t feel right, so I made >>>> them static and moved them to SearchIndexItem.java, which feels more >>>> appropriate to me. The generic one that is used multiple times is now a >>>> singleton. >>>> >>>> I also forgot to update SearchTest.java for the changes in search.js file, >>>> that is included in the new webrev. >>>> >>>> Hannes >>>> >>>> >>>>> Am 18.06.2018 um 20:23 schrieb Jonathan Gibbons >>>>> <[email protected]>: >>>>> >>>>> Minor issues to address: >>>>> >>>>> HtmlConfiguration, 397, 406: >>>>> You're using .toUpperCase() which depends on the Locale. The convention >>>>> in javadoc is to use Utils.to{Lower,Upper}Case(String), which forces the >>>>> en-US locale (to avoid the Turkish-i problem). There is an equivalent >>>>> convention in javac as well. >>>>> >>>>> SearchIndexItem >>>>> If I understand the code correctly, "NestedName" is not the correct term >>>>> to be using. I think you're trying to get the "simple name". Nesting is >>>>> a different concept, as in, "nested classes". In a better/future world, >>>>> SearchIndexItem should contain an Element (not should not always be only >>>>> string based) and once you have an Element, you can easily get the simple >>>>> name. >>>>> >>>>> Style issue: >>>>> >>>>> Although not wrong, it seems less than ideal to have functions creating >>>>> and returning equal instances of the comparators, as compared to having >>>>> singleton instances stored as needed. But then, it's also weird to have >>>>> these search indexes stored in HtmlConfiguration, as compared to a >>>>> search-related class. In addition, Utils has many comparators, so you >>>>> arguably should not be adding more comparators here in HtmlConfiguration. >>>>> (Not that I like the overuse of the Utils bucket.) >>>>> >>>>> -- Jon >>>>> >>>>> >>>>> On 06/15/2018 12:43 AM, Hannes Wallnöfer wrote: >>>>>> This changes sorting order of packages and modules in the search box >>>>>> from last name segment to whole package or module name, respectively. >>>>>> Apart from fixing the observed issue that leads to more intuitive >>>>>> listings as package and module names are hierarchic by nature. >>>>>> >>>>>> The sorting order for types, members, and search tags is not changed. >>>>>> >>>>>> The patch also moves sorting from client side JavaScript to Java, >>>>>> speeding up rendering of search results by at over 2x. It also provides >>>>>> the benefit of secondary order, so members and types with the same name >>>>>> and signature are now ordered by package name, whereas their order was >>>>>> undefined before. >>>>>> >>>>>> Please review: >>>>>> >>>>>> Webrev: cr.openjdk.java.net/~hannesw/8190876/webrev.00/ >>>>>> Issue: https://bugs.openjdk.java.net/browse/JDK-8190876 >>>>>> >>>>>> Thanks, >>>>>> Hannes >
