Skip to content

Commit 525e191

Browse files
committed
Rewrite show_md_content_with_pager.
I think the control flow in this function is complicated and confusing, largely due to the use of two booleans `print_formatted` and `fallback_to_println` that are set in multiple places and then used to guide proceedings. As well as hurting readability, this leads to at least one bug: if the `write_termcolor_buf` call fails and the pager also fails, the function will try to print color output to stdout, but that output will be empty because `write_termcolor_buf` failed. I.e. the `if fallback_to_println` body fails to check `print_formatted`. This commit rewrites the function to be neater and more Rust-y, e.g. by putting the result of `write_termcolor_buf` into an `Option` so it can only be used on success, and by using `?` more. It also changes terminology a little, using "pretty" to mean "formatted and colorized". The result is a little shorter, more readable, and less buggy.
1 parent 9116495 commit 525e191

File tree

1 file changed

+43
-49
lines changed
  • compiler/rustc_driver_impl/src

1 file changed

+43
-49
lines changed

Diff for: compiler/rustc_driver_impl/src/lib.rs

+43-49
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
#![feature(panic_update_hook)]
1616
#![feature(result_flattening)]
1717
#![feature(rustdoc_internals)]
18+
#![feature(try_blocks)]
1819
#![warn(unreachable_pub)]
1920
// tidy-alphabetical-end
2021

@@ -564,70 +565,63 @@ fn handle_explain(early_dcx: &EarlyDiagCtxt, registry: Registry, code: &str, col
564565
}
565566
}
566567

567-
/// If color is always or auto, print formatted & colorized markdown. If color is never or
568-
/// if formatted printing fails, print the raw text.
568+
/// If `color` is `always` or `auto`, try to print pretty (formatted & colorized) markdown. If
569+
/// that fails or `color` is `never`, print the raw markdown.
569570
///
570-
/// Prefers a pager, falls back standard print
571+
/// Uses a pager if possible, falls back to stdout.
571572
fn show_md_content_with_pager(content: &str, color: ColorConfig) {
572-
let mut fallback_to_println = false;
573573
let pager_name = env::var_os("PAGER").unwrap_or_else(|| {
574574
if cfg!(windows) { OsString::from("more.com") } else { OsString::from("less") }
575575
});
576576

577577
let mut cmd = Command::new(&pager_name);
578-
// FIXME: find if other pagers accept color options
579-
let mut print_formatted = if pager_name == "less" {
580-
cmd.arg("-R");
581-
true
582-
} else {
583-
["bat", "batcat", "delta"].iter().any(|v| *v == pager_name)
584-
};
585-
586-
if color == ColorConfig::Never {
587-
print_formatted = false;
588-
} else if color == ColorConfig::Always {
589-
print_formatted = true;
578+
if pager_name == "less" {
579+
cmd.arg("-R"); // allows color escape sequences
590580
}
591581

592-
let mdstream = markdown::MdStream::parse_str(content);
593-
let bufwtr = markdown::create_stdout_bufwtr();
594-
let mut mdbuf = bufwtr.buffer();
595-
if mdstream.write_termcolor_buf(&mut mdbuf).is_err() {
596-
print_formatted = false;
597-
}
598-
599-
if let Ok(mut pager) = cmd.stdin(Stdio::piped()).spawn() {
600-
if let Some(pipe) = pager.stdin.as_mut() {
601-
let res = if print_formatted {
602-
pipe.write_all(mdbuf.as_slice())
603-
} else {
604-
pipe.write_all(content.as_bytes())
605-
};
606-
607-
if res.is_err() {
608-
fallback_to_println = true;
609-
}
582+
let pretty_on_pager = match color {
583+
ColorConfig::Auto => {
584+
// Add other pagers that accept color escape sequences here.
585+
["less", "bat", "batcat", "delta"].iter().any(|v| *v == pager_name)
610586
}
587+
ColorConfig::Always => true,
588+
ColorConfig::Never => false,
589+
};
611590

612-
if pager.wait().is_err() {
613-
fallback_to_println = true;
614-
}
615-
} else {
616-
fallback_to_println = true;
617-
}
591+
// Try to prettify the raw markdown text. The result can be used by the pager or on stdout.
592+
let pretty_data = {
593+
let mdstream = markdown::MdStream::parse_str(content);
594+
let bufwtr = markdown::create_stdout_bufwtr();
595+
let mut mdbuf = bufwtr.buffer();
596+
if mdstream.write_termcolor_buf(&mut mdbuf).is_ok() { Some((bufwtr, mdbuf)) } else { None }
597+
};
618598

619-
// If pager fails for whatever reason, we should still print the content
620-
// to standard output
621-
if fallback_to_println {
622-
let fmt_success = match color {
623-
ColorConfig::Auto | ColorConfig::Always => bufwtr.print(&mdbuf).is_ok(),
624-
ColorConfig::Never => false,
599+
// Try to print via the pager, pretty output if possible.
600+
let pager_res: Option<()> = try {
601+
let mut pager = cmd.stdin(Stdio::piped()).spawn().ok()?;
602+
603+
let pager_stdin = pager.stdin.as_mut()?;
604+
if pretty_on_pager && let Some((_, mdbuf)) = &pretty_data {
605+
pager_stdin.write_all(mdbuf.as_slice()).ok()?;
606+
} else {
607+
pager_stdin.write_all(content.as_bytes()).ok()?;
625608
};
626609

627-
if !fmt_success {
628-
safe_print!("{content}");
629-
}
610+
pager.wait().ok()?;
611+
};
612+
if pager_res.is_some() {
613+
return;
630614
}
615+
616+
// The pager failed. Try to print pretty output to stdout.
617+
if let Some((bufwtr, mdbuf)) = &pretty_data
618+
&& bufwtr.print(&mdbuf).is_ok()
619+
{
620+
return;
621+
}
622+
623+
// Everything failed. Print the raw markdown text.
624+
safe_print!("{content}");
631625
}
632626

633627
fn process_rlink(sess: &Session, compiler: &interface::Compiler) {

0 commit comments

Comments
 (0)