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 <[email protected]>: > > > 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
