Thanks Roger! I reckon this is the green light from you, right?
If so, do you want me to add you as "Reviewer"?

Hopefully, we'll figure out how to convey the fact that
there are missing <title>s to the user real soon.

-Pavel

> On 6 Mar 2020, at 19:10, Roger Riggs <[email protected]> wrote:
> 
> Hi Pavel,
> 
> The System Properties page looks better with the links qualified by class or 
> package, or
> the html document title. Developers will want to have a <title> on their 
> document
> so the properties listing is most informative.
> 
> Thanks, Roger
> 
> On 3/6/20 7:51 AM, Pavel Rappo wrote:
>> Hello,
>> 
>> Please review the change for 
>> https://bugs.openjdk.java.net/browse/JDK-8239487
>> :
>> 
>>   
>> http://cr.openjdk.java.net/~prappo/8239487/webrev.00/
>> 
>> 
>> This change addresses 2 closely related issues on the "System Properties 
>> Page":
>> 
>>     8239485: Define behavior of the System Properties page when no system 
>> properties are available
>>     8239487: Better links generation for system properties found in HTML 
>> files
>> 
>> This change does NOT affect the (somewhat related) representation of search
>> results in the Interactive Search or the looks of the Index pages in any way.
>> 
>> The net effect of JDK-8239487 can be seen when comparing the outputs.
>> (For practical reasons the below two links contain only bare-bones versions
>> of the system properties page out of all the documentation.)
>> 
>> Before:
>> 
>>   
>> http://cr.openjdk.java.net/~prappo/8239487/before/system-properties.html
>> 
>> 
>> After:
>> 
>>   
>> http://cr.openjdk.java.net/~prappo/8239487/webrev.00/after/system-properties.html
>> 
>> 
>>   ***
>> 
>> For convenience I split this change into 4 changesets, called phase-{1, 4},
>> which are applied in this particular order.
>> 
>> phase-{1, 2} are intermediate, but somewhat necessary, refactorings.
>> phase-{3, 4} are the core of the change.
>> 
>> * phase-1 splits Category.SEARCH_TAGS into two separate categories, 
>> Category.INDEX
>> and Category.SYSTEM_PROPERTY. The reason is that more often than not, items 
>> of
>> those categories are consumed separately. This also allows to remove the 
>> hacky
>> systemProperty flag from SearchIndexItem.
>> 
>> There are still a few cases left where items of those categories are consumed
>> as if they were of a single category. In these cases careful attention is
>> required. For example, since INDEX and SYSTEM_PROPERTY items are no longer
>> sorted as a whole, a use-site sorting is applied where necessary.
>> 
>> There are changes to make the SearchIndexItems container work with the Stream
>> API rather than with Collections. I'm not 100% sure if the Stream API is the
>> right abstraction here, but for now, it seems to fit better than the 
>> alternatives.
>> That said, in some places Streams API had to be shoehorned (for now) by using
>> stream.iterator() "escape hatch".
>> 
>> Signatures of the query methods in SearchIndexItems have also been fixed to
>> accommodate for the change, described above, in Category. There's a top-level
>> @apiNote in SearchIndexItems describing the details.
>> 
>> On a related note. If that (required, optional...) signature pattern proves
>> useful, we could move SearchIndexItems.concat to Utils for reuse.
>> 
>> * phase-2 is a small cleanup that leverages Stream API from SearchIndexItems
>> where practical.
>> 
>> * phase-3 is the meat of the change. The DocFilesHandlerImpl.getWindowTitle
>> method is extracted and moved to Utils. A String field "referrer" is 
>> introduced
>> in SearchIndexItem to hold a description (title) of a non-program elements, 
>> such
>> as HTML files. The System Properties Page is no longer generated if there 
>> are no
>> system properties. Links to program elements are displayed in a monospace 
>> font,
>> whereas non-program elements are displayed in a proportional font. "Overview"
>> file is treated specially (it's not of kind DOCFILE, neither is it a program
>> element) as it should be displayed in a proportional font.
>> 
>> Some tests are adapted to reflect that the System Properties Page is now
>> generated conditionally. "system-properties.html" has been excluded from the
>> "standardExpectFiles" set in APITest. A single {@systemProperty} tag is 
>> injected
>> into TestMetadata.java. TestSystemPropertyPage is updated. Fixed some typos
>> along the way.
>> 
>> * By phase-4 it became apparent that search item needs to store richer 
>> context
>> than that of provided by a bunch of string fields. So the anemic String 
>> referrer
>> field is swapped for the DocletElement element field. A cache of HTML titles 
>> is
>> moved to the use site. Once again updated the TestSystemPropertyPage test.
>> 
>> Changed presentation in the SystemPropertiesWriter.createLink. Non-program
>> elements are rendered with an extra bit of context: the enclosing package. 
>> If a
>> non-program element has no <title>, the code falls back to the file name. 
>> This
>> is done in a more reliable way than it used to be (the code that relied on
>> "doc-files" substring was pretty brittle).
>> 
>> Notes
>> =====
>> 
>> 1. I must say that it is unsettling to see how much of the presentation 
>> detail
>> spills over into the Java code :( This is something to think about while 
>> working
>> on HTML and CSS.
>> 
>> 2. I do believe that this change addresses most of the concerns raised when 
>> the
>> System Properties page was introduced:
>> 
>>   
>> https://mail.openjdk.java.net/pipermail/javadoc-dev/2019-December/001234.html
>> 
>> 
>> That said, there is a lot more that can be done here. For example, even the
>> current, improved, page might look untidy and cluttered should there appear 
>> system
>> properties linked from both doc files and program elements. Have a look at 
>> the
>> following, contrived, example (this is the actual output of the 
>> TestSystemPropertyPage test):
>> 
>>   
>> http://cr.openjdk.java.net/~prappo/8239487/webrev.00/test-output/system-properties.html
>> 
>> 
>> This suggests some visual improvements are required. One way it could be 
>> done is
>> by using description lists (<dd>) instead of plain comma-separated lists.
>> (A note to self. Should the way <dl>s are rendered depend on the used locale?
>> Are those subject to the same localization concerns as comma-separated 
>> lists?)
>> 
>> -Pavel
>> 
>> 
> 

Reply via email to