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.

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

rustdoc: Cleanup associated const value rendering #42286

Merged
merged 1 commit into from
Jun 1, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/librustdoc/clean/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1487,7 +1487,7 @@ pub struct PolyTrait {
/// A representation of a Type suitable for hyperlinking purposes. Ideally one can get the original
/// type out of the AST/TyCtxt given one of these, if more information is needed. Most importantly
/// it does not preserve mutability or boxes.
#[derive(Clone, RustcEncodable, RustcDecodable, PartialEq)]
#[derive(Clone, RustcEncodable, RustcDecodable, PartialEq, Debug)]
pub enum Type {
/// structs/enums/traits (most that'd be an hir::TyPath)
ResolvedPath {
Expand Down
206 changes: 53 additions & 153 deletions src/librustdoc/html/format.rs
Original file line number Diff line number Diff line change
Expand Up @@ -106,16 +106,6 @@ impl<'a, T: fmt::Display> fmt::Display for CommaSep<'a, T> {
}
}

impl<'a, T: fmt::Debug> fmt::Debug for CommaSep<'a, T> {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
for (i, item) in self.0.iter().enumerate() {
if i != 0 { write!(f, ", ")?; }
fmt::Debug::fmt(item, f)?;
}
Ok(())
}
}

impl<'a> fmt::Display for TyParamBounds<'a> {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
let &TyParamBounds(bounds) = self;
Expand Down Expand Up @@ -469,8 +459,7 @@ pub fn href(did: DefId) -> Option<(String, ItemType, Vec<String>)> {
/// Used when rendering a `ResolvedPath` structure. This invokes the `path`
/// rendering function with the necessary arguments for linking to a local path.
fn resolved_path(w: &mut fmt::Formatter, did: DefId, path: &clean::Path,
print_all: bool, use_absolute: bool, is_not_debug: bool,
need_paren: bool) -> fmt::Result {
print_all: bool, use_absolute: bool) -> fmt::Result {
let empty = clean::PathSegment {
name: String::new(),
params: clean::PathParameters::Parenthesized {
Expand Down Expand Up @@ -499,13 +488,9 @@ fn resolved_path(w: &mut fmt::Formatter, did: DefId, path: &clean::Path,
} else {
root.push_str(&seg.name);
root.push_str("/");
if is_not_debug {
Copy link
Member

Choose a reason for hiding this comment

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

I'm pretty surprised that we have the same outcome in here.

Copy link
Member Author

Choose a reason for hiding this comment

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

With w.alternate() == true it does write!(w, "{}::", seg.name)?;, as can be seen above. The is_not_debug == false case did exactly the same thing.

Copy link
Member

Choose a reason for hiding this comment

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

Oh I see!

write!(w, "<a class=\"mod\" href=\"{}index.html\">{}</a>::",
root,
seg.name)?;
} else {
write!(w, "{}::", seg.name)?;
}
write!(w, "<a class=\"mod\" href=\"{}index.html\">{}</a>::",
root,
seg.name)?;
}
}
}
Expand All @@ -517,39 +502,21 @@ fn resolved_path(w: &mut fmt::Formatter, did: DefId, path: &clean::Path,
}
}
if w.alternate() {
if is_not_debug {
write!(w, "{:#}{:#}", HRef::new(did, &last.name), last.params)?;
} else {
write!(w, "{:?}{}", HRef::new(did, &last.name), last.params)?;
}
write!(w, "{:#}{:#}", HRef::new(did, &last.name), last.params)?;
} else {
if is_not_debug {
let path = if use_absolute {
match href(did) {
Some((_, _, fqp)) => format!("{}::{}",
fqp[..fqp.len()-1].join("::"),
HRef::new(did, fqp.last()
.unwrap_or(&String::new()))),
None => format!("{}", HRef::new(did, &last.name)),
let path = if use_absolute {
match href(did) {
Some((_, _, fqp)) => {
format!("{}::{}",
fqp[..fqp.len() - 1].join("::"),
HRef::new(did, fqp.last().unwrap_or(&String::new())))
}
} else {
format!("{}", HRef::new(did, &last.name))
};
write!(w, "{}{}{}", if need_paren { "(" } else { "" }, path, last.params)?;
None => format!("{}", HRef::new(did, &last.name)),
}
} else {
let path = if use_absolute {
match href(did) {
Some((_, _, fqp)) => format!("{:?}::{:?}",
fqp[..fqp.len()-1].join("::"),
HRef::new(did, fqp.last()
.unwrap_or(&String::new()))),
None => format!("{:?}", HRef::new(did, &last.name)),
}
} else {
format!("{:?}", HRef::new(did, &last.name))
};
write!(w, "{}{}{}", if need_paren { "(" } else { "" }, path, last.params)?;
}
format!("{}", HRef::new(did, &last.name))
};
write!(w, "{}{}", path, last.params)?;
}
Ok(())
}
Expand Down Expand Up @@ -600,17 +567,13 @@ fn primitive_link(f: &mut fmt::Formatter,

/// Helper to render type parameters
fn tybounds(w: &mut fmt::Formatter,
typarams: &Option<Vec<clean::TyParamBound>>,
need_paren: bool) -> fmt::Result {
typarams: &Option<Vec<clean::TyParamBound>>) -> fmt::Result {
match *typarams {
Some(ref params) => {
for param in params {
write!(w, " + ")?;
fmt::Display::fmt(param, w)?;
}
if need_paren {
write!(w, ")")?;
}
Ok(())
}
None => Ok(())
Expand All @@ -637,30 +600,18 @@ impl<'a> fmt::Display for HRef<'a> {
}
}

impl<'a> fmt::Debug for HRef<'a> {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
write!(f, "{}", self.text)
}
}

fn fmt_type(t: &clean::Type, f: &mut fmt::Formatter, use_absolute: bool,
is_not_debug: bool, is_ref: bool) -> fmt::Result {
fn fmt_type(t: &clean::Type, f: &mut fmt::Formatter, use_absolute: bool) -> fmt::Result {
match *t {
clean::Generic(ref name) => {
f.write_str(name)
}
clean::ResolvedPath{ did, ref typarams, ref path, is_generic } => {
// Paths like T::Output and Self::Output should be rendered with all segments
let need_paren = match *typarams {
Some(ref v) => !v.is_empty(),
_ => false,
} && is_ref;
resolved_path(f, did, path, is_generic, use_absolute, is_not_debug, need_paren)?;
tybounds(f, typarams, need_paren)
resolved_path(f, did, path, is_generic, use_absolute)?;
tybounds(f, typarams)
}
clean::Infer => write!(f, "_"),
clean::Primitive(prim) if is_not_debug => primitive_link(f, prim, prim.as_str()),
clean::Primitive(prim) => write!(f, "{}", prim.as_str()),
clean::Primitive(prim) => primitive_link(f, prim, prim.as_str()),
clean::BareFunction(ref decl) => {
if f.alternate() {
write!(f, "{}{}fn{:#}{:#}",
Expand All @@ -678,30 +629,26 @@ fn fmt_type(t: &clean::Type, f: &mut fmt::Formatter, use_absolute: bool,
}
clean::Tuple(ref typs) => {
match &typs[..] {
&[] if is_not_debug => primitive_link(f, PrimitiveType::Tuple, "()"),
&[] => write!(f, "()"),
&[ref one] if is_not_debug => {
&[] => primitive_link(f, PrimitiveType::Tuple, "()"),
&[ref one] => {
primitive_link(f, PrimitiveType::Tuple, "(")?;
//carry f.alternate() into this display w/o branching manually
fmt::Display::fmt(one, f)?;
primitive_link(f, PrimitiveType::Tuple, ",)")
}
&[ref one] => write!(f, "({:?},)", one),
many if is_not_debug => {
many => {
primitive_link(f, PrimitiveType::Tuple, "(")?;
fmt::Display::fmt(&CommaSep(&many), f)?;
primitive_link(f, PrimitiveType::Tuple, ")")
}
many => write!(f, "({:?})", &CommaSep(&many)),
}
}
clean::Vector(ref t) if is_not_debug => {
clean::Vector(ref t) => {
primitive_link(f, PrimitiveType::Slice, "[")?;
fmt::Display::fmt(t, f)?;
primitive_link(f, PrimitiveType::Slice, "]")
}
clean::Vector(ref t) => write!(f, "[{:?}]", t),
clean::FixedVector(ref t, ref s) if is_not_debug => {
clean::FixedVector(ref t, ref s) => {
primitive_link(f, PrimitiveType::Array, "[")?;
fmt::Display::fmt(t, f)?;
if f.alternate() {
Expand All @@ -712,17 +659,10 @@ fn fmt_type(t: &clean::Type, f: &mut fmt::Formatter, use_absolute: bool,
&format!("; {}]", Escape(s)))
}
}
clean::FixedVector(ref t, ref s) => {
if f.alternate() {
write!(f, "[{:?}; {}]", t, s)
} else {
write!(f, "[{:?}; {}]", t, Escape(s))
}
}
clean::Never => f.write_str("!"),
clean::RawPointer(m, ref t) => {
match **t {
clean::Generic(_) | clean::ResolvedPath {is_generic: true, ..} if is_not_debug => {
clean::Generic(_) | clean::ResolvedPath {is_generic: true, ..} => {
if f.alternate() {
primitive_link(f, clean::PrimitiveType::RawPointer,
&format!("*{}{:#}", RawMutableSpace(m), t))
Expand All @@ -731,21 +671,11 @@ fn fmt_type(t: &clean::Type, f: &mut fmt::Formatter, use_absolute: bool,
&format!("*{}{}", RawMutableSpace(m), t))
}
}
clean::Generic(_) | clean::ResolvedPath {is_generic: true, ..} => {
if f.alternate() {
write!(f, "*{}{:#?}", RawMutableSpace(m), t)
} else {
write!(f, "*{}{:?}", RawMutableSpace(m), t)
}
}
_ if is_not_debug => {
_ => {
primitive_link(f, clean::PrimitiveType::RawPointer,
&format!("*{}", RawMutableSpace(m)))?;
fmt::Display::fmt(t, f)
}
_ => {
write!(f, "*{}{:?}", RawMutableSpace(m), t)
}
}
}
clean::BorrowedRef{ lifetime: ref l, mutability, type_: ref ty} => {
Expand All @@ -757,7 +687,7 @@ fn fmt_type(t: &clean::Type, f: &mut fmt::Formatter, use_absolute: bool,
match **ty {
clean::Vector(ref bt) => { // BorrowedRef{ ... Vector(T) } is &[T]
match **bt {
clean::Generic(_) if is_not_debug => {
clean::Generic(_) => {
if f.alternate() {
primitive_link(f, PrimitiveType::Slice,
&format!("&{}{}[{:#}]", lt, m, **bt))
Expand All @@ -766,14 +696,7 @@ fn fmt_type(t: &clean::Type, f: &mut fmt::Formatter, use_absolute: bool,
&format!("&amp;{}{}[{}]", lt, m, **bt))
}
}
clean::Generic(_) => {
if f.alternate() {
write!(f, "&{}{}[{:#?}]", lt, m, **bt)
} else {
write!(f, "&{}{}[{:?}]", lt, m, **bt)
}
}
_ if is_not_debug => {
_ => {
if f.alternate() {
primitive_link(f, PrimitiveType::Slice,
&format!("&{}{}[", lt, m))?;
Expand All @@ -785,26 +708,25 @@ fn fmt_type(t: &clean::Type, f: &mut fmt::Formatter, use_absolute: bool,
}
primitive_link(f, PrimitiveType::Slice, "]")
}
_ => {
if f.alternate() {
write!(f, "&{}{}[{:#?}]", lt, m, **bt)
} else {
write!(f, "&{}{}[{:?}]", lt, m, **bt)
}
}
}
}
clean::ResolvedPath { typarams: Some(ref v), .. } if !v.is_empty() => {
if f.alternate() {
write!(f, "&{}{}", lt, m)?;
} else {
write!(f, "&amp;{}{}", lt, m)?;
}
write!(f, "(")?;
Copy link
Member

Choose a reason for hiding this comment

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

Oh I see. Better indeed!

fmt_type(&ty, f, use_absolute)?;
write!(f, ")")
}
_ => {
if f.alternate() {
write!(f, "&{}{}", lt, m)?;
fmt_type(&ty, f, use_absolute, is_not_debug, true)
fmt_type(&ty, f, use_absolute)
} else {
if is_not_debug {
write!(f, "&amp;{}{}", lt, m)?;
} else {
write!(f, "&{}{}", lt, m)?;
}
fmt_type(&ty, f, use_absolute, is_not_debug, true)
write!(f, "&amp;{}{}", lt, m)?;
fmt_type(&ty, f, use_absolute)
}
}
}
Expand Down Expand Up @@ -833,32 +755,16 @@ fn fmt_type(t: &clean::Type, f: &mut fmt::Formatter, use_absolute: bool,
_ => true,
};
if f.alternate() {
if is_not_debug {
if should_show_cast {
write!(f, "<{:#} as {:#}>::", self_type, trait_)?
} else {
write!(f, "{:#}::", self_type)?
}
if should_show_cast {
Copy link
Member

Choose a reason for hiding this comment

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

Once again I'm surprised the code does the same thing.

Copy link
Member Author

Choose a reason for hiding this comment

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

The is_not_debug == false case was:

if should_show_cast {
    write!(f, "<{:?} as {:?}>::", self_type, trait_)?
} else {
    write!(f, "{:?}::", self_type)?
}

The f.alternate() == true case is:

if should_show_cast {
    write!(f, "<{:#} as {:#}>::", self_type, trait_)?
} else {
    write!(f, "{:#}::", self_type)?
}

write!(f, "<{:#} as {:#}>::", self_type, trait_)?
} else {
if should_show_cast {
write!(f, "<{:#?} as {:#?}>::", self_type, trait_)?
} else {
write!(f, "{:#?}::", self_type)?
}
write!(f, "{:#}::", self_type)?
}
} else {
if is_not_debug {
if should_show_cast {
write!(f, "&lt;{} as {}&gt;::", self_type, trait_)?
} else {
write!(f, "{}::", self_type)?
}
if should_show_cast {
write!(f, "&lt;{} as {}&gt;::", self_type, trait_)?
} else {
if should_show_cast {
write!(f, "<{:?} as {:?}>::", self_type, trait_)?
} else {
write!(f, "{:?}::", self_type)?
}
write!(f, "{}::", self_type)?
}
};
match *trait_ {
Expand All @@ -874,7 +780,7 @@ fn fmt_type(t: &clean::Type, f: &mut fmt::Formatter, use_absolute: bool,
// look at).
box clean::ResolvedPath { did, ref typarams, .. } => {
let path = clean::Path::singleton(name.clone());
resolved_path(f, did, &path, true, use_absolute, is_not_debug, false)?;
resolved_path(f, did, &path, true, use_absolute)?;

// FIXME: `typarams` are not rendered, and this seems bad?
drop(typarams);
Expand All @@ -893,13 +799,7 @@ fn fmt_type(t: &clean::Type, f: &mut fmt::Formatter, use_absolute: bool,

impl fmt::Display for clean::Type {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
fmt_type(self, f, false, true, false)
}
}

impl fmt::Debug for clean::Type {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
fmt_type(self, f, false, false, false)
fmt_type(self, f, false)
}
}

Expand Down Expand Up @@ -933,7 +833,7 @@ fn fmt_impl(i: &clean::Impl,
write!(f, " for ")?;
}

fmt_type(&i.for_, f, use_absolute, true, false)?;
fmt_type(&i.for_, f, use_absolute)?;

fmt::Display::fmt(&WhereClause { gens: &i.generics, indent: 0, end_newline: true }, f)?;
Ok(())
Expand Down Expand Up @@ -1139,7 +1039,7 @@ impl fmt::Display for clean::Import {
impl fmt::Display for clean::ImportSource {
fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
match self.did {
Some(did) => resolved_path(f, did, &self.path, true, false, true, false),
Some(did) => resolved_path(f, did, &self.path, true, false),
_ => {
for (i, seg) in self.path.segments.iter().enumerate() {
if i > 0 {
Expand Down
4 changes: 2 additions & 2 deletions src/librustdoc/html/render.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1662,9 +1662,9 @@ fn md_render_assoc_item(item: &clean::Item) -> String {
match item.inner {
clean::AssociatedConstItem(ref ty, ref default) => {
if let Some(default) = default.as_ref() {
format!("```\n{}: {:?} = {}\n```\n\n", item.name.as_ref().unwrap(), ty, default)
format!("```\n{}: {:#} = {}\n```\n\n", item.name.as_ref().unwrap(), ty, default)
} else {
format!("```\n{}: {:?}\n```\n\n", item.name.as_ref().unwrap(), ty)
format!("```\n{}: {:#}\n```\n\n", item.name.as_ref().unwrap(), ty)
}
}
_ => String::new(),
Expand Down
Loading