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

Doclet does not work with Java 21 LTS #18

Closed
rmuller opened this issue Sep 22, 2023 · 6 comments · Fixed by #19
Closed

Doclet does not work with Java 21 LTS #18

rmuller opened this issue Sep 22, 2023 · 6 comments · Fixed by #19

Comments

@rmuller
Copy link

rmuller commented Sep 22, 2023

As explained here #17 (comment), it is "pure luck" if this doclet will work with Java 21.
I can confirm that it has no luck. All generated HTML is escaped (as also reported for Java 20).

It would be great if LTS releases of Java are supported.

@mnlipp
Copy link
Owner

mnlipp commented Sep 22, 2023

Thanks for the reminder. The Java 21 release escaped my notice, didn't make it to the "headlines" of my usual sources.

I'll have a look into it, most likely next week.

@rmuller
Copy link
Author

rmuller commented Sep 22, 2023

@mnlipp
Copy link
Owner

mnlipp commented Sep 23, 2023

What breaks it:

JDK-17 had this code:

                public Boolean visitText(TextTree node, Content c) {
                    String text = node.getBody();
                    result.add(new RawHtml(textCleanup(text, isLastNode, commentRemoved)));
                    return false;
                }

The text from TextTree.getBody was considered raw HTML (new RawHtml(...)).

JDK-21 has this code:

                public Boolean visitText(TextTree node, Content content) {
                    String text = node.getBody();
                    result.add(text.startsWith("<![CDATA[")
                            ? RawHtml.cdata(text)
                            : Text.of(textCleanup(text, isLastNode, commentRemoved)));
                    return false;
                }

Now, the text from TextTree.getBody is considered Text (Text.of(...)).

Text.write escapes HTML, hence the problem.

This can only be solved by overwriting methods of the HtmlDocletWriter (i.e. deriving an "adapted" version of the class). I tried this before and failed, because -- contrary to the idea of OOP -- the authors had deliberately prevented reuse with this code. However, this ridiculous restriction seems to have gone now, so I'll give it another try.

@mnlipp
Copy link
Owner

mnlipp commented Sep 23, 2023

It turns out that it's not feasible to overwrite methods of HtmlDocletWriter. First, HtmlDocletWriter is a base class, so you have to overwrite the methods in all derived classes. But due to extensive usage of package visibility in the jdk.javadoc code, you have to copy lots of classes into your own package. (To be perfectly honest, the complete jdk.javadoc implementation is crap and violates every rule of good object oriented design. Working with this code has physical effects on my stomach.)

As I see it, there are three ways to go.

  1. Wrap the result from the markup processor in "<![CDATA[...]]>" and unwrap the HTML again in all generated html files. This is ugly.
  2. Copy everything from jdk.javadoc.internal into a package (tree) of your own and make the required changes for processing Markdown. This may become a maintenance nightmare if you want to track bug fixes/enhancements from the original code.
  3. Write a well designed base doclet that allows to plugin "markdown processing" and "HTML processing" (the latter as a proof of concept). This is a major undertaking, of course. (I have always been hoping that the base for this would come from asciidoctor, but they seem to have given up.)

I'll postpone this issue. I'll keep my projects Java-17 compliant anyway until the next LTS after Java-21 comes out. Maybe the "/**md ... */" becomes reality in the meantime. I'll give proposal 2 a try with some very limited time during the next few weeks (mainly because I want to "document" what I've found out about so far). Maybe it works out easier than I think. But else, I won't continue to work on this issue unless a good reason comes up.

mnlipp added a commit that referenced this issue Sep 24, 2023
@rmuller
Copy link
Author

rmuller commented Sep 25, 2023

Ah, i understand. It is a pity.

Nevertheless, thanks for the research, much appreciated!

@mnlipp
Copy link
Owner

mnlipp commented Sep 25, 2023

Couldn't resist the challenge and turned out to be easier than anticipated.

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.

2 participants