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

Missing breadcrumb name in maven site module #763

Closed
1 of 3 tasks
abelsromero opened this issue Feb 4, 2024 · 8 comments
Closed
1 of 3 tasks

Missing breadcrumb name in maven site module #763

abelsromero opened this issue Feb 4, 2024 · 8 comments

Comments

@abelsromero
Copy link
Member

Thank you for taking your time to talk with us!

What is this issue about?

  • Bug report
  • Feature request
  • Question

Description

When enabling breadcumbs in a maven site, the name for the current page is not present.
image

The expected behauviour should be similar to:

image

Environment information

  • asciidoctor-maven-plugin version: 2.2.5
  • asciidoctorj version: 2.5.11
  • Maven, Java and OS version: all
@abelsromero abelsromero added the bug label Feb 4, 2024
@abelsromero abelsromero added this to the 2.2.6 milestone Feb 4, 2024
@abelsromero
Copy link
Member Author

abelsromero commented Feb 4, 2024

Some reverse engineering from the maven-site-plugin documentation shows, that information must come from the metadata encoded in the same document. For the site plugin the APT module uses the document title which is the document header https://github.com/apache/maven-site-plugin/blob/d78b8da70fd186738707f14166812d721064b766/src/site/apt/examples/adding-deploy-protocol.apt.vm#L2.

Note that the breadcrumb configuration is inherited from https://github.com/apache/maven-parent/blob/e1bc57ba6a9b1ac220d810af9d7301b8a95f0806/src/site/site.xml#L65-L68. To test in isolation those elements and fluido config need to be added to a local project.

We can take the opportunity to also add the other metadata https://github.com/apache/maven-doxia/blob/4eda5adb3c9010131ac1f9118d8ed6c262883762/doxia-sink-api/src/main/java/org/apache/maven/doxia/sink/Sink.java#L162-L176.

@abelsromero
Copy link
Member Author

For a possible approach for the asciidoctor plugin we can use asciidoctor.load() to obtain the source metadata and pass it to the sink, the issue is that we'll be effectively parsing the documents twice with a significant performance cost.

sink.head();
sink.title();
sink.text("Example Manual");
sink.title_();
sink.head_();

@mojavelinux
Copy link
Member

Fortunately, there is a trick to avoid this overhead. First, you can set the :parse_header_only option to restrict the parsing to the document header. (See https://docs.asciidoctor.org/asciidoctor/latest/api/options/). Another trick you can use is to try to detect the end of the header and split the source so the parser only has to read the lines from the header. We use this trick in Antora. See https://gitlab.com/antora/antora/-/blob/main/packages/asciidoc-loader/lib/load-asciidoc.js?ref_type=heads#L72-77 However, that trick does impose some restrictions on how the document is formatted and may not be appropriate. You'll need to decide whether it is worth it.

@abelsromero
Copy link
Member Author

You'll need to decide whether it is worth it.

I like the idea of splitting the header. The issue is the parse receives an InputStreamReader so we'd need to see how to extract the data without impacting the conversion or duplicating buffers.

Also, note to future self, we'd need to pass options and attributes to load in case the user is composing that data into other attributes like title.

@abelsromero
Copy link
Member Author

PR is ready #769. Still on plan to release tomorrow.

abelsromero added a commit that referenced this issue Feb 9, 2024
* Set document title, author(s) and date for Maven site integration

fixes #763

---------

Co-authored-by: Alexander Kriegisch <Alexander@Kriegisch.name>
@abelsromero abelsromero modified the milestones: 2.2.6, 3.0.1 Feb 9, 2024
@abelsromero
Copy link
Member Author

@kriegaex v2.2.6 is out, thanks for the report and the research.

kriegaex added a commit to dev-aspectj/aspectj-maven-plugin that referenced this issue Feb 10, 2024
@kriegaex
Copy link

I upgraded, and it works. Thank you so much.

kriegaex added a commit to dev-aspectj/aspectj-maven-plugin that referenced this issue Feb 10, 2024
@abelsromero abelsromero modified the milestones: 3.0.1, 3.0.0 Feb 11, 2024
@abelsromero abelsromero added 3.0.0 and removed 3.1.0 labels Feb 11, 2024
@abelsromero
Copy link
Member Author

Moving this to 3.0.0. The fix for 2.2.6 was already done in a way that can be easily ported.

abelsromero added a commit that referenced this issue Feb 11, 2024
* Add new HeaderParser and HeaderMetadata components to obtain Asciidoctor header
details and inject them into the Sink header
* Renames package `org.asciidoctor.maven.site.ast` to `org.asciidoctor.maven.site.parser`
to match module name
* Updated Integration Tests
* Quick docs update (may require review after v3.0.0 release)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants