RFR: 8276124: Provide snippet support for properties files

[Jonathan Gibbons]

On Mon, 15 Nov 2021 20:34:20 GMT, Pavel Rappo <prappo at openjdk.org> wrote:

> The initial integration (JDK-8266666) of JEP 413 did not support properties files. This commit rights that wrong.

I think you can simplify the code by reducing `CoarseParser` down to a language-specific regular expression with "standard" named groups for the payload and markup.

I'd also recommend not using an `enum` for `Language` to leave open the possibility of easily/dynamically adding support for additional languages.

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/taglets/SnippetTaglet.java line 67:

> 65: public class SnippetTaglet extends BaseTaglet {
> 66: 
> 67:     public enum Language {

The idea of a `Language` object is good, but using an `enum` precludes easy extensibility.

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/taglets/snippet/Parser.java line 87:

> 85:         int payloadEnd();
> 86:         int markupStart();
> 87:     }

I'm surprised/disappointed that this may be necessary, although I accept there is a notable difference between "whole-line comments" and "end-of-line comments".  Is it possible to reduce `CoarseParser` down to a language-specific regular expression with named groups for the payload and markup, such that the regular expression could be  a property of the `Language` object?

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/taglets/snippet/Parser.java line 93:

> 91: 
> 92:         @Override
> 93:         public boolean parse(String line) {return m.reset(line).matches();}

Given that matchers are lightweight objects, what is the benefit of using a pre-allocated matcher and `reset` as compared to a temporary `Matcher` object?  Is it just style, or is there something more?

src/jdk.javadoc/share/classes/jdk/javadoc/internal/doclets/toolkit/taglets/snippet/Parser.java line 99:

> 97: 
> 98:         @Override
> 99:         public int markupStart() {return m.start(3);}

Suggest using named groups

test/langtools/jdk/javadoc/doclet/testSnippetTag/TestLangProperties.java line 44:

> 42:  * @build javadoc.tester.* toolbox.ToolBox toolbox.ModuleBuilder builder.ClassBuilder
> 43:  * @run main TestLangProperties
> 44:  */

move before imports

test/langtools/jdk/javadoc/doclet/testSnippetTag/TestLangProperties.java line 47:

> 45: public class TestLangProperties extends SnippetTester {
> 46: 
> 47:     private final ToolBox tb = new ToolBox();

Suggest moving to SnippetTester (suggested in dependent PR)

test/langtools/jdk/javadoc/doclet/testSnippetTag/TestLangProperties.java line 49:

> 47:     private final ToolBox tb = new ToolBox();
> 48: 
> 49:     private TestLangProperties() {}

Just curious: why force this?

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

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