Correct codeblocks to not break cargo test --doc
#633
Conversation
|
I can see that this is failing with the MSRV of 1.51. I'll look into that. |
|
Based on the rule mentioned in #552 I think it's OK for me to bump MSRV to 1.53.0. |
|
@LucioFranco We spoke about this back when I first wrote the patch - is there any chance it can be considered soon? If it needs big changes then I'm now back from my travels and able to spend time on it again. In the meantime I've rebased onto current mainline. |
|
Hey! Sorry, I must have closed my tab when looking at this. So I am not convinced I want to add pulldown_cmark as a dependency (knowing people will complain). I wonder if we can either 1) vendor the code required to do this parsing or 2) implement a custom way to just swap out the way it generates the markdown. What are your thoughts? (btw thanks for the ping, I sometimes lose track of things gh isn't always the best at keeping track) |
|
Regarding not wanting pulldown-cmark, I can understand that, and I tried to write something which would avoid it, but ultimately there's no standard for the comments in protobuf files; and rustdoc does use pulldown-cmark which basically led me to "this is ultimately the best bet" - I'd be happy to add a feature flag for it though. |
|
@kinnison That is fair, I think actually an opt-in feature flag is worth it so people can disable it. I'd be happy to merge that. |
|
I'll get on that tomorrow then. Also I'll see if I can minimise features on the new deps too |
|
I've taken your comment to mean that you want an off-by-default feature flag, so I've put that together. For ease of having the bootstrapped files still nice for It's worth noting though that the extra deps are pretty small; however I accept that not everyone will want their doc strings being modified like this, so I'm happy with it being optional-off-by-default. |
|
I apologise for the noise on this PR, the minversions check threw me for a loop. |
Because Google make no statement about the format of the content of comments in protocol buffers, since they're essentially just comments, there is a mix of content out there, some markdown, some not. And where it parses as markdown, some comments have code blocks in them, for example `google.api.HttpRule` does. Since prost-build puts these comments into the output code as documentation comments, we treat the text as markdown and process it to ensure that code blocks are not incorrectly treated as Rust doctests. Signed-off-by: Daniel Silverstone <[email protected]>
Since comments are now passed through a markdown processor, leading spaces which do not form code blocks end up stripped. As such, we need to restore them in our line sanitiser. Signed-off-by: Daniel Silverstone <[email protected]>
With the change to prost-build to process all comments as markdown and sanitise it, these files needed updating. The resulting text is not necessarily as pleasing to the eye, but it is faithful to how `rustdoc` will ultimately process it. Signed-off-by: Daniel Silverstone <[email protected]>
Required because the new pulldown-cmark-to-cmark dependency uses "or" patterns which requires 1.53.0 or newer. This is acceptable since 1.53.0 was released 2021-06-17 Signed-off-by: Daniel Silverstone <[email protected]>
In order to ensure that we do not end up with extra dependencies being added to everyone's build, make the cleanup feature optional and not enabled by default. This will permit people to opt in to the feature. If over time it is discovered that enough people opt in, we can make it default later. Signed-off-by: Daniel Silverstone <[email protected]>
|
@kinnison sweet, lgtm! One more thing can you add some docs to the crate docs about this feature flag? I think that is the last remaining thing along with CI passing! Thanks. |
|
Yep, I'll add the docs. (and fix the minversions thing which bit again) |
This corrects the minversions check since we need the latest versions to make things work. Signed-off-by: Daniel Silverstone <[email protected]>
Signed-off-by: Daniel Silverstone <[email protected]>
Signed-off-by: Daniel Silverstone <[email protected]>
|
Hopefully that'll do the trick. If that isn't good enough documentation then please let me know what else you would like to see - I'm awful at writing these things :D |
|
We have a green CI run 🎉 |
I have been working with protocol buffers whose documentation includes code blocks, often things like example service descriptions and the like (consider https://github.com/googleapis/googleapis/blob/master/google/api/http.proto#L67-L81 )
As such, it's important that by the time prost-build has run, the code blocks in those comments have been transformed into something safe from
rustdoc's test extractor.This PR is split into three commits. The first adds the code-block processing support. The second fixes up ensuring there's leading spaces in the comments to retain the pleasant-ish human look, and the third updates the generated code files for
prost-typesso that things are consistent.