RFR: JDK-8275788: Create code element with suitable attributes for code snippets [v5]

Hannes Wallnöfer hannesw at openjdk.java.net
Tue Nov 2 20:10:14 UTC 2021


On Tue, 2 Nov 2021 18:54:48 GMT, Hannes Wallnöfer <hannesw at openjdk.org> wrote:

>> 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 refreshed the contents of this pull request, and previous commits have been removed. The incremental views will show differences compared to the previous content of the PR.

I have reduced the list of recognized file extensions to the bare minimum for now - just .java and .properties. (Accidentally this is also the set of file extensions covered by the test.) Whether or how this list should be made extensible should be discussed separately as it is not an essential part of this issue.

-------------

PR: https://git.openjdk.java.net/jdk/pull/6165


More information about the javadoc-dev mailing list