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

[Hannes Wallnöfer]

On Thu, 28 Oct 2021 21:49:46 GMT, Pavel Rappo <prappo 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.
>
> src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/taglets/SnippetTaglet.java line 336:
> 
>> 334:             case ".scala", ".sc"                     -> "scala";
>> 335:             case ".sh"                               -> "bash";
>> 336:             case ".tex"                              -> "latex";
> 
> My initial reaction to this is that we should not bake this list in its current form in javadoc. I might be okay with just two cases: `.java` and `.properties`. Perhaps we could provide a way for users to configure any mappings beyond those two externally.

Remember it's just a convenience/default mapping to create a language attribute without explicitly setting it. File extensions do not change over night, so I think the only risk here is missing out on some newly popular language or carrying some obsolete language that nobody will use (well I guess latex may fall in that category :)

I thought about (adding a comment about) making the mapping configurable, but I think any such mechanism would be much more complicated than just adding a "lang" attribute to your snippets.

I also thought about keeping the list minimal, but then it's just a little nod at anyone who may be using one of these languages.

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

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