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

Use mdbook::utils::new_cmark_parser to preserve tables #17

Merged
merged 4 commits into from
Dec 2, 2022
Merged

Use mdbook::utils::new_cmark_parser to preserve tables #17

merged 4 commits into from
Dec 2, 2022

Conversation

mgeisler
Copy link
Collaborator

@mgeisler mgeisler commented Dec 1, 2022

Before, we would ignore the Markdown settings from mdBook and parse the content with default settings. This meant that we did not support Markdown tables, which are an extension to regular Markdown. The result was that tables were mangled after the preprocessor had run. They looked like this:

image

We now use the mdBook utility function to construct the Markdown parser, thus getting consistent results. This makes tables survive the parsing.

The PR also has a commit which restores the preprocessing functionality, it was accidentally lost in the refactor in 900cd77.

This was accidentally lost in 900cd77.
Before, we would ignore the Markdown settings from mdBook and parse
the content with default settings. This meant that we did not support
Markdown tables, which are an extension to regular Markdown. The
result was that tables were mangled after the preprocessor had run.

We now use the mdBook utility function to construct the Markdown
parser, thus getting consistent results. This makes tables survive the
parsing.
Copy link
Owner

@boozook boozook left a comment

Choose a reason for hiding this comment

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

Great! Thank you so much 👍

@boozook
Copy link
Owner

boozook commented Dec 1, 2022

@mgeisler,
All looks perfect, but formatting. Please just accept my PR firstly.

@mgeisler
Copy link
Collaborator Author

mgeisler commented Dec 2, 2022

@mgeisler, All looks perfect, but formatting. Please just accept my PR firstly.

Thanks, happy to merge that of course!

About the formatting: I'm pretty sure my editor ran rustfmt with the settings from the .rustfmt.toml file in the repository. I see that because it used TABs instead of spaces for the indentation. Are you formatting things further outside of running rustfmt?

@mgeisler
Copy link
Collaborator Author

mgeisler commented Dec 2, 2022

Ah, I see now: the rustfmt run failed.

The problem was that I had a stable compiler as my default. When I switch to nightly, the code looks like your PR. Mystery solved :-D

Comment on lines +94 to +119
let events = parser.map(|e| {
use State::*;
use Event::*;
use CowStr::*;
use CodeBlockKind::*;
use Tag::{CodeBlock, Paragraph};

match (&e, &mut state) {
(Start(CodeBlock(Fenced(Borrowed(mark)))), None) if mark == &cfg.code_block => {
state = Open;
Some(Start(Paragraph))
},

(Text(Borrowed(text)), Open) => {
state = Closing;
Some(Html(bob_handler(text, &cfg.settings).into()))
},

(End(CodeBlock(Fenced(Borrowed(mark)))), Closing) if mark == &cfg.code_block => {
state = None;
Some(End(Paragraph))
},
_ => Some(e),
}
})
.flatten();

Check warning

Code scanning / clippy

called `map(..).flatten()` on `Iterator`

called `map(..).flatten()` on `Iterator`
Comment on lines +94 to +119
let events = parser.map(|e| {
use State::*;
use Event::*;
use CowStr::*;
use CodeBlockKind::*;
use Tag::{CodeBlock, Paragraph};

match (&e, &mut state) {
(Start(CodeBlock(Fenced(Borrowed(mark)))), None) if mark == &cfg.code_block => {
state = Open;
Some(Start(Paragraph))
},

(Text(Borrowed(text)), Open) => {
state = Closing;
Some(Html(bob_handler(text, &cfg.settings).into()))
},

(End(CodeBlock(Fenced(Borrowed(mark)))), Closing) if mark == &cfg.code_block => {
state = None;
Some(End(Paragraph))
},
_ => Some(e),
}
})
.flatten();

Check warning

Code scanning / clippy

called `map(..).flatten()` on `Iterator`

called `map(..).flatten()` on `Iterator`
@boozook boozook merged commit 7b92144 into boozook:master Dec 2, 2022
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 this pull request may close these issues.

None yet

2 participants