-
Notifications
You must be signed in to change notification settings - Fork 14k
fix(rustdoc): Color doctest errors #148834
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 all commits
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 |
|---|---|---|
|
|
@@ -8,8 +8,8 @@ use std::sync::Arc; | |
| use rustc_ast::token::{Delimiter, TokenKind}; | ||
| use rustc_ast::tokenstream::TokenTree; | ||
| use rustc_ast::{self as ast, AttrStyle, HasAttrs, StmtKind}; | ||
| use rustc_errors::emitter::stderr_destination; | ||
| use rustc_errors::{AutoStream, ColorConfig, DiagCtxtHandle}; | ||
| use rustc_errors::emitter::get_stderr_color_choice; | ||
| use rustc_errors::{AutoStream, ColorChoice, ColorConfig, DiagCtxtHandle}; | ||
| use rustc_parse::lexer::StripTokens; | ||
| use rustc_parse::new_parser_from_source_str; | ||
| use rustc_session::parse::ParseSess; | ||
|
|
@@ -446,7 +446,7 @@ fn parse_source( | |
| span: Span, | ||
| ) -> Result<ParseSourceInfo, ()> { | ||
| use rustc_errors::DiagCtxt; | ||
| use rustc_errors::emitter::{Emitter, HumanEmitter}; | ||
| use rustc_errors::emitter::HumanEmitter; | ||
| use rustc_span::source_map::FilePathMapping; | ||
|
|
||
| let mut info = | ||
|
|
@@ -458,9 +458,12 @@ fn parse_source( | |
|
|
||
| let sm = Arc::new(SourceMap::new(FilePathMapping::empty())); | ||
| let translator = rustc_driver::default_translator(); | ||
| info.supports_color = | ||
| HumanEmitter::new(stderr_destination(ColorConfig::Auto), translator.clone()) | ||
| .supports_color(); | ||
| let supports_color = match get_stderr_color_choice(ColorConfig::Auto, &std::io::stderr()) { | ||
|
Member
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 should've realized that much earlier: Obviously, like on stable, I consider this a bug but I don't want it to be fixed in this PR since there are some subtleties. E.g., if you pass |
||
| ColorChoice::Auto => unreachable!(), | ||
| ColorChoice::AlwaysAnsi | ColorChoice::Always => true, | ||
| ColorChoice::Never => false, | ||
| }; | ||
| info.supports_color = supports_color; | ||
| // Any errors in parsing should also appear when the doctest is compiled for real, so just | ||
| // send all the errors that the parser emits directly into a `Sink` instead of stderr. | ||
| let emitter = HumanEmitter::new(AutoStream::never(Box::new(io::sink())), translator); | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.
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.
I'll provide some more context for other T-rustdoc members. In my opinion
to_color_choiceas used in both rustc & rustdoc is suboptimal as it doesn't harness the full power of crateanstyle-querydue to the fact that it yieldsNeverif the config isAutoand stderr is not a terminal meaningNO_COLOR=1works but e.g.,CLICOLOR_FORCE=1doesn't.That's a separate bug I or Muscraft will deal with in the future, hopefully.
I'm saying all that because I originally wanted to use
CLICOLOR_FORCE=1in a potential regression test for #148749 which requires--color=auto(not--color=always) which obviously never leads to colored output since it's captured by compiletest, not by the terminal. I wanted to useCLICOLOR_FORCE=1to indirectly test--color=auto(and have the golden file be an SVG or ANSI codes if that doesn't work out).That's the reason why there's no regression test. If you have any other ideas, please let me know.