-
-
Notifications
You must be signed in to change notification settings - Fork 14.9k
Simplify code to generate line numbers in highlight #148171
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
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -343,7 +343,7 @@ struct TokenHandler<'a, 'tcx, F: Write> { | |||||
| /// We need to keep the `Class` for each element because it could contain a `Span` which is | ||||||
| /// used to generate links. | ||||||
| href_context: Option<HrefContext<'a, 'tcx>>, | ||||||
| write_line_number: fn(u32) -> String, | ||||||
| write_line_number: LineNumberKind, | ||||||
| line: u32, | ||||||
| max_lines: u32, | ||||||
| } | ||||||
|
|
@@ -355,10 +355,10 @@ impl<F: Write> std::fmt::Debug for TokenHandler<'_, '_, F> { | |||||
| } | ||||||
|
|
||||||
| impl<'a, F: Write> TokenHandler<'a, '_, F> { | ||||||
| fn handle_backline(&mut self) -> Option<String> { | ||||||
| fn handle_backline(&mut self) -> Option<impl Display + use<F>> { | ||||||
| self.line += 1; | ||||||
| if self.line < self.max_lines { | ||||||
| return Some((self.write_line_number)(self.line)); | ||||||
| return Some(self.write_line_number.render(self.line)); | ||||||
| } | ||||||
| None | ||||||
| } | ||||||
|
|
@@ -376,8 +376,7 @@ impl<'a, F: Write> TokenHandler<'a, '_, F> { | |||||
| if text == "\n" | ||||||
| && let Some(backline) = self.handle_backline() | ||||||
| { | ||||||
| self.out.write_str(&text).unwrap(); | ||||||
| self.out.write_str(&backline).unwrap(); | ||||||
| write!(self.out, "{text}{backline}").unwrap(); | ||||||
| } else { | ||||||
| self.push_token_without_backline_check(class, text, true); | ||||||
| } | ||||||
|
|
@@ -437,20 +436,29 @@ impl<F: Write> Drop for TokenHandler<'_, '_, F> { | |||||
| } | ||||||
| } | ||||||
|
|
||||||
| fn scraped_line_number(line: u32) -> String { | ||||||
| // https://developers.google.com/search/docs/crawling-indexing/robots-meta-tag#data-nosnippet-attr | ||||||
| // Do not show "1 2 3 4 5 ..." in web search results. | ||||||
| format!("<span data-nosnippet>{line}</span>") | ||||||
| } | ||||||
|
|
||||||
| fn line_number(line: u32) -> String { | ||||||
| // https://developers.google.com/search/docs/crawling-indexing/robots-meta-tag#data-nosnippet-attr | ||||||
| // Do not show "1 2 3 4 5 ..." in web search results. | ||||||
| format!("<a href=#{line} id={line} data-nosnippet>{line}</a>") | ||||||
| /// Represents line numbers to be generated as HTML. | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit:
Suggested change
maybe?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agreed, adding it. :) |
||||||
| #[derive(Clone, Copy)] | ||||||
| enum LineNumberKind { | ||||||
| /// Used for scraped code examples. | ||||||
| Scraped, | ||||||
| /// Used for source code pages. | ||||||
| Normal, | ||||||
| /// Code examples in documentation don't have line number generated by rustdoc. | ||||||
| Empty, | ||||||
| } | ||||||
|
|
||||||
| fn empty_line_number(_: u32) -> String { | ||||||
| String::new() | ||||||
| impl LineNumberKind { | ||||||
| fn render(self, line: u32) -> impl Display { | ||||||
| fmt::from_fn(move |f| { | ||||||
| match self { | ||||||
| // https://developers.google.com/search/docs/crawling-indexing/robots-meta-tag#data-nosnippet-attr | ||||||
| // Do not show "1 2 3 4 5 ..." in web search results. | ||||||
| Self::Scraped => write!(f, "<span data-nosnippet>{line}</span>"), | ||||||
| Self::Normal => write!(f, "<a href=#{line} id={line} data-nosnippet>{line}</a>"), | ||||||
| Self::Empty => Ok(()), | ||||||
| } | ||||||
| }) | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| fn get_next_expansion( | ||||||
|
|
@@ -537,12 +545,12 @@ pub(super) fn write_code( | |||||
| write_line_number: match line_info { | ||||||
| Some(line_info) => { | ||||||
| if line_info.is_scraped_example { | ||||||
| scraped_line_number | ||||||
| LineNumberKind::Scraped | ||||||
| } else { | ||||||
| line_number | ||||||
| LineNumberKind::Normal | ||||||
| } | ||||||
| } | ||||||
| None => empty_line_number, | ||||||
| None => LineNumberKind::Empty, | ||||||
| }, | ||||||
| line: 0, | ||||||
| max_lines: u32::MAX, | ||||||
|
|
@@ -552,8 +560,12 @@ pub(super) fn write_code( | |||||
| if let Some(line_info) = line_info { | ||||||
| token_handler.line = line_info.start_line - 1; | ||||||
| token_handler.max_lines = line_info.max_lines; | ||||||
| if let Some(text) = token_handler.handle_backline() { | ||||||
| token_handler.push_token_without_backline_check(None, Cow::Owned(text), false); | ||||||
| if let Some(backline) = token_handler.handle_backline() { | ||||||
| token_handler.push_token_without_backline_check( | ||||||
| None, | ||||||
| Cow::Owned(backline.to_string()), | ||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The fact that we're using Actually, I think it could even be just
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I mean, I think Regardless, the lifetime still is overly restrictive, and it's hard to see that as anything but a mistake.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hum, might be worth trying in a follow-up. |
||||||
| false, | ||||||
| ); | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
|
|
||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good idea.