yupiik / tools-maven-plugin

An Apache Maven plugin set of tools.
https://www.yupiik.io/tools-maven-plugin
Apache License 2.0
13 stars 4 forks source link

Could be great if we have a enum or fields that contains all attributes keys #18

Closed mcruzdev closed 2 months ago

mcruzdev commented 2 months ago

Is your feature request related to a problem? Please describe. A clear and concise description of what the problem is. Ex. I'm always frustrated when [...]

Actually to use AsciidoctorLikeHtmlRenderer:

AsciidoctorLikeHtmlRenderer renderer = new AsciidoctorLikeHtmlRenderer(new AsciidoctorLikeHtmlRenderer.Configuration()
                .setAttributes(Map.of("noheader", "true")));

We need to know the API internally to know what attribute could be used.

Describe the solution you'd like A clear and concise description of what you want to happen.

Add all attribute keys available for Configuration#attributes to be used ( enum or field ) and add documentation about it.

Something like it:

 public enum Attributes {
            NO_HEADER_ATTRIBUTE("noheader"),
            DATA_URI_ATTRIBUTE("data-uri"),
            IMAGES_DIR_ATTRIBUTE("imagesdir");

            private final String attributeName;

            Attributes(String attributeName) {
                this.attributeName = attributeName;
            }

            public String key() {
                return attributeName;
            }
        }
rmannibucau commented 2 months ago

Hi,

Here are some points to keep in mind IMHO - also trying to explain why the original choice was to not go that way at all:

Side note: a final static class with static final constants only can makes as much sense since the enum ordinal will not be an information so maybe something to think about.

Hope it makes sense.

mcruzdev commented 2 months ago

Thank you for the quick response and clarification!

The main idea here is to expose a configuration key for lib users, I think that AsciidoctorLikeHtmlRenderer.Attributesmakes total sense (being a enum or not). I would like to facilitate my side when a upgrade change some attribute key in the future.

mcruzdev commented 2 months ago

I have a doubt, I am working on this pull request to make a PoC with this tooling (thank you for this great job)!

I have a test here:

    @Test
    public void testH1() {
        Engine engine = Engine.builder().addDefaults()
                .addSectionHelper(new AsciidocSectionHelperFactory()).build();

        String result = engine.parse("{#ascii}= Quarkus and Roq{/ascii}").render();

        assertThat(result).contains("<h1>Quarkus and Roq</h1>");
    }

The result is "" but the expected is to have <h1>..., It happens when I add noheader attribute, is it the expected behavior?

I think it should be <h1>... no?

rmannibucau commented 2 months ago

@mcruzdev looks normal since noheader implies notitle which skips the = element (https://github.com/yupiik/tools-maven-plugin/blob/master/asciidoc-java/src/main/java/io/yupiik/asciidoc/renderer/html/AsciidoctorLikeHtmlRenderer.java#L299)

mcruzdev commented 2 months ago

Is there a way to have just <h1>...<h1> and not the complete HTML? <!DOCTYPE ...?

rmannibucau commented 2 months ago

@mcruzdev guess overriding the public void visitHeader(final Header header) { method of the renderer is the fastest adding at the top the line builder.append(" <h1>").append(escape(header.title())).append("</h1>\n"); before delegating to the super impl.

fpapon commented 2 months ago

@mcruzdev thank you for using the project! Is it possible for you to provide a PR for that?

mcruzdev commented 2 months ago

Of course!

Do you mean for the AsciidoctorLikeHtmlRenderer.Attributes comment? or <h1>?

fpapon commented 2 months ago

Of course!

Do you mean for the AsciidoctorLikeHtmlRenderer.Attributes comment? or <h1>?

I mean for the <h1>

mcruzdev commented 2 months ago

Of course!

Do you mean for the AsciidoctorLikeHtmlRenderer.Attributes comment? or <h1>?

I mean for the <h1>

Tomorrow I will work on it!

mcruzdev commented 2 months ago

Thank you @rmannibucau and @fpapon for the helping! I created a new issue to solve the comment above! TY guys!