RFR: 8220497: Improve Javadoc search feature and add test coverage

[Hannes Wallnöfer]

Thanks for the review, Jon. 

New spec/webrev/docs:

Spec: http://cr.openjdk.java.net/~hannesw/8220497/spec.01/
Webrev: http://cr.openjdk.java.net/~hannesw/8220497/webrev.01/
API docs: http://cr.openjdk.java.net/~hannesw/8220497/api.01/


> Am 29.03.2019 um 21:34 schrieb Jonathan Gibbons <jonathan.gibbons at oracle.com>:
> 
> 
> The spec: 
> The overview  is a bit of a mix of spec and impl details.

I scrapped the overview and replaced it with a short rationale of why we need this spec, which is basically as an addendum to JEP 225 that defines the actual search behaviour.


> Under Definitions
> 
> I was surprised to see the new word "entity" until I realized it needs to include both elements and custom searchable items. Maybe       that should be said somewhere.

Added.

> "Upper case" and "lower case" are normally written as single words.
> 

Fixed.

> Are we allowed non-letter characters in a camel-case search?  i.e. should the definition of camel-case include the non-alphabetic characters

I changed the definition of camel-case so it doesn’t sound like it lists all valid types of characters - that’ s what we refer to the JLS for. 

> Should "all-caps" (and "camel-case" include $
> 

I removed all-caps since the underscore as word boundary now applies to all identifiers, see below.

We already do support ‚$‘ in camel-case and partial matches.


> Under Searchable Entities
> 
> typo: entitis
> 
> Under Types
> 
> 
> Change "consist" to "consists"
> 
> Under each of Packages, Types and Members
> 
> 
> The current pattern is "The signature of <plural> consists of the ..."  It would be more correct to say "The signature of a <single-item> consists of …“.
> 

All fixed.


> Under Matching Rules/Left Boundaries:
> 
> 
> 3rd bullet: why restrict letter following "_" to all-caps, and not after "_" in all cases?


Great idea, I made that change and it works well.


> Under Matching Rules/Camel case:
> 
> 
> Wouldn't it be better to allow digits and other non-alpha characters, treating them as the same as lower-case?
> 

Again, tweaked the text to make that clear.


> Under Matching Rules/White space:
> 
> 
> It seems a bug that typing excess whitespace should cause the match to fail. You should ignore all whitespace everywhere in the search string.

I spent quite a bit of time on getting whitespace right. Ignoring all whitespace seems wrong as it would mean that „ob ject“ matches java.lang.Object. The solution I found is to make white space significant if it separates two words/identifiers, and make it optional if it occurs at the edge of the query string or before/after a separator character.

This more or less replicates white space in Java syntax, and it happens to also work well for search tags.

   
> Under Browser Requirements:
> 
> 
> "Requirements" seems like the wrong word. How about "Supported Browsers", with a note that it may work on other browsers as well. Also, you should support Chrome on All OSs that support Chrome.
> 

Changed it to „supported browsers“.

> 
> Code
> 
> I note the watermark is fixed in English. Is there any easy way to make it localizable?
> 

That would have been part of my HTML5 placeholder attribute solution for JDK-8221366. It is still possible as things are, but a bit more cumbersome and I’d prefer to keep that separate from this issue.

> General question, for later: should we publish/use a minified version of search.js?
> 

I don’t think that’s necessary. The file is below 15 kb, so one of the smaller assets we load.

Hannes


> Other functionality questions are implicit in the comments on the spec.
> 
> -- Jon