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

ZOOKEEPER-3807: fix the bad format when website pages build due to bash marker #1336

Closed
wants to merge 1 commit into from

Conversation

maoling
Copy link
Member

@maoling maoling commented Apr 25, 2020

Copy link
Member

@ctubbsii ctubbsii left a comment

Choose a reason for hiding this comment

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

This says more details are on the issue, but there are zero details on the issue at all.

What is the nature of the breakage?

Also, this change seems to switch from explicit fenced code blocks to the more basic ones that are whitespace sensitive. In my experience, the fenced code blocks are better, and less prone to breakage due to improper spacing.

Can this PR be updated to a more minimal change that does a targeted fix of whatever is broken, leaving the existing fenced code blocks in place, rather than switch to the basic code blocks, which seems unrelated?

@maoling
Copy link
Member Author

maoling commented Apr 26, 2020

@ctubbsii

@ctubbsii
Copy link
Member

@maoling From what I can tell, the bad formatting has absolutely nothing to do with the markdown at all, but the markdown-page-generator-plugin, which generates different HTML for the two kinds of code blocks. The basic block generates <pre><code>content here</code></pre>, whereas the fenced code block is generated as <p><code>content here</code></p>.

This difference matters, because the profile.css stylesheet styles <pre> blocks, but not <code> blocks. A much simpler change (pretty trivial, actually), that retains use of the (IMO superior) fenced code block style is to simply add the appropriate CSS for the <code> to zookeeper-docs/src/main/resources/markdown/skin/profile.css

Long-term, I think the build should be changed to stop rendering static HTML in a Maven build with a Java plugin and a non-standard Markdown parser, and instead use a better tool designed for the job (such as Jekyll, with the widely popular Kramdown).

@symat
Copy link
Contributor

symat commented May 7, 2020

simply add the appropriate CSS for the <code> to zookeeper-docs/src/main/resources/markdown/skin/profile.css

this would make a lot of sense. Also it would make the PR easier to backport to older branches (as I am sure the current approach will resulting a lot of merge conflicts).

Still, I saw other smaller improvements in the PR (unrelated to the code blocks), I would keep those.

@ztzg
Copy link
Contributor

ztzg commented Sep 1, 2021

Closing this (valiant) attempt in favor of #1741 (review), as acknowledged by @maoling. Fenced code blocks are specified by CommonMark, so I don't feel retaining them locks us into a specific rendering engine.

@ztzg ztzg closed this Sep 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants