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

Allow multiple format and themes in the Config Doc generator #42907

Merged
merged 3 commits into from
Sep 11, 2024

Conversation

gsmet
Copy link
Member

@gsmet gsmet commented Aug 30, 2024

The idea is to allow generating Markdown for easily publishing the config on GitHub.

The Javadoc -> Markdown part will probably change a lot soon as we need to support Markdown for the IDEs.

This is the fruit of a collaboration with @Athou (Jérémie Panzer), who is marked as a co-author.

It also provides the foundation of Markdown support that we will need for the IDE work (it's still partial but I'd like to get it in so that I can rebase the IDE PR on it).

@quarkus-bot quarkus-bot bot added area/core area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins area/documentation labels Aug 30, 2024
Copy link

github-actions bot commented Aug 30, 2024

🙈 The PR is closed and the preview is expired.

@gsmet
Copy link
Member Author

gsmet commented Aug 30, 2024

Ideally, inject the generated in GitHub somewhere to test how it looks. I'm interested in a link once you have a good enough result. Thanks!

@Athou
Copy link
Contributor

Athou commented Aug 30, 2024

Perfect, I think I have everything I need to get started.

@Athou
Copy link
Contributor

Athou commented Aug 30, 2024

I tried to push on your branch but I'm not allowed to, maybe I didn't understand what you meant:

remote: Permission to gsmet/quarkus.git denied to Athou.

Here's what I got at the moment, I'll work on it some more later: https://gist.github.com/Athou/38211a5a6600e97f3daa8fd909dbd99f

@Athou
Copy link
Contributor

Athou commented Aug 30, 2024

@gsmet I think I'm almost done. I have a couple of questions:

  • What do I need to put in my @ConfigMapping class to trigger a config section? At the moment I never go through the configSection.qute template
  • When is the allConfig.qute template invoked? I don't see any output of it when using the plugin in my app

Also, I'm still unable to push to your branch.
I updated the gist above with the output of the latest template modifications.

Thanks!

@gsmet
Copy link
Member Author

gsmet commented Aug 31, 2024

@Athou looks good!

So for the section, what you need is a nested @ConfigGroup with a @ConfigDocSection annotation on it (and then some properties in the config group). The javadoc in of the section attribute will be the section title.

We have an issue with the constraints, I need to check if it's you having a problem or me.

As for pushing to my branch, you can't. What you can do is to create a PR for my fork here: https://github.com/gsmet/quarkus on the branch output-markdown. (Yes I know, I have a lot of branches :))

@gsmet gsmet force-pushed the output-markdown branch 2 times, most recently from b038bd0 to ac75ce9 Compare September 10, 2024 15:32
@quarkus-bot quarkus-bot bot added the area/docstyle issues related for manual docstyle review label Sep 10, 2024
@gsmet gsmet marked this pull request as ready for review September 10, 2024 15:33
@gsmet
Copy link
Member Author

gsmet commented Sep 10, 2024

@Athou I rebased this work and fixed a few things here and there. I tested it with your Commafeed project and it looks good out of the box.

@gsmet gsmet requested a review from gastaldi September 10, 2024 15:35
@quarkus-bot

This comment has been minimized.

@quarkus-bot

This comment has been minimized.

@gsmet
Copy link
Member Author

gsmet commented Sep 10, 2024

@ppalaga this PR will change quite a few things as the Javadoc won't be transformed in Asciidoc at the source anymore.

You will have to call:

String description = JavadocTransformer.transform(javadocElement.get().description(), javadocElement.get().format(),
                JavadocFormat.ASCIIDOC);

to transform the raw Javadoc to Asciidoc in your plugin.

@quarkus-bot

This comment has been minimized.

@gastaldi gastaldi added the triage/waiting-for-ci Ready to merge when CI successfully finishes label Sep 10, 2024
gsmet and others added 3 commits September 10, 2024 22:21
The idea is to allow generating Markdown for easily publishing the
config on GitHub.

The Javadoc -> Markdown part will probably change a lot soon as we need
to support Markdown for the IDEs.

This is the fruit of a collaboration with @Athou (Jérémie Panzer), who
is marked as a co-author.

Co-authored-by: Jérémie Panzer <jeremiepanzer@gmail.com>
In the end, let's keep it around but handles it properly in the
generated config doc.
For IDEs, we will need to be able to convert our javadoc to Markdown.
And obviously, transforming our javadoc to Asciidoc right in the
generated YAML files is a bad idea.
We now pass the raw Javadoc and it is going to be transformed at a later
stage.
@quarkus-bot
Copy link

quarkus-bot bot commented Sep 10, 2024

Status for workflow Quarkus Documentation CI

This is the status report for running Quarkus Documentation CI on commit 3cfa644.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

Warning

There are other workflow runs running, you probably need to wait for their status before merging.

@maxandersen
Copy link
Member

Makes sense. Just a clarifying quesrion:

Different Formats I grok in sense we store one uniform and then convert to target for the specific output (asciidoc, metadata, etc).

Where does themes come in? Didn't grok that part ? Is that just the different outputs you call "theme" ?

@quarkus-bot
Copy link

quarkus-bot bot commented Sep 11, 2024

Status for workflow Quarkus CI

This is the status report for running Quarkus CI on commit 3cfa644.

✅ The latest workflow run for the pull request has completed successfully.

It should be safe to merge provided you have a look at the other checks in the summary.

You can consult the Develocity build scans.


Flaky tests - Develocity

⚙️ JVM Tests - JDK 21

📦 integration-tests/virtual-threads/grpc-virtual-threads

io.quarkus.grpc.example.streaming.VertxVirtualThreadTest.testStreamingOutputCall - History

  • INTERNAL: Half-closed without a request - io.grpc.StatusRuntimeException
io.grpc.StatusRuntimeException: INTERNAL: Half-closed without a request
	at io.grpc.Status.asRuntimeException(Status.java:533)
	at io.grpc.stub.ClientCalls$BlockingResponseStream.hasNext(ClientCalls.java:631)
	at java.base/java.util.Iterator.forEachRemaining(Iterator.java:132)
	at io.quarkus.grpc.example.streaming.VirtualThreadTestBase.testStreamingOutputCall(VirtualThreadTestBase.java:43)
	at java.base/java.lang.reflect.Method.invoke(Method.java:580)
	at io.quarkus.test.junit.QuarkusTestExtension.runExtensionMethod(QuarkusTestExtension.java:971)
	at io.quarkus.test.junit.QuarkusTestExtension.interceptTestMethod(QuarkusTestExtension.java:821)

@gsmet gsmet merged commit f9530d6 into quarkusio:main Sep 11, 2024
55 checks passed
@quarkus-bot quarkus-bot bot added this to the 3.16 - main milestone Sep 11, 2024
@quarkus-bot quarkus-bot bot removed the triage/waiting-for-ci Ready to merge when CI successfully finishes label Sep 11, 2024
@gsmet
Copy link
Member Author

gsmet commented Sep 11, 2024

The theme might not be useful in the end but it's something we discussed with Peter at some point so that we would have the CXF "theme" in Quarkus itself.
And I thought again about that because there are several flavors of Markdown (typically GitHub supports things that others don't) so we might have to have some tweaks for various flavors.

I wouldn't have done it if I wasn't already adjusting this and thus it came basically for free. We don't use it atm.

@maxandersen
Copy link
Member

Cool. Was just curious what/where it fits.

@gsmet
Copy link
Member Author

gsmet commented Sep 11, 2024

@Athou I thought about this a bit more and I wonder if we should switch to HTML tables to render the config tables.

My main worry is if some code is added to the Javadoc for instance. But I think it could easily be broken in all sorts of ways as Markdown tables are extremely basic.
Also we have to prepare for config actually being Markdown as Java might allow that at some point and people might more easily include complex Markdown in the config doc.

See https://stackoverflow.com/questions/28508141/code-block-inside-table-row-in-markdown .

Comment on lines +33 to +34
// it's Asciidoc, the fun begins...
return "";
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What are your plans here, @gsmet? Are you going to implement AsciiDoc -> Markdown transformation?

I migrated all QCXF to @asciidoclet because the JavaDoc -> AsciiDoc transformation did not work well in some edge cases.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that's the plan.

I don't expect it to be perfect as I won't implement something heavy (I would like to avoid including AsciiDoctorJ + JRuby if we can) but hope it's going to be good enough for the IDEs to be able to consume it.
And we will improve over time when edge cases pop up.

If you have the Javadoc edge cases still around, I'm interested in making it better (if possible). It will never be perfect but it would be nice for it to be good enough in most cases.

But in any case, AsciiDoc will be supported if that's what worried you.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The IDE work is slowly being put in shape here: #42677 .

I need to rebase and wire the Markdown transformer and... then... implement it somehow :).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's this thing in Javascript that looks interesting and matches what I would like to achieve: https://github.com/opendevise/downdoc .

But I'm not sure how easy it would be to convert it in Java. The code is not exactly readable :).

Not sure if AI could help translating downdoc to Java.

@Athou
Copy link
Contributor

Athou commented Sep 11, 2024

@Athou I thought about this a bit more and I wonder if we should switch to HTML tables to render the config tables.

My main worry is if some code is added to the Javadoc for instance. But I think it could easily be broken in all sorts of ways as Markdown tables are extremely basic. Also we have to prepare for config actually being Markdown as Java might allow that at some point and people might more easily include complex Markdown in the config doc.

See https://stackoverflow.com/questions/28508141/code-block-inside-table-row-in-markdown .

I agree, those are valid points, switching to HTML tables would indeed be more robust for the future.
I think GFM also supports table headers in the middle of a table, which would be nice since that would solve the issue of tables with different widths.

The templates would also be easier to read and maintain since we would not have to put everything on the same line.

I'll play a bit with HTML tables and open a PR if it goes well.

@gsmet
Copy link
Member Author

gsmet commented Sep 11, 2024

Awesome, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/core area/devtools Issues/PR related to maven, gradle, platform and cli tooling/plugins area/docstyle issues related for manual docstyle review area/documentation release/noteworthy-feature triage/flaky-test
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants