RFR: JDK-8275788: Create code element with suitable attributes for code snippets [v4]
Hannes Wallnöfer
hannesw at openjdk.java.net
Tue Nov 2 18:35:27 UTC 2021
> Please review a change to add a nested `<code>` element for snippet tags as well as HTML attributes for the snippet's `id` and `lang` attributes. The change is quite simple, I did however encounter some gotchas. Below is a short list of notes to explain some aspects of the change.
>
> - Both `id` and `lang` are only meaningful if they have non-empty values, so defining them with empty or missing values will cause the respective HTML attributes to be not defined.
> - I decided to put the logic for extracting the `id` and `lang` attributes into the already quite long `SnippetTaglet.getInlineTagOutput` method as this is where the required preparation and infrastructure are located. Both values are then passed as additional arguments to `TagletWriter.snippetTagOutput` which is not great. I think it's ok for now, but maybe if we need to pass more values along that way we should reconsider this design.
> - I'm not sure whether we should assume `lang=java` for internal snippets. This change currently does not, it only derives implicit `lang` attributes for external snippets based on the extension of the snippet source file.
> - In `SnippetTaglet.languageFromFilename()` I tried to cover what I reckoned would be the most useful languages for javadoc users, including a few major JVM languages. I'm sure I forgot some, but I hope I got at least those file extensions right.
> - We previously added a newline character to the snippet's `<pre>` element. I think this was done in order to make sure the element is rendered even if the snippet content is empty. This was fine with just the `<pre>` element, but with a nested `<code>` element that newline is visible in the browser. I therefore replaced the newline with `HtmlTree.EMPTY`.
> - For the test, I was able to convert the existing `testIdAndLangAttributes` test to cover explicit `id` and `lang` attributes. I added a new test called `testExternalImplicitAttributes` to test derived `lang` attributes.
Hannes Wallnöfer has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains six commits:
- Minimize list of recognized file extensions
- Keep list of recognized file extensions to a minimum
- Merge branch 'master' into JDK-8275788
- Avoid use of String.toLowerCase() without locale and reverse renaming artefact
- JDK-8275788: Remove trailing whitespace
- JDK-8275788: Create code element with suitable attributes for code snippets
-------------
Changes: https://git.openjdk.java.net/jdk/pull/6165/files
Webrev: https://webrevs.openjdk.java.net/?repo=jdk&pr=6165&range=03
Stats: 274 lines in 4 files changed: 133 ins; 13 del; 128 mod
Patch: https://git.openjdk.java.net/jdk/pull/6165.diff
Fetch: git fetch https://git.openjdk.java.net/jdk pull/6165/head:pull/6165
PR: https://git.openjdk.java.net/jdk/pull/6165
More information about the javadoc-dev
mailing list