rustdoc: general cleanup #63639
Merged
rustdoc: general cleanup #63639
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
|
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 |
JohnTitor
added
the
S-waiting-on-review
matklad
reviewed
Aug 18, 2019
src/librustdoc/html/render.rs
Outdated
| @@ -218,7 +219,7 @@ struct SharedContext { | |||
| } | |||
|
|
|||
| impl SharedContext { | |||
| fn ensure_dir(&self, dst: &Path) -> Result<(), Error> { | |||
| pub fn ensure_dir(&self, dst: &Path) -> Result<(), Error> { | |||
There was a problem hiding this comment.
Wouldn't this become an "unreachable pub", once we turn that warning on, due to the fact that SharedContext is crate?
src/librustdoc/html/format.rs
Outdated
| @@ -1063,3 +1063,21 @@ impl fmt::Display for DefaultSpace { | |||
| } | |||
| } | |||
| } | |||
|
|
|||
| pub struct WithFormatter<F>(Cell<Option<F>>); | |||
There was a problem hiding this comment.
Might go one step further with abstraction:
fn display_fn(f: impl FnOnce(&mut fmt::Formatter) -> fmt::Result) -> impl fmt::Display {
...
}
There was a problem hiding this comment.
Also, does this need to be pub as opposed to just crate?
src/librustdoc/html/format.rs
Outdated
| fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { | ||
| if let Some((url, short_ty, fqp)) = href(self.did) { | ||
| pub fn anchor(did: DefId, text: &str) -> impl fmt::Display + '_ { | ||
| WithFormatter::new(move |f| { |
There was a problem hiding this comment.
I think WithFormatter is introduced several commits ahead, which is mildly confusing
|
I like the direction it's taking: less |
|
Okay, I'll fix up @matklad's comments here in the next few days hopefully and probably leave further fmt::Display cleanup to a future PR. |
|
Moved this from description since I both don't want it to get lost but it also isn't noteworthy enough to go into commit history: I'm still thinking about a good way to deal with the HTML formatting. One possible answer is explored in two of these commits, in particular "Transition CommaSep to function" and " I can drop any of these commits if we want to discuss them more, but I think most of them here are straight improvements. |
Mark-Simulacrum
force-pushed
the
rustdoc-clean-3
branch
from
fb313ff
to
e6297dc
Compare
|
Okay, I think I've fixed nits and hopefully CI; this is ready for review. |
Mark-Simulacrum
force-pushed
the
rustdoc-clean-3
branch
from
e6297dc
to
4a24b60
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 |
|
☔ The latest upstream changes (presumably #61613) made this pull request unmergeable. Please resolve the merge conflicts. |
This allows us to pass it a `&mut DocContext` which will allow removal of RefCells, etc. in the following commits. It's also somewhat a unique Clean impl in that it previously ignored `self` (re-retriveing hir::Crate), which it no longer needs to do.
We have &mut access, so remove the RefCell borrowing
The alternate mode merely prints out the passed in text which is largely useless (as the text can simply be directly printed).
Previously we stored the entire current path which is a bit expensive and only ever accessed its length. This stores the length directly.
This introduces a WithFormatter abstraction that permits one-time fmt::Display on an arbitrary closure, created via `display_fn`. This allows us to prevent allocation while still using functions instead of structs, which are a bit unwieldy to thread arguments through as they can't easily call each other (and are generally a bit opaque). The eventual goal here is likely to move us off of the formatting infrastructure entirely in favor of something more structured, but this is a good step to move us in that direction as it makes, for example, passing a context describing current state to the formatting impl much easier.
Mark-Simulacrum
force-pushed
the
rustdoc-clean-3
branch
from
4a24b60
to
b0fab96
Compare
|
Rebased -- dropped one commit which was leading to some of the failures and fixed another which was leading to the other test failures. Should pass CI (though I've said that before...) |
|
Thanks! @bors: r+ |
|
📌 Commit b0fab96 has been approved by |
bors
added
S-waiting-on-bors
|
☀️ Test successful - checks-azure |
This was referenced Aug 27, 2019
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.
No description provided.