On Thu, 28 Oct 2021 21:02:04 GMT, Hannes Wallnöfer <[email protected]> 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.
test/langtools/jdk/javadoc/doclet/testSnippetTag/TestSnippetTag.java line 137:
> 135: // new SnippetAttributes("""
> 136: // {@snippet id=foo6:
> 137: // Hello, SnippetAttributes!
Looks like a renaming artefact.
-------------
PR: https://git.openjdk.java.net/jdk/pull/6165
