Update pulldown-cmark version #65894
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Do we expect any functional changes? i.e., is this just an API change? The changelog suggests that's the case pulldown-cmark/pulldown-cmark@v0.5.3...v0.6.0. Maybe a rustdoc crater run makes sense here -- it won't test html is unchanged, but at least new doctests I guess and such. |
tesuji
reviewed
Oct 28, 2019
|
I'd be concerned about making this change until pulldown-cmark/pulldown-cmark#398 is fixed, since it is relatively easy to trigger a panic. Also, I'm wondering if you ran into issues with intra-rustdoc links? When I tried this a while back, I found that it more often used Strings instead of &str, and that messes up the pointer subtraction in the bad link callback. |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
@ehuss Indeed! Let's wait until it's merged then. |
Dylan-DPC-zz
added
the
S-blocked
|
It's now merged |
|
Thanks for the info. Waiting for the release now! :) |
GuillaumeGomez
force-pushed
the
up-pulldown-cmark
branch
2 times, most recently
from
6ae51ba
to
638945d
Compare
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
GuillaumeGomez
force-pushed
the
up-pulldown-cmark
branch
from
638945d
to
b413fe3
Compare
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
|
@GuillaumeGomez @Mark-Simulacrum @ehuss pulldown-cmark released a new version on Monday, so this is no longer blocked on that |
Dylan-DPC-zz
added
S-waiting-on-review
|
Updated! |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
Mark-Simulacrum
added
S-waiting-on-author
GuillaumeGomez
force-pushed
the
up-pulldown-cmark
branch
from
a09635c
to
be6c639
Compare
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
GuillaumeGomez
force-pushed
the
up-pulldown-cmark
branch
from
be6c639
to
339e29a
Compare
|
Updated. Let's see if all the tests are happy this time! |
|
Once tests are passing and presuming there are no functional changes, r=me. |
|
@bors: r=Mark-Simulacrum |
|
📌 Commit 339e29a has been approved by |
bors
added
S-waiting-on-bors
|
⌛ Testing commit 339e29a with merge e3af139bc36297a97f607f5ae119f28496338e43... |
bjorn3
reviewed
Nov 17, 2019
| @@ -115,7 +115,8 @@ warning: could not parse code block as Rust code | |||
| LL | /// code with bad syntax | |||
| | _________^ | |||
| LL | | /// \_ | |||
| | |__________^ | |||
| LL | | /// | |||
| | |_ | |||
|
💔 Test failed - checks-azure |
|
The job Click to expand the log.I'm a bot! I can only do what humans tell me to, so if this was not helpful or you have suggestions for improvements, please ping or otherwise contact |
bors
added
S-waiting-on-review
ehuss
reviewed
Nov 17, 2019
| } | ||
| code_block = Some(offset_range.clone()); | ||
|
|
||
| code_start = if !md[previous_offset.end..offset_range.start].ends_with(" ") { |
There was a problem hiding this comment.
I don't think it is necessary to scan through the text to find the code block start/end now that it is using into_offset_iter. It has the exact ranges provided by pulldown-cmark. Perhaps something like this will be a little more reliable?
diff --git a/src/librustdoc/html/markdown.rs b/src/librustdoc/html/markdown.rs
index d10b131af8e..e1fc7075372 100644
--- a/src/librustdoc/html/markdown.rs
+++ b/src/librustdoc/html/markdown.rs
@@ -922,11 +922,11 @@ crate fn rust_code_blocks(md: &str) -> Vec<RustCodeBlock> {
let mut p = Parser::new_ext(md, opts()).into_offset_iter();
- let mut code_block = None;
+ let mut code_block_range = 0..0;
let mut code_start = 0;
+ let mut code_end = 0;
let mut is_fenced = false;
let mut in_rust_code_block = false;
- let mut previous_offset = Range { start: 0, end: 0 };
while let Some((event, offset_range)) = p.next() {
match event {
Event::Start(Tag::CodeBlock(syntax)) => {
@@ -938,36 +938,23 @@ crate fn rust_code_blocks(md: &str) -> Vec<RustCodeBlock> {
if lang_string.rust {
in_rust_code_block = true;
- code_block = Some(offset_range.clone());
-
- code_start = if !md[previous_offset.end..offset_range.start].ends_with(" ") {
- is_fenced = true;
- offset_range.start + md[offset_range.clone()]
- .lines()
- .next()
- .map_or(0, |x| x.len() + 1)
- } else {
- is_fenced = false;
- offset_range.start
- };
+ code_block_range = offset_range.clone();
+ is_fenced = md[offset_range].starts_with("```");
+ }
+ }
+ Event::Text(_) if in_rust_code_block => {
+ // Once inside a code block, the Text event gives the contents
+ // of the code block.
+ if code_start == 0 {
+ code_start = offset_range.start;
}
+ code_end = offset_range.end;
}
Event::End(Tag::CodeBlock(syntax)) if in_rust_code_block => {
in_rust_code_block = false;
-
- let code_end = if is_fenced {
- let last_len = md[offset_range.clone()]
- .lines()
- .last()
- .filter(|l| l.ends_with("```"))
- .map_or(0, |l| l.len());
- offset_range.end - last_len
- } else {
- offset_range.end
- };
code_blocks.push(RustCodeBlock {
is_fenced,
- range: code_block.clone().unwrap(),
+ range: code_block_range.clone(),
code: Range {
start: code_start,
end: code_end,
@@ -978,10 +965,10 @@ crate fn rust_code_blocks(md: &str) -> Vec<RustCodeBlock> {
None
},
});
+ code_start = 0;
}
_ => (),
}
- previous_offset = offset_range;
}
code_blocksThere was a problem hiding this comment.
Unfortunately it's not. I had this issue as well. :)
In the case the block code is indented and not started with backticks, then if we have backticks, it's text. We even have a test for this use case. Therefore, we need to check if the code block is preceded by whitespaces.
There was a problem hiding this comment.
Just to be clear, are you referring to this test? If it is important to avoid adding the suggestion to add text, a modification of the patch above should work: Add this to Event::Text:
is_fenced = code_start != code_block_range.start;
and remove is_fenced from Tag::CodeBlock. That is, when the Event::Text has the same offset as Event::Start(Tag::CodeBlock)), then it should be an indented codeblock.
There was a problem hiding this comment.
That's a hack and not a good one unfortunately. Like I said below, the easiest way is to simply know beforehand the kind of code block this is. I sent a PR for it in pulldown-cmark. That'll allow us to just remove all of this.
|
Actually, it's too much of an issue to be able to tell from code if this is indented or not. Therefore I opened pulldown-cmark/pulldown-cmark#415 and I'm sending a PR to fix it. It'll be simpler this way. |
|
Marking this as blocked waiting for the upstream issue |
Dylan-DPC-zz
added
S-blocked
|
Ok, waiting on pulldown-cmark/pulldown-cmark#416 |
|
☔ The latest upstream changes (presumably #67198) made this pull request unmergeable. Please resolve the merge conflicts. |
|
🔒 Merge conflict This pull request and the master branch diverged in a way that cannot be automatically merged. Please rebase on top of the latest master branch, and let the reviewer approve again. How do I rebase?Assuming
You may also read Git Rebasing to Resolve Conflicts by Drew Blessing for a short tutorial. Please avoid the "Resolve conflicts" button on GitHub. It uses Sometimes step 4 will complete without asking for resolution. This is usually due to difference between how Error message |
|
Closing this due to inactivity, and merge conflicts with the base branch. Feel free to re-open once fixes for |
JohnTitor
added a commit
to JohnTitor/rust
that referenced
this pull request
Feb 14, 2020
…k, r=Dylan-DPC Update pulldown-cmark dependency r? @kinnison cc @ollie27 Reopening of rust-lang#65894.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
r? @Mark-Simulacrum