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: simplify highlight.rs #99337

Merged
merged 1 commit into from
Aug 12, 2022
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
88 changes: 55 additions & 33 deletions src/librustdoc/html/highlight.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,48 +33,80 @@ pub(crate) struct HrefContext<'a, 'b, 'c> {

/// Decorations are represented as a map from CSS class to vector of character ranges.
/// Each range will be wrapped in a span with that class.
#[derive(Default)]
pub(crate) struct DecorationInfo(pub(crate) FxHashMap<&'static str, Vec<(u32, u32)>>);

/// Highlights `src`, returning the HTML output.
pub(crate) fn render_with_highlighting(
#[derive(Eq, PartialEq, Clone, Copy)]
pub(crate) enum Tooltip {
Ignore,
CompileFail,
ShouldPanic,
Edition(Edition),
None,
}

/// Highlights `src` as an inline example, returning the HTML output.
pub(crate) fn render_example_with_highlighting(
Copy link
Member

Choose a reason for hiding this comment

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

This new distinction is really nice!

src: &str,
out: &mut Buffer,
class: Option<&str>,
tooltip: Tooltip,
playground_button: Option<&str>,
tooltip: Option<(Option<Edition>, &str)>,
edition: Edition,
extra_content: Option<Buffer>,
href_context: Option<HrefContext<'_, '_, '_>>,
decoration_info: Option<DecorationInfo>,
) {
debug!("highlighting: ================\n{}\n==============", src);
if let Some((edition_info, class)) = tooltip {
let class = match tooltip {
Tooltip::Ignore => " ignore",
Tooltip::CompileFail => " compile_fail",
Tooltip::ShouldPanic => " should_panic",
Tooltip::Edition(_) => " edition",
Tooltip::None => "",
};

if tooltip != Tooltip::None {
write!(
out,
"<div class='information'><div class='tooltip {}'{}>ⓘ</div></div>",
"<div class='information'><div class='tooltip{}'{}>ⓘ</div></div>",
class,
if let Some(edition_info) = edition_info {
if let Tooltip::Edition(edition_info) = tooltip {
format!(" data-edition=\"{}\"", edition_info)
} else {
String::new()
},
);
}

write_header(out, class, extra_content);
write_code(out, src, edition, href_context, decoration_info);
write_header(out, &format!("rust-example-rendered{}", class), None);
write_code(out, src, None, None);
write_footer(out, playground_button);
}

fn write_header(out: &mut Buffer, class: Option<&str>, extra_content: Option<Buffer>) {
/// Highlights `src` as a macro, returning the HTML output.
pub(crate) fn render_macro_with_highlighting(src: &str, out: &mut Buffer) {
write_header(out, "macro", None);
write_code(out, src, None, None);
write_footer(out, None);
}

/// Highlights `src` as a source code page, returning the HTML output.
pub(crate) fn render_source_with_highlighting(
src: &str,
out: &mut Buffer,
line_numbers: Buffer,
href_context: HrefContext<'_, '_, '_>,
decoration_info: DecorationInfo,
) {
write_header(out, "", Some(line_numbers));
write_code(out, src, Some(href_context), Some(decoration_info));
write_footer(out, None);
}

fn write_header(out: &mut Buffer, class: &str, extra_content: Option<Buffer>) {
write!(out, "<div class=\"example-wrap\">");
if let Some(extra) = extra_content {
out.push_buffer(extra);
}
if let Some(class) = class {
write!(out, "<pre class=\"rust {}\">", class);
} else {
if class.is_empty() {
Copy link
Member

Choose a reason for hiding this comment

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

This seems less good. Having to check if a string is empty is easy to forget whereas you cannot make the mistake with Option. Does it make sense?

write!(out, "<pre class=\"rust\">");
} else {
write!(out, "<pre class=\"rust {}\">", class);
}
write!(out, "<code>");
}
Expand All @@ -93,7 +125,6 @@ fn write_header(out: &mut Buffer, class: Option<&str>, extra_content: Option<Buf
fn write_code(
out: &mut Buffer,
src: &str,
edition: Edition,
href_context: Option<HrefContext<'_, '_, '_>>,
decoration_info: Option<DecorationInfo>,
) {
Expand All @@ -102,7 +133,6 @@ fn write_code(
let mut closing_tags: Vec<&'static str> = Vec::new();
Classifier::new(
&src,
edition,
href_context.as_ref().map(|c| c.file_span).unwrap_or(DUMMY_SP),
decoration_info,
)
Expand Down Expand Up @@ -220,7 +250,7 @@ impl<'a> Iterator for TokenIter<'a> {
}

/// Classifies into identifier class; returns `None` if this is a non-keyword identifier.
fn get_real_ident_class(text: &str, edition: Edition, allow_path_keywords: bool) -> Option<Class> {
fn get_real_ident_class(text: &str, allow_path_keywords: bool) -> Option<Class> {
let ignore: &[&str] =
if allow_path_keywords { &["self", "Self", "super", "crate"] } else { &["self", "Self"] };
if ignore.iter().any(|k| *k == text) {
Expand All @@ -229,7 +259,7 @@ fn get_real_ident_class(text: &str, edition: Edition, allow_path_keywords: bool)
Some(match text {
"ref" | "mut" => Class::RefKeyWord,
"false" | "true" => Class::Bool,
_ if Symbol::intern(text).is_reserved(|| edition) => Class::KeyWord,
_ if Symbol::intern(text).is_reserved(|| Edition::Edition2021) => Class::KeyWord,
Copy link
Member

Choose a reason for hiding this comment

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

This is quite a big change. Until now the highlight depended on the edition provided to rustdoc. Even though it is a welcomed simplification, I'm not too sure if it's a good idea...

Another problem I have with this approach: we will need to not forget to update it whenever the edition is updated. To go around that limitation, you could add a current method on Edition which would return the current highest edition. To be discussed I suppose.

Copy link
Member

Choose a reason for hiding this comment

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

Also, it seems it's not taking into account the edition parameter that the users can use on codeblocks.

_ => return None,
})
}
Expand Down Expand Up @@ -311,7 +341,6 @@ struct Classifier<'a> {
in_attribute: bool,
in_macro: bool,
in_macro_nonterminal: bool,
edition: Edition,
byte_pos: u32,
file_span: Span,
src: &'a str,
Expand All @@ -321,20 +350,14 @@ struct Classifier<'a> {
impl<'a> Classifier<'a> {
/// Takes as argument the source code to HTML-ify, the rust edition to use and the source code
/// file span which will be used later on by the `span_correspondance_map`.
fn new(
src: &str,
edition: Edition,
file_span: Span,
decoration_info: Option<DecorationInfo>,
) -> Classifier<'_> {
fn new(src: &str, file_span: Span, decoration_info: Option<DecorationInfo>) -> Classifier<'_> {
let tokens = PeekIter::new(TokenIter { src });
let decorations = decoration_info.map(Decorations::new);
Classifier {
tokens,
in_attribute: false,
in_macro: false,
in_macro_nonterminal: false,
edition,
byte_pos: 0,
file_span,
src,
Expand All @@ -354,7 +377,6 @@ impl<'a> Classifier<'a> {
let start = self.byte_pos as usize;
let mut pos = start;
let mut has_ident = false;
let edition = self.edition;

loop {
let mut nb = 0;
Expand All @@ -376,7 +398,7 @@ impl<'a> Classifier<'a> {

if let Some((None, text)) = self.tokens.peek().map(|(token, text)| {
if *token == TokenKind::Ident {
let class = get_real_ident_class(text, edition, true);
let class = get_real_ident_class(text, true);
(class, text)
} else {
// Doesn't matter which Class we put in here...
Expand Down Expand Up @@ -634,7 +656,7 @@ impl<'a> Classifier<'a> {
sink(Highlight::Token { text, class: None });
return;
}
TokenKind::Ident => match get_real_ident_class(text, self.edition, false) {
TokenKind::Ident => match get_real_ident_class(text, false) {
None => match text {
"Option" | "Result" => Class::PreludeTy,
"Some" | "None" | "Ok" | "Err" => Class::PreludeVal,
Expand Down
11 changes: 5 additions & 6 deletions src/librustdoc/html/highlight/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ use crate::html::format::Buffer;
use expect_test::expect_file;
use rustc_data_structures::fx::FxHashMap;
use rustc_span::create_default_session_globals_then;
use rustc_span::edition::Edition;

const STYLE: &str = r#"
<style>
Expand All @@ -23,7 +22,7 @@ fn test_html_highlighting() {
let src = include_str!("fixtures/sample.rs");
let html = {
let mut out = Buffer::new();
write_code(&mut out, src, Edition::Edition2018, None, None);
write_code(&mut out, src, None, None);
format!("{}<pre><code>{}</code></pre>\n", STYLE, out.into_inner())
};
expect_file!["fixtures/sample.html"].assert_eq(&html);
Expand All @@ -37,7 +36,7 @@ fn test_dos_backline() {
println!(\"foo\");\r\n\
}\r\n";
let mut html = Buffer::new();
write_code(&mut html, src, Edition::Edition2018, None, None);
write_code(&mut html, src, None, None);
expect_file!["fixtures/dos_line.html"].assert_eq(&html.into_inner());
});
}
Expand All @@ -51,7 +50,7 @@ let x = super::b::foo;
let y = Self::whatever;";

let mut html = Buffer::new();
write_code(&mut html, src, Edition::Edition2018, None, None);
write_code(&mut html, src, None, None);
expect_file!["fixtures/highlight.html"].assert_eq(&html.into_inner());
});
}
Expand All @@ -61,7 +60,7 @@ fn test_union_highlighting() {
create_default_session_globals_then(|| {
let src = include_str!("fixtures/union.rs");
let mut html = Buffer::new();
write_code(&mut html, src, Edition::Edition2018, None, None);
write_code(&mut html, src, None, None);
expect_file!["fixtures/union.html"].assert_eq(&html.into_inner());
});
}
Expand All @@ -75,7 +74,7 @@ let y = 2;";
decorations.insert("example", vec![(0, 10)]);

let mut html = Buffer::new();
write_code(&mut html, src, Edition::Edition2018, None, Some(DecorationInfo(decorations)));
write_code(&mut html, src, None, Some(DecorationInfo(decorations)));
expect_file!["fixtures/decorations.html"].assert_eq(&html.into_inner());
});
}
23 changes: 8 additions & 15 deletions src/librustdoc/html/markdown.rs
Original file line number Diff line number Diff line change
Expand Up @@ -330,34 +330,27 @@ impl<'a, I: Iterator<Item = Event<'a>>> Iterator for CodeBlocks<'_, 'a, I> {
});

let tooltip = if ignore != Ignore::None {
Some((None, "ignore"))
highlight::Tooltip::Ignore
} else if compile_fail {
Some((None, "compile_fail"))
highlight::Tooltip::CompileFail
} else if should_panic {
Some((None, "should_panic"))
highlight::Tooltip::ShouldPanic
} else if explicit_edition {
Some((Some(edition), "edition"))
highlight::Tooltip::Edition(edition)
} else {
None
highlight::Tooltip::None
};

// insert newline to clearly separate it from the
// previous block so we can shorten the html output
let mut s = Buffer::new();
s.push_str("\n");
highlight::render_with_highlighting(

highlight::render_example_with_highlighting(
&text,
&mut s,
Some(&format!(
"rust-example-rendered{}",
if let Some((_, class)) = tooltip { format!(" {}", class) } else { String::new() }
)),
playground_button.as_deref(),
tooltip,
edition,
None,
None,
None,
playground_button.as_deref(),
);
Some(Event::Html(s.into_inner().into()))
}
Expand Down
3 changes: 1 addition & 2 deletions src/librustdoc/html/render/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2764,11 +2764,10 @@ fn render_call_locations(w: &mut Buffer, cx: &mut Context<'_>, item: &clean::Ite
sources::print_src(
w,
contents_subset,
call_data.edition,
file_span,
cx,
&root_path,
Some(highlight::DecorationInfo(decoration_info)),
highlight::DecorationInfo(decoration_info),
sources::SourceContext::Embedded { offset: line_min },
);
write!(w, "</div></div>");
Expand Down
12 changes: 1 addition & 11 deletions src/librustdoc/html/render/print_item.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1320,17 +1320,7 @@ fn item_enum(w: &mut Buffer, cx: &mut Context<'_>, it: &clean::Item, e: &clean::

fn item_macro(w: &mut Buffer, cx: &mut Context<'_>, it: &clean::Item, t: &clean::Macro) {
wrap_into_docblock(w, |w| {
highlight::render_with_highlighting(
&t.source,
w,
Some("macro"),
None,
None,
it.span(cx.tcx()).inner().edition(),
None,
None,
None,
);
highlight::render_macro_with_highlighting(&t.source, w);
});
document(w, cx, it, None, HeadingOffset::H2)
}
Expand Down
17 changes: 5 additions & 12 deletions src/librustdoc/html/sources.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ use rustc_data_structures::fx::{FxHashMap, FxHashSet};
use rustc_hir::def_id::LOCAL_CRATE;
use rustc_middle::ty::TyCtxt;
use rustc_session::Session;
use rustc_span::edition::Edition;
use rustc_span::source_map::FileName;

use std::ffi::OsStr;
Expand Down Expand Up @@ -213,11 +212,10 @@ impl SourceCollector<'_, '_> {
print_src(
buf,
contents,
cx.shared.edition(),
file_span,
cx,
&root_path,
None,
highlight::DecorationInfo::default(),
Copy link
Member

Choose a reason for hiding this comment

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

Even though it could be simplified into Default::default(), I think it's personally much better this way. :)

SourceContext::Standalone,
)
},
Expand Down Expand Up @@ -266,11 +264,10 @@ pub(crate) enum SourceContext {
pub(crate) fn print_src(
buf: &mut Buffer,
s: &str,
edition: Edition,
file_span: rustc_span::Span,
context: &Context<'_>,
root_path: &str,
decoration_info: Option<highlight::DecorationInfo>,
decoration_info: highlight::DecorationInfo,
source_context: SourceContext,
) {
let lines = s.lines().count();
Expand All @@ -289,15 +286,11 @@ pub(crate) fn print_src(
}
}
line_numbers.write_str("</pre>");
highlight::render_with_highlighting(
highlight::render_source_with_highlighting(
s,
buf,
None,
None,
None,
edition,
Some(line_numbers),
Some(highlight::HrefContext { context, file_span, root_path }),
line_numbers,
highlight::HrefContext { context, file_span, root_path },
decoration_info,
);
}