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

[MNGSITE-504] Fix rendering / markdown warnings #614

Merged
merged 9 commits into from
Jan 4, 2025

Conversation

Bukama
Copy link
Contributor

@Bukama Bukama commented Dec 29, 2024

With this PR I updated some very old (, but still existing) pages to reduce the renderer warnings during the site goal (see MNGSITE-504).

This PR is *not to update the outdated content of those pages!

I could fix all warnings in the existing files, except the following four:

[WARNING] Unrecognized xdoc tag <base> at [32:52]
[WARNING] Unrecognized xdoc tag <link> at [34:100]
[WARNING] Unrecognized xdoc tag <link> at [35:75]
[WARNING] Unrecognized xdoc tag <link> at [36:90]

These are from CSS definition in the errors/404.xml.vm. I couldn't find a documentation how to define them properly, esp. as we need dynamic links. This why I transformed the page to a markdown file.

edit: Reverted changes done to 404.xml.vm - see discussion below.

@@ -0,0 +1,29 @@
<!--
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't think markdown supports comments?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think markdown supports comments?

Works fine:
image

Copy link
Contributor

Choose a reason for hiding this comment

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

I think that's a function of some particular implementation that extends the spec. I'm not sure we can/should rely on that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that's a function of some particular implementation that extends the spec. I'm not sure we can/should rely on that.

Then we need to change all other .md files too and remove every comments and licences there...

Copy link
Member

Choose a reason for hiding this comment

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

No, that is a bug @kwin had ready fixed.on master.

Copy link
Contributor

Choose a reason for hiding this comment

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

My bad. I forgot about inline HTML. This is OK synatctically

content/markdown/errors/404.md Outdated Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

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

We can not move it to md
We need additional html page headers.

This content can be displayed in any path .... and without those header content will be corrupted.

@kwin @michael-o can we add html headers in md format?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can not move it to md We need additional html page headers.

This content can be displayed in any path .... and without those header content will be corrupted.

@kwin @michael-o can we add html headers in md format?

To add as an information what I've tested locally:
I checked the .htaccess which redirects to this in case of 404 and I checked if /errors/404.html is displayed correctly - which it is.
But I'm honest: I'm not very experienced with .htaccess file (as you probably already noticed).

Copy link
Member

Choose a reason for hiding this comment

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

The problem will be on page like: https://maven.apache.org/plugins/maven-abc-plugin/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, will wait for an answer of @kwin or @michael-o about your question and then change or revert the 404 file

Copy link
Member

Choose a reason for hiding this comment

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

In general every valid HTML is also valid in MD (https://daringfireball.net/projects/markdown/syntax#html). This is more a question on how the resulting HTML is being partitioned by the doxia-sitetools (to inject things like navigation).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

In general every valid HTML is also valid in MD (https://daringfireball.net/projects/markdown/syntax#html). This is more a question on how the resulting HTML is being partitioned by the doxia-sitetools (to inject things like navigation).

Thank you for your response. For the moment I reverted the changes in the 404 (meaning the vm is back there and the md is gone), until we are sure. To change the 404 to MD is done in few mins anyway.
But this means that the rendering warnings, caused by this page (double title and the HTTP Unrecognized xdoc tag are back in place)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Side note: Did some tests about using test and filed an issue DOXIASITETOOLS-358

@slachiewicz slachiewicz added maintenance documentation Improvements or additions to documentation labels Jan 4, 2025
@Bukama
Copy link
Contributor Author

Bukama commented Jan 4, 2025

Rebased after several other commits were merged into master

@slachiewicz slachiewicz merged commit 85b1e4e into apache:master Jan 4, 2025
@Bukama Bukama deleted the renderwarnings branch January 4, 2025 17:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation maintenance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants