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.

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

Commit messages:
 - 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=00
  Issue: https://bugs.openjdk.java.net/browse/JDK-8275788
  Stats: 277 lines in 4 files changed: 148 ins; 10 del; 119 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

Reply via email to