Re: RFR: 8307652: sealed class hierarchy graph doesn't distinguish non-sealed classes [v2]
On Tue, 16 May 2023 17:16:57 GMT, Chen Liang wrote: >> `@sealedGraph` had a mechanism to render non-sealed classes differently, but >> it's useless because the graph nodes are not bordered. This patch converts >> the non-sealed classes to be rendered in italics instead. >> >> An example of `ConstantDesc`, which has a sealed hierarchy except >> `DynamicConstantDesc`: >> JDK 20: >>  >> >> This patch: >>  > > Chen Liang has updated the pull request with a new target base due to a merge > or a rebase. The incremental webrev excludes the unrelated changes brought in > by the merge/rebase. The pull request contains three additional commits since > the last revision: > > - Use an edge and node to indicate non-sealed hierarchy > - Merge branch 'master' into fix/sealedgraph-nonsealed > - 8307652: sealed class hierarchy graph doesn't distinguish non-sealed > classes These changes look very good. A significant improvement to the graph rendering. - Marked as reviewed by pminborg (Committer). PR Review: https://git.openjdk.org/jdk/pull/13877#pullrequestreview-1429913989
Re: RFR: 8307652: sealed class hierarchy graph doesn't distinguish non-sealed classes
On Mon, 15 May 2023 06:46:28 GMT, Per Minborg wrote:
>> `@sealedGraph` had a mechanism to render non-sealed classes differently, but
>> it's useless because the graph nodes are not bordered. This patch converts
>> the non-sealed classes to be rendered in italics instead.
>>
>> An example of `ConstantDesc`, which has a sealed hierarchy except
>> `DynamicConstantDesc`:
>> JDK 20:
>> 
>>
>> This patch:
>> 
>
> Thanks for this improvement suggestion. Indicating "openness" is certainly
> important. I was playing around with various ways of expressing it and came
> up with this:
>
> 
>
> What is your opinion on it? It might be slightly more intuitive? We could
> even do this:
>
> 
>
>
>
> Here is the code:
>
>
>
> digraph G {
>
> shape="none"
> rankdir = "BT"
>
> ClassDesc -> ConstantDesc
> MethodHandleDesc -> ConstantDesc
> DirectMethodHandleDesc -> MethodHandleDesc
> DynamicConstantDesc -> ConstantDesc
> Float -> ConstantDesc
> Hidden1 -> DynamicConstantDesc [style="dashed"]
>
> Hidden1 [label=""]
>
> ClassDesc [shape=none];
> ConstantDesc [shape=none];
> MethodHandleDesc [shape=none];
> DynamicConstantDesc [shape=none];
> DirectMethodHandleDesc [shape=none];
> Float [shape=none];
> Hidden1 [shape=none];
> }
@minborg Done. I've made the `` in italics and its hover label is
"Non-sealed Hierarchy". Example images of ConstantDesc and CallSite:
In addition, the copyright year is updated to reflect the correct creation date
of this taglet (2022 instead of 2017)
-
PR Comment: https://git.openjdk.org/jdk/pull/13877#issuecomment-1550071543
Re: RFR: 8307652: sealed class hierarchy graph doesn't distinguish non-sealed classes [v2]
> `@sealedGraph` had a mechanism to render non-sealed classes differently, but > it's useless because the graph nodes are not bordered. This patch converts > the non-sealed classes to be rendered in italics instead. > > An example of `ConstantDesc`, which has a sealed hierarchy except > `DynamicConstantDesc`: > JDK 20: >  > > This patch: >  Chen Liang has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains three additional commits since the last revision: - Use an edge and node to indicate non-sealed hierarchy - Merge branch 'master' into fix/sealedgraph-nonsealed - 8307652: sealed class hierarchy graph doesn't distinguish non-sealed classes - Changes: - all: https://git.openjdk.org/jdk/pull/13877/files - new: https://git.openjdk.org/jdk/pull/13877/files/cab36e65..bb2d215c Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=13877&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=13877&range=00-01 Stats: 99505 lines in 1624 files changed: 79242 ins; 8145 del; 12118 mod Patch: https://git.openjdk.org/jdk/pull/13877.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/13877/head:pull/13877 PR: https://git.openjdk.org/jdk/pull/13877
RFR: JDK-8305710: Line breaks in search tags cause invalid JSON in index file
This fixes a number of issues that can or could result in invalid JavaScript search index files, and a single invalid index file breaking the whole JavaDoc search feature. - The `IndexItem` now throws `IllegalArgumentException` if the label argument contains a line break. This was the original cause for this bug and was already fixed in [JDK-8305407](https://bugs.openjdk.org/browse/JDK-8305407), but it is a good idea to add the explicit check to fail in a visible way should it occur again. - The `IndexItem.escapeQuotes(String)` method now escapes backslashes in addition to quote. Failure to escape a trailing backslash would result in an escaped quote, leading to an invalid index file. - The search script gets a check for null/undefined to avoid a missing tag index file to break the search feature as a whole. Note that the added test covers only the second item in the list above. The other two changes are trivial and would require undue effort to test. - Commit messages: - JDK-8305710: Line breaks in search tags cause invalid JSON in index file Changes: https://git.openjdk.org/jdk/pull/14016/files Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=14016&range=00 Issue: https://bugs.openjdk.org/browse/JDK-8305710 Stats: 75 lines in 4 files changed: 45 ins; 6 del; 24 mod Patch: https://git.openjdk.org/jdk/pull/14016.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/14016/head:pull/14016 PR: https://git.openjdk.org/jdk/pull/14016
