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.

Sign up for GitHub

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

Jump to bottom

Remove short doc where it starts with a codeblock #55136

Merged
merged 1 commit into from
Nov 14, 2018
Merged

Remove short doc where it starts with a codeblock #55136

merged 1 commit into from
Nov 14, 2018

Conversation

GuillaumeGomez
Copy link
Member

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 17, 2018
@GuillaumeGomez
Copy link
Member Author

Forgot to add test, I'll update the commit later on.

@TimNN TimNN added S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Oct 30, 2018
QuietMisdreavus
QuietMisdreavus reviewed Oct 30, 2018
View reviewed changes
src/librustdoc/html/markdown.rs Outdated
while let Some(event) = self.inner.next() {
let mut is_start = true;
let is_allowed_tag = match event {
Event::Start(Tag::CodeBlock(_)) => {
Copy link
Member

Choose a reason for hiding this comment

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

Why was the extra handling of the CodeBlock tag necessary?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is the whole point of the PR. :)

Copy link
Member

Choose a reason for hiding this comment

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

I think i see it now. I had thought that just taking it out of check_if_allowed_tag above would work, but i guess that would then splice the entire code block as a code block instead of adding the code as a <p> tag. With these extra checks, we can properly skip it.

It does seem like you're making it pick the first non-code-block paragraph, though. Is that intended? Can you add a test to demonstrate this?

Copy link
Member Author

Choose a reason for hiding this comment

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

It didn't pick anything on my tests but I'll see if it takes the next element just in case.

Copy link
Member Author

Choose a reason for hiding this comment

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

I confirm it takes the next item. Do you want to keep this behavior or do you want me to just remove the short docblock in case it starts with a codeblock?

Copy link
Member

Choose a reason for hiding this comment

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

I think i'd prefer if it removed the short docblock in that case.

@GuillaumeGomez
Copy link
Member Author

Removed text if anything is after the codeblock.

@QuietMisdreavus
Copy link
Member

Can you add a test?

@GuillaumeGomez
Copy link
Member Author

Sure.

@GuillaumeGomez
Copy link
Member Author

Added test.

QuietMisdreavus
QuietMisdreavus approved these changes Nov 12, 2018
View reviewed changes
Copy link
Member

@QuietMisdreavus QuietMisdreavus left a comment

Choose a reason for hiding this comment

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

Would have preferred the space-collapsing change went in its own commit, but this works.

@QuietMisdreavus
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Nov 12, 2018

📌 Commit 3030cbe has been approved by QuietMisdreavus

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-author Status: This is awaiting some action (such as code changes or more information) from the author. labels Nov 12, 2018
@bors
Copy link
Contributor

bors commented Nov 13, 2018

⌛ Testing commit 3030cbe with merge 210dc6a20c10810ad6b959467243498d1015c9a9...

@bors
Copy link
Contributor

bors commented Nov 13, 2018

💔 Test failed - status-travis

@bors bors removed the S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. label Nov 13, 2018
@rust-highfive
Copy link
Collaborator

The job i686-gnu of your PR failed on Travis (raw log). Through arcane magic we have determined that the following fragments from the build log may contain information about the problem.

Click to expand the log. 
[02:07:41] test collections/hash/map.rs - collections::hash::map::HashMap<K, V, S>::values_mut (line 1104) ... ok
[02:07:41] test collections/hash/map.rs - collections::hash::map::HashMap<K, V, S>::with_capacity_and_hasher (line 772) ... ok
[02:07:41] test collections/hash/map.rs - collections::hash::map::HashMap<K, V, S>::with_hasher (line 741) ... ok
[02:07:42] test collections/hash/map.rs - collections::hash::map::OccupiedEntry<'a, K, V>::get (line 2811) ... ok
No output has been received in the last 30m0s, this potentially indicates a stalled build or something wrong with the build itself.
Check the details on how to adjust your build configuration on: https://docs.travis-ci.com/user/common-build-problems/#Build-times-out-because-no-output-was-received
The build has been terminated

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 @TimNN. (Feature Requests)

@bors bors added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Nov 13, 2018
@kennytm
Copy link
Member

kennytm commented Nov 13, 2018 •
edited
Loading

@bors retry rollup

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Nov 13, 2018
@kennytm kennytm mentioned this pull request Nov 13, 2018
Merged
kennytm added a commit to kennytm/rust that referenced this pull request Nov 13, 2018
@kennytm
Rollup merge of rust-lang#55136 - GuillaumeGomez:short-doc, r=QuietMi…
2173681 
…sdreavus

Remove short doc where it starts with a codeblock

Fixes rust-lang#54975.
bors added a commit that referenced this pull request Nov 13, 2018
@bors
Auto merge of #55912 - kennytm:rollup, r=kennytm
f1d6183 
Rollup of 20 pull requests

Successful merges:

 - #55136 (Remove short doc where it starts with a codeblock)
 - #55711 (Format BtreeMap::range_mut example)
 - #55722 (impl_stable_hash_for: support enums and tuple structs with generic parameters)
 - #55754 (Avoid converting bytes to UTF-8 strings to print, just pass bytes to stdout/err)
 - #55804 (rustdoc: don't inline `pub use some_crate` unless directly asked to)
 - #55805 (Move `static_assert!` into librustc_data_structures)
 - #55837 (Make PhantomData #[structural_match])
 - #55840 (Fix TLS errors when downloading stage0)
 - #55843 (add FromIterator<A> to Box<[A]>)
 - #55858 (Small fixes on code blocks in rustdoc)
 - #55863 (Fix a typo in std::panic)
 - #55870 (Fix typos.)
 - #55874 (string: Add documentation for `From` impls)
 - #55879 (save-analysis: Don't panic for macro-generated use globs)
 - #55882 (Reference count `crate_inherent_impls`s return value.)
 - #55888 (miri: for uniformity, also move memory_deallocated to AllocationExtra)
 - #55889 (global allocators: add a few comments)
 - #55896 (Document optimizations enabled by FusedIterator)
 - #55905 (Change `Lit::short_name` to `Lit::literal_name`.)
 - #55908 (Fix their/there grammar nit)
@bors bors merged commit 3030cbe into rust-lang:master Nov 14, 2018
@GuillaumeGomez GuillaumeGomez deleted the short-doc branch November 14, 2018 09:54
@kennytm kennytm mentioned this pull request Nov 20, 2018
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@QuietMisdreavus QuietMisdreavus QuietMisdreavus approved these changes

Assignees

@QuietMisdreavus QuietMisdreavus

Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@GuillaumeGomez @QuietMisdreavus @bors @rust-highfive @kennytm @TimNN