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

Pavel Rappo prappo at openjdk.java.net
Wed Nov 3 15:27:16 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. The pull request contains one new commit since the last revision:
> 
>   Minimize list of recognized file extensions

> Thanks for the pointers, I didn't know this although I realized there had to be special rules with leading newlines in `<pre>` content. The rule does not apply to the content of a `<code>` element inside a `<pre>`. We could keep the newline before the `<code>` element, but it is no longer needed so I think it can be removed.

Given the HTML representation of `@snippet` that this PR proposes, we should not keep that leading newline. Here's why. If we push it into `<code>`, this newline will become significant and as such will be visible in the browser:

    <pre><code>
        content line 1
        content line 2
        ...</code></pre>
```        
If we leave it as is, the HTML will no longer resemble snippet indentation because the content will be misaligned: the first content line will be shifted 6 positions to the right (the length of the `<code>` string):

    <pre>
        <code>content line 1
        content line 2
        ...</code></pre>

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

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


More information about the javadoc-dev mailing list