From 8fb1250aba8135679463351a3813c04ae45bf311 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Tue, 10 Oct 2017 23:39:10 +0200 Subject: [PATCH 1/4] Improve sidebar rendering and add methods list --- src/librustdoc/html/render.rs | 62 ++++++++++++++++++++------ src/librustdoc/html/static/rustdoc.css | 27 ++++++++--- 2 files changed, 69 insertions(+), 20 deletions(-) diff --git a/src/librustdoc/html/render.rs b/src/librustdoc/html/render.rs index d538428a7e9a9..b73988823f448 100644 --- a/src/librustdoc/html/render.rs +++ b/src/librustdoc/html/render.rs @@ -3513,12 +3513,29 @@ impl<'a> fmt::Display for Sidebar<'a> { } } +fn get_methods(i: &clean::Impl) -> Vec { + i.items.iter().filter_map(|item| { + match item.name { + // Maybe check with clean::Visibility::Public as well? + Some(ref name) if !name.is_empty() && item.visibility.is_some() && item.is_method() => { + Some(format!("{name}", name = name)) + } + _ => None, + } + }).collect::>() +} + fn sidebar_assoc_items(it: &clean::Item) -> String { let mut out = String::new(); let c = cache(); if let Some(v) = c.impls.get(&it.def_id) { - if v.iter().any(|i| i.inner_impl().trait_.is_none()) { - out.push_str("
  • Methods
    • "); + let ret = v.iter() + .filter(|i| i.inner_impl().trait_.is_none()) + .flat_map(|i| get_methods(i.inner_impl())) + .collect::(); + if !ret.is_empty() { + out.push_str(&format!("Methods\ +
    {}
    ", ret)); } if v.iter().any(|i| i.inner_impl().trait_.is_some()) { @@ -3534,16 +3551,33 @@ fn sidebar_assoc_items(it: &clean::Item) -> String { let inner_impl = target.def_id().or(target.primitive_type().and_then(|prim| { c.primitive_locations.get(&prim).cloned() })).and_then(|did| c.impls.get(&did)); - if inner_impl.is_some() { - out.push_str("
  • "); + if let Some(impls) = inner_impl { + out.push_str(""); out.push_str(&format!("Methods from {:#}<Target={:#}>", - impl_.inner_impl().trait_.as_ref().unwrap(), - target)); - out.push_str("
    • "); + impl_.inner_impl().trait_.as_ref().unwrap(), + target)); + out.push_str(""); + let ret = impls.iter() + .filter(|i| i.inner_impl().trait_.is_none()) + .flat_map(|i| get_methods(i.inner_impl())) + .collect::(); + out.push_str(&format!("
    {}
    ", ret)); } } } - out.push_str("
  • Trait Implementations
    • "); + let ret = v.iter() + .filter_map(|i| if let Some(ref i) = i.inner_impl().trait_ { + let out = format!("{:#}", i).replace("<", "<").replace(">", ">"); + Some(format!("{name}", i, out)) + } else { + None + }) + .collect::(); + if !ret.is_empty() { + out.push_str("\ + Trait Implementations"); + out.push_str(&format!("
    {}
    ", ret)); + } } } @@ -3564,7 +3598,7 @@ fn sidebar_struct(fmt: &mut fmt::Formatter, it: &clean::Item, sidebar.push_str(&sidebar_assoc_items(it)); if !sidebar.is_empty() { - write!(fmt, "
      {}
    ", sidebar)?; + write!(fmt, "
    {}
    ", sidebar)?; } Ok(()) } @@ -3606,7 +3640,7 @@ fn sidebar_trait(fmt: &mut fmt::Formatter, it: &clean::Item, sidebar.push_str("
  • Implementors
    • "); - write!(fmt, "
      {}
    ", sidebar) + write!(fmt, "
    {}
    ", sidebar) } fn sidebar_primitive(fmt: &mut fmt::Formatter, it: &clean::Item, @@ -3614,7 +3648,7 @@ fn sidebar_primitive(fmt: &mut fmt::Formatter, it: &clean::Item, let sidebar = sidebar_assoc_items(it); if !sidebar.is_empty() { - write!(fmt, "
      {}
    ", sidebar)?; + write!(fmt, "
    {}
    ", sidebar)?; } Ok(()) } @@ -3624,7 +3658,7 @@ fn sidebar_typedef(fmt: &mut fmt::Formatter, it: &clean::Item, let sidebar = sidebar_assoc_items(it); if !sidebar.is_empty() { - write!(fmt, "
      {}
    ", sidebar)?; + write!(fmt, "
    {}
    ", sidebar)?; } Ok(()) } @@ -3641,7 +3675,7 @@ fn sidebar_union(fmt: &mut fmt::Formatter, it: &clean::Item, sidebar.push_str(&sidebar_assoc_items(it)); if !sidebar.is_empty() { - write!(fmt, "
      {}
    ", sidebar)?; + write!(fmt, "
    {}
    ", sidebar)?; } Ok(()) } @@ -3657,7 +3691,7 @@ fn sidebar_enum(fmt: &mut fmt::Formatter, it: &clean::Item, sidebar.push_str(&sidebar_assoc_items(it)); if !sidebar.is_empty() { - write!(fmt, "
      {}
    ", sidebar)?; + write!(fmt, "
    {}
    ", sidebar)?; } Ok(()) } diff --git a/src/librustdoc/html/static/rustdoc.css b/src/librustdoc/html/static/rustdoc.css index 9978435a11243..dcb0de3bb92de 100644 --- a/src/librustdoc/html/static/rustdoc.css +++ b/src/librustdoc/html/static/rustdoc.css @@ -188,18 +188,16 @@ nav.sub { .js-only, .hidden { display: none !important; } -.sidebar { - padding: 10px; -} .sidebar img { margin: 20px auto; display: block; + margin-top: 10px; } .sidebar .location { border: 1px solid; font-size: 17px; - margin: 30px 0 20px 0; + margin: 30px 10px 20px 10px; text-align: center; word-wrap: break-word; } @@ -220,7 +218,7 @@ nav.sub { .location a:first-child { font-weight: 500; } .block { - padding: 0 10px; + padding: 0; margin-bottom: 14px; } .block h2, .block h3 { @@ -229,7 +227,7 @@ nav.sub { text-align: center; } .block ul, .block li { - margin: 0; + margin: 0 10px; padding: 0; list-style: none; } @@ -245,6 +243,23 @@ nav.sub { transition: border 500ms ease-out; } +.sidebar-title { + border-top: 1px solid #777; + border-bottom: 1px solid #777; + text-align: center; + font-size: 17px; + margin-bottom: 5px; +} + +.sidebar-links { + margin-bottom: 15px; +} + +.sidebar-links > a { + padding-left: 10px; + width: 100%; +} + .content { padding: 15px 0; } From 7bde5913274baf1b316b7900b1566fca066d8b9f Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Fri, 27 Oct 2017 00:16:44 +0200 Subject: [PATCH 2/4] Change sidebar items order --- src/librustdoc/html/render.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/librustdoc/html/render.rs b/src/librustdoc/html/render.rs index b73988823f448..bdcfa782bb7c7 100644 --- a/src/librustdoc/html/render.rs +++ b/src/librustdoc/html/render.rs @@ -3568,7 +3568,7 @@ fn sidebar_assoc_items(it: &clean::Item) -> String { let ret = v.iter() .filter_map(|i| if let Some(ref i) = i.inner_impl().trait_ { let out = format!("{:#}", i).replace("<", "<").replace(">", ">"); - Some(format!("{name}", i, out)) + Some(format!("{}", i, out)) } else { None }) @@ -3625,8 +3625,6 @@ fn sidebar_trait(fmt: &mut fmt::Formatter, it: &clean::Item, sidebar.push_str("
  • Provided Methods
    • "); } - sidebar.push_str(&sidebar_assoc_items(it)); - let c = cache(); if let Some(implementors) = c.implementors.get(&it.def_id) { @@ -3640,6 +3638,8 @@ fn sidebar_trait(fmt: &mut fmt::Formatter, it: &clean::Item, sidebar.push_str("
  • Implementors
    • "); + sidebar.push_str(&sidebar_assoc_items(it)); + write!(fmt, "
    {}
    ", sidebar) } From 3dafd2c6908f19d573a94b8fc1e99b58460453a8 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Fri, 27 Oct 2017 23:09:10 +0200 Subject: [PATCH 3/4] Encode urls --- src/librustdoc/html/render.rs | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) diff --git a/src/librustdoc/html/render.rs b/src/librustdoc/html/render.rs index bdcfa782bb7c7..8aad4d301ee9f 100644 --- a/src/librustdoc/html/render.rs +++ b/src/librustdoc/html/render.rs @@ -3525,6 +3525,21 @@ fn get_methods(i: &clean::Impl) -> Vec { }).collect::>() } +// The point is to url encode any potential character from a type with genericity. +fn small_url_encode(s: &str) -> String { + s.replace("<", "%3C") + .replace(">", "%3E") + .replace(" ", "%20") + .replace("?", "%3F") + .replace("'", "%27") + .replace("&", "%26") + .replace(",", "%2C") + .replace(":", "%3A") + .replace(";", "%3B") + .replace("[", "%5B") + .replace("]", "%5D") +} + fn sidebar_assoc_items(it: &clean::Item) -> String { let mut out = String::new(); let c = cache(); @@ -3568,7 +3583,8 @@ fn sidebar_assoc_items(it: &clean::Item) -> String { let ret = v.iter() .filter_map(|i| if let Some(ref i) = i.inner_impl().trait_ { let out = format!("{:#}", i).replace("<", "<").replace(">", ">"); - Some(format!("{}", i, out)) + let encoded = small_url_encode(&format!("{:#}", i)); + Some(format!("{}", encoded, out)) } else { None }) From 6fa521c491b365b354771213186e35c2e2e7a628 Mon Sep 17 00:00:00 2001 From: Guillaume Gomez Date: Sat, 28 Oct 2017 01:11:01 +0200 Subject: [PATCH 4/4] Fix weird bugs --- src/librustdoc/html/render.rs | 74 ++++++++++++++++++++--------------- src/tools/linkchecker/main.rs | 17 ++++++++ 2 files changed, 60 insertions(+), 31 deletions(-) diff --git a/src/librustdoc/html/render.rs b/src/librustdoc/html/render.rs index 8aad4d301ee9f..2de5ddf243f49 100644 --- a/src/librustdoc/html/render.rs +++ b/src/librustdoc/html/render.rs @@ -37,7 +37,7 @@ pub use self::ExternalLocation::*; use std::ascii::AsciiExt; use std::cell::RefCell; use std::cmp::Ordering; -use std::collections::BTreeMap; +use std::collections::{BTreeMap, HashSet}; use std::default::Default; use std::error; use std::fmt::{self, Display, Formatter, Write as FmtWrite}; @@ -3206,12 +3206,37 @@ fn render_deref_methods(w: &mut fmt::Formatter, cx: &Context, impl_: &Impl, } } +fn should_render_item(item: &clean::Item, deref_mut_: bool) -> bool { + let self_type_opt = match item.inner { + clean::MethodItem(ref method) => method.decl.self_type(), + clean::TyMethodItem(ref method) => method.decl.self_type(), + _ => None + }; + + if let Some(self_ty) = self_type_opt { + let (by_mut_ref, by_box) = match self_ty { + SelfTy::SelfBorrowed(_, mutability) | + SelfTy::SelfExplicit(clean::BorrowedRef { mutability, .. }) => { + (mutability == Mutability::Mutable, false) + }, + SelfTy::SelfExplicit(clean::ResolvedPath { did, .. }) => { + (false, Some(did) == cache().owned_box_did) + }, + _ => (false, false), + }; + + (deref_mut_ || !by_mut_ref) && !by_box + } else { + false + } +} + fn render_impl(w: &mut fmt::Formatter, cx: &Context, i: &Impl, link: AssocItemLink, render_mode: RenderMode, outer_version: Option<&str>, show_def_docs: bool) -> fmt::Result { if render_mode == RenderMode::Normal { let id = derive_id(match i.inner_impl().trait_ { - Some(ref t) => format!("impl-{}", Escape(&format!("{:#}", t))), + Some(ref t) => format!("impl-{}", small_url_encode(&format!("{:#}", t))), None => "impl".to_string(), }); write!(w, "

    {}", @@ -3243,30 +3268,7 @@ fn render_impl(w: &mut fmt::Formatter, cx: &Context, i: &Impl, link: AssocItemLi let render_method_item: bool = match render_mode { RenderMode::Normal => true, - RenderMode::ForDeref { mut_: deref_mut_ } => { - let self_type_opt = match item.inner { - clean::MethodItem(ref method) => method.decl.self_type(), - clean::TyMethodItem(ref method) => method.decl.self_type(), - _ => None - }; - - if let Some(self_ty) = self_type_opt { - let (by_mut_ref, by_box) = match self_ty { - SelfTy::SelfBorrowed(_, mutability) | - SelfTy::SelfExplicit(clean::BorrowedRef { mutability, .. }) => { - (mutability == Mutability::Mutable, false) - }, - SelfTy::SelfExplicit(clean::ResolvedPath { did, .. }) => { - (false, Some(did) == cache().owned_box_did) - }, - _ => (false, false), - }; - - (deref_mut_ || !by_mut_ref) && !by_box - } else { - false - } - }, + RenderMode::ForDeref { mut_: deref_mut_ } => should_render_item(&item, deref_mut_), }; match item.inner { @@ -3513,12 +3515,16 @@ impl<'a> fmt::Display for Sidebar<'a> { } } -fn get_methods(i: &clean::Impl) -> Vec { +fn get_methods(i: &clean::Impl, for_deref: bool) -> Vec { i.items.iter().filter_map(|item| { match item.name { // Maybe check with clean::Visibility::Public as well? Some(ref name) if !name.is_empty() && item.visibility.is_some() && item.is_method() => { - Some(format!("{name}", name = name)) + if !for_deref || should_render_item(item, false) { + Some(format!("{name}", name = name)) + } else { + None + } } _ => None, } @@ -3546,7 +3552,7 @@ fn sidebar_assoc_items(it: &clean::Item) -> String { if let Some(v) = c.impls.get(&it.def_id) { let ret = v.iter() .filter(|i| i.inner_impl().trait_.is_none()) - .flat_map(|i| get_methods(i.inner_impl())) + .flat_map(|i| get_methods(i.inner_impl(), false)) .collect::(); if !ret.is_empty() { out.push_str(&format!("Methods\ @@ -3574,17 +3580,23 @@ fn sidebar_assoc_items(it: &clean::Item) -> String { out.push_str(""); let ret = impls.iter() .filter(|i| i.inner_impl().trait_.is_none()) - .flat_map(|i| get_methods(i.inner_impl())) + .flat_map(|i| get_methods(i.inner_impl(), true)) .collect::(); out.push_str(&format!("
    {}
    ", ret)); } } } + let mut links = HashSet::new(); let ret = v.iter() .filter_map(|i| if let Some(ref i) = i.inner_impl().trait_ { let out = format!("{:#}", i).replace("<", "<").replace(">", ">"); let encoded = small_url_encode(&format!("{:#}", i)); - Some(format!("{}", encoded, out)) + let generated = format!("{}", encoded, out); + if !links.contains(&generated) && links.insert(generated.clone()) { + Some(generated) + } else { + None + } } else { None }) diff --git a/src/tools/linkchecker/main.rs b/src/tools/linkchecker/main.rs index 3ea2e6313af4c..e0153e1e6f67c 100644 --- a/src/tools/linkchecker/main.rs +++ b/src/tools/linkchecker/main.rs @@ -69,15 +69,32 @@ struct FileEntry { type Cache = HashMap; +fn small_url_encode(s: &str) -> String { + s.replace("<", "%3C") + .replace(">", "%3E") + .replace(" ", "%20") + .replace("?", "%3F") + .replace("'", "%27") + .replace("&", "%26") + .replace(",", "%2C") + .replace(":", "%3A") + .replace(";", "%3B") + .replace("[", "%5B") + .replace("]", "%5D") +} + impl FileEntry { fn parse_ids(&mut self, file: &Path, contents: &str, errors: &mut bool) { if self.ids.is_empty() { with_attrs_in_source(contents, " id", |fragment, i, _| { let frag = fragment.trim_left_matches("#").to_owned(); + let encoded = small_url_encode(&frag); if !self.ids.insert(frag) { *errors = true; println!("{}:{}: id is not unique: `{}`", file.display(), i, fragment); } + // Just in case, we also add the encoded id. + self.ids.insert(encoded); }); } }