Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Children of a non-formatted tag are formatted #138

Closed
jamierocks opened this issue Sep 13, 2019 · 11 comments · Fixed by #139
Closed

Children of a non-formatted tag are formatted #138

jamierocks opened this issue Sep 13, 2019 · 11 comments · Fixed by #139

Comments

@jamierocks
Copy link
Contributor

The pre tag is correctly not formatted, though it's children shouldn't be either, as they will affect the look of the page.

This can be seen by:

        <pre class="prettyprint lang-java">            <span>
                /*
 * Decompiled with CFR 0.141.
 */
package 
            </span>
            <span class="package">
                net.minecraftforge.api.distmarker
            </span>
            <span>
                ;

public enum Dist {
    CLIENT,
    DEDICATED_SERVER;
    

    public boolean isDedicatedServer() {
        return !this.isClient();
    }

    public boolean isClient() {
        return this == CLIENT;
    }
}

            </span>
        </pre>
@tipsy
Copy link
Owner

tipsy commented Sep 13, 2019

Would you like to submit a PR? :)

jamierocks added a commit to jamierocks/j2html that referenced this issue Sep 13, 2019
jamierocks added a commit to jamierocks/j2html that referenced this issue Sep 13, 2019
@jamierocks
Copy link
Contributor Author

👍 See GH-139 :)

jamierocks added a commit to jamierocks/j2html that referenced this issue Sep 13, 2019
tipsy pushed a commit that referenced this issue Sep 13, 2019
@obecker
Copy link
Contributor

obecker commented Apr 18, 2021

@tipsy Hi, since I stumbled over the same issue, would you consider to create a new release?
(and it would be great to also have a public method renderFormatted(Appendable) 😉 )
Thanks a lot!

@tipsy
Copy link
Owner

tipsy commented Apr 18, 2021

Hey! I have to admit I've lost track of the current master branch, I'm not sure if there are breaking changes. You were quite active last year @pointbazaar, do you know?

@obecker
Copy link
Contributor

obecker commented Apr 19, 2021

Yes, there are. I've cloned and installed the library locally and got compile errors with

html().withLang('en').with(
     head(...),
     body(...)
)

The with(...) method is missing.
Actually, I like declaring the attributes before the content (just as in the HTML markup), so please bring it back. 🙂

Moreover, when changing this to

html(
     head(...),
     body(...)
).withLang('en')

then the lang attribute isn't rendered.

@tipsy
Copy link
Owner

tipsy commented Apr 19, 2021

Are you a heavy user of the project @obecker? I need someone to be in charge of what goes into the next release.

@obecker
Copy link
Contributor

obecker commented Apr 19, 2021

Hi @tipsy, I started using j2html one week ago for a project of mine, and I'm still exploring all of its features. So. I'm not the one to decide when the next release is ready. However, I can assist in testing and may also look into the code.
Cheers, Oliver

@tipsy
Copy link
Owner

tipsy commented Apr 19, 2021

Not an ideal fit, no... 😅 I'll try to find someone for that job soon :)

@pointbazaar
Copy link
Contributor

@tipsy sorry for responding so late. changes to the API were absolutely intended by me, as part of making the API more typesafe. The changes would break some small amount of code, but only invalid html , and it would be very easy for
users of the library to adjust their code.

@obecker that the .withLang('en') does not work is absolutely unintended. (at least by my changes)
https://www.w3schools.com/tags/att_global_lang.asp
specifies a global 'lang' attribute, so it should be present on all tags.

the implementation

public T withLang(String lang) { return attr(Attr.LANG, lang); }

is written in the Tag class and if it does not work there is a chance of
all those similar methods around it also being affected.

but basic Tag features like this should have tests if i remember correctly...

As for the next release, it would be great if my changes could make it in there :)
I hope there are no problems with them.

Maybe open source projects depending on j2html could be found, then tried with the new version before making a release.
You see, i have no idea about releases :)

I wish i had more time :(

@obecker
Copy link
Contributor

obecker commented Apr 23, 2021

Maybe this closed issue is not the best place for discussing these other issues. Nevertheless..
The HtmlTag problem stems from the fact that this is apparently a custom implementation that is not complete (most of the tag classes are in j2html.tags.specialized.generated, HtmlTag is the only class with some extra logic in j2html.tags.specialized.manual). It tries to enforce allowing only head and body as children, but in my eyes it is not worth this effort. To keep things simple I would remove this logic (since there is also no similar logic for table, tr or other tags).
Just my 2 cents ...

@tipsy
Copy link
Owner

tipsy commented Apr 23, 2021

No worries @pointbazaar, sorry for forgetting about your changes too :)

@obecker @pointbazaar I've made an issue here #172 to discuss the future of the project, you can move the discussion there. I've found a new maintainer (and another person willing to step up). It would be good if we could all get aligned.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants