On Wed, 16 Mar 2022 16:59:22 GMT, Jonathan Gibbons <j...@openjdk.org> wrote:
>> This PR started in a draft mode as a refactoring effort. After a few commits >> and chats with Jonathan Gibbons, I decided that further refactoring belongs >> to follow-up PRs and that this PR could be marked as "Ready for review". >> >> The motivation for the initial refactoring effort was as follows. The word >> "tree" is heavily overloaded in the JavaDoc codebase, both in comments and >> code. Here are some of the contexts this word appears in: >> >> * Hierarchy Tree (i.e. overview-tree.html and package-tree.html) >> * DocTree (AST nodes) >> * ClassTree, *TreeWriter (data structures and entities related to >> supertype-subtype relationship) >> * HtmlTree (HTML nodes) and specifically UL/OL elements which are nested >> lists >> >> Sometimes contexts overlap, making the word "tree" ambiguous. To reduce this >> ambiguity, the word "tree" should be dropped in favor of a more specific >> word of phrase where possible. >> >> In the case where the context is >> `jdk.javadoc.internal.doclets.toolkit.Content`, the programmer is already >> aware that they are dealing with trees in the sense that `Content` objects >> support recursive composition. There's no need to have the phrase "content >> tree" where "content" would do. Moreover, in the context that is exclusively >> about `Content` objects, the word "content" can be dropped too, since the >> type information is assumed. >> >> As an example of content overlap, have a look at the source of >> `jdk.javadoc.internal.doclets.toolkit.builders.MemberSummaryBuilder::buildSummary`. >> This method used to needlessly mix DocTree with Content tree. > > src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/AbstractOverviewIndexWriter.java > line 70: > >> 68: addConfigurationTitle(target); >> 69: addOverviewComment(target); >> 70: addOverviewTags(target); > > Interesting. The name `main` was used to evoke the intent that the argument > was an HTML `<main>` element. > > Given this method does not override anything in `toolkit`, I think you could > reasonably change the parameter type to `HtmlTree` and even (*cough*) add an > assert that the argument has a `Tag.Name.MAIN`. On the other hand, that would > be inconsistent with other targets. I could revert it if you'd like. While refactoring, I followed a principle that if a method is specific, then its arguments should hint on that specificity. Conversely, if a method could be used in more than a single context, then its arguments should have general names. For example, the below methods should only be passed the HEAD HTML element, so their respective `HtmlTree` arguments are called "head" rather than "target": * jdk.javadoc.internal.doclets.formats.html.markup.Head#addStylesheets * jdk.javadoc.internal.doclets.formats.html.markup.Head#addStylesheet * jdk.javadoc.internal.doclets.formats.html.markup.Head#addScripts So it's not that I don't agree with you, it's that sometimes I miss stuff. Which is not that weird given that plethora of tiny, incohesive methods that plague writers. Since we're here, the way builders and writers work together to assemble an HTML tree is strange [^1]. Although I think that it is a result of evolution, we have to fix it. I'm not ready to have this conversation right now, as I think this PR is already doing way too much beyond its original intent. [^1]: I hope to investigate alternatives in https://bugs.openjdk.java.net/browse/JDK-8283576. > src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/formats/html/ClassWriterImpl.java > line 222: > >> 220: >> 221: /** >> 222: * Get the class inheritance tree for the given class. > > * Gets > > "class inheritance" suggests superclass, which is not a tree. Depending on > functional semantics[1], maybe just "class hierarchy", since superclasses do > not form a tree and interfaces form an acyclic graph. > > [1] impl looks like it is just going up the superclass hierarchy I picked "inheritance" over "hierarchy" for consistency, because the relevant entities in HTML and CSS referred to "inheritance"; for example: * jdk/javadoc/internal/doclets/formats/html/resources/standard.properties:164 * jdk/javadoc/internal/doclets/formats/html/markup/HtmlStyle.java:934 On the other hand, "hierarchy" was also a good choice because it could be used as a standalone noun, thus eliminating the need for the word "tree". Thoughts? ------------- PR: https://git.openjdk.java.net/jdk/pull/7843
