On 2/4/20 12:16 PM, Pavel Rappo wrote:
On 3 Feb 2020, at 19:58, Jonathan Gibbons <[email protected]> wrote:

Approved, but for bonus points, add a comment to SearchIndexItem.toString
listing all the letters we use and what they are used for.
If it's okay I'd prefer not to do that. That method is pretty self-describing
and is not a part of public API. However, I'd agree that it is not obvious that
its output is not for debugging or otherwise human consumption, but for later
parsing. The should be a separate method, really.
OK, the problem may fix itself anyway, depending on how you address the
other improvements for the .toString() method. The suggestion was just
about making it easy to see what letters are in use, and why, without having
to read all the .toString method.



For further consideration (later?), could we have a worker method that takes a 
varargs list of pairs, perhaps similar to Map.of, or even using Map.of?  Or use 
a builder object with methods named for the single-characters whose arg is the 
value ... although the conditional get in the way of fluent API usage.
This definitely makes sense. I dislike the fact that it is a simple POJO, prone
to all sort of internal conflicts and inconsistencies. Should be addressed in a
follow-up bug.

This should be cleaned up further. Perhaps, an appropriate time would be right
after JDK-8235827: Improve the System Properties page.

Separately, (later?) what if the description contains a quote character ('"') 
... there is no attempt to escape any values.
Thanks! I've filed a bug on that, JDK-8238495, and added a corresponding
TODO comment to the code.

-Pavel

-- Jon


On 01/31/2020 05:47 AM, Pavel Rappo wrote:
Hello,

Please review the change for https://bugs.openjdk.java.net/browse/JDK-8238291:

   http://cr.openjdk.java.net/~prappo/8238291/webrev.00/

This is a tiny cleanup change. The goal is to fix inconsistency in abbreviating
the fields of the index structure.

The fields of the index structure, such as "label", "containingModule",
"containingPackage", "containingClass", "holder", and "description" are
abbreviated in the index files to a single letter, "l", "m", "p", "c", "h", and
"d" respectively. The "url" field is not. Sometimes it is referred to as "u",
other times as "url".

 From my archival research it seems that this peculiarity has been there from 
day 1.
While "url" was used for indexing packages, types, members, and then modules,
"u" was used for everything else, which back then comprised the "search tags"
category. I couldn't find any evidence of that being done for some purpose.
I do believe it's just a typo, thus I propose to fix it.

Incidentally, this change reduces the size of the index files. In case you
wonder the reduction is 1% (one percent), otherwise known as "not something to
brag about".

Thanks,
-Pavel

Reply via email to