From 15528b2d6c2d7d83d007440ab84d859bc60ab40d Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 20 Nov 2024 10:34:42 +1100 Subject: [PATCH 1/3] Fix `catbat` pager typo. `bat` is known as `batcat` on Ubuntu and Debian, not `catbat`. --- compiler/rustc_driver_impl/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/compiler/rustc_driver_impl/src/lib.rs b/compiler/rustc_driver_impl/src/lib.rs index 8dd043be6ad3c..038a0d6c2883d 100644 --- a/compiler/rustc_driver_impl/src/lib.rs +++ b/compiler/rustc_driver_impl/src/lib.rs @@ -580,7 +580,7 @@ fn show_md_content_with_pager(content: &str, color: ColorConfig) { cmd.arg("-R"); true } else { - ["bat", "catbat", "delta"].iter().any(|v| *v == pager_name) + ["bat", "batcat", "delta"].iter().any(|v| *v == pager_name) }; if color == ColorConfig::Never { From 91164957ec18aa65aa00412f703f2270e47779c1 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 20 Nov 2024 14:10:44 +1100 Subject: [PATCH 2/3] Remove redundant `is_terminal` check. It's not necessary because `show_md_content_with_pager` is only ever called if `is_terminal` is true. --- compiler/rustc_driver_impl/src/lib.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/compiler/rustc_driver_impl/src/lib.rs b/compiler/rustc_driver_impl/src/lib.rs index 038a0d6c2883d..6198f40e83d95 100644 --- a/compiler/rustc_driver_impl/src/lib.rs +++ b/compiler/rustc_driver_impl/src/lib.rs @@ -620,8 +620,7 @@ fn show_md_content_with_pager(content: &str, color: ColorConfig) { // to standard output if fallback_to_println { let fmt_success = match color { - ColorConfig::Auto => io::stdout().is_terminal() && bufwtr.print(&mdbuf).is_ok(), - ColorConfig::Always => bufwtr.print(&mdbuf).is_ok(), + ColorConfig::Auto | ColorConfig::Always => bufwtr.print(&mdbuf).is_ok(), ColorConfig::Never => false, }; From 525e1919f730e921f8cc6f917c064c7d083c7048 Mon Sep 17 00:00:00 2001 From: Nicholas Nethercote Date: Wed, 20 Nov 2024 14:15:59 +1100 Subject: [PATCH 3/3] 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. --- compiler/rustc_driver_impl/src/lib.rs | 92 +++++++++++++-------------- 1 file changed, 43 insertions(+), 49 deletions(-) diff --git a/compiler/rustc_driver_impl/src/lib.rs b/compiler/rustc_driver_impl/src/lib.rs index 6198f40e83d95..d2333454f289c 100644 --- a/compiler/rustc_driver_impl/src/lib.rs +++ b/compiler/rustc_driver_impl/src/lib.rs @@ -15,6 +15,7 @@ #![feature(panic_update_hook)] #![feature(result_flattening)] #![feature(rustdoc_internals)] +#![feature(try_blocks)] #![warn(unreachable_pub)] // tidy-alphabetical-end @@ -564,70 +565,63 @@ fn handle_explain(early_dcx: &EarlyDiagCtxt, registry: Registry, code: &str, col } } -/// If color is always or auto, print formatted & colorized markdown. If color is never or -/// if formatted printing fails, print the raw text. +/// If `color` is `always` or `auto`, try to print pretty (formatted & colorized) markdown. If +/// that fails or `color` is `never`, print the raw markdown. /// -/// Prefers a pager, falls back standard print +/// Uses a pager if possible, falls back to stdout. fn show_md_content_with_pager(content: &str, color: ColorConfig) { - let mut fallback_to_println = false; let pager_name = env::var_os("PAGER").unwrap_or_else(|| { if cfg!(windows) { OsString::from("more.com") } else { OsString::from("less") } }); let mut cmd = Command::new(&pager_name); - // FIXME: find if other pagers accept color options - let mut print_formatted = if pager_name == "less" { - cmd.arg("-R"); - true - } else { - ["bat", "batcat", "delta"].iter().any(|v| *v == pager_name) - }; - - if color == ColorConfig::Never { - print_formatted = false; - } else if color == ColorConfig::Always { - print_formatted = true; + if pager_name == "less" { + cmd.arg("-R"); // allows color escape sequences } - let mdstream = markdown::MdStream::parse_str(content); - let bufwtr = markdown::create_stdout_bufwtr(); - let mut mdbuf = bufwtr.buffer(); - if mdstream.write_termcolor_buf(&mut mdbuf).is_err() { - print_formatted = false; - } - - if let Ok(mut pager) = cmd.stdin(Stdio::piped()).spawn() { - if let Some(pipe) = pager.stdin.as_mut() { - let res = if print_formatted { - pipe.write_all(mdbuf.as_slice()) - } else { - pipe.write_all(content.as_bytes()) - }; - - if res.is_err() { - fallback_to_println = true; - } + let pretty_on_pager = match color { + ColorConfig::Auto => { + // Add other pagers that accept color escape sequences here. + ["less", "bat", "batcat", "delta"].iter().any(|v| *v == pager_name) } + ColorConfig::Always => true, + ColorConfig::Never => false, + }; - if pager.wait().is_err() { - fallback_to_println = true; - } - } else { - fallback_to_println = true; - } + // Try to prettify the raw markdown text. The result can be used by the pager or on stdout. + let pretty_data = { + let mdstream = markdown::MdStream::parse_str(content); + let bufwtr = markdown::create_stdout_bufwtr(); + let mut mdbuf = bufwtr.buffer(); + if mdstream.write_termcolor_buf(&mut mdbuf).is_ok() { Some((bufwtr, mdbuf)) } else { None } + }; - // If pager fails for whatever reason, we should still print the content - // to standard output - if fallback_to_println { - let fmt_success = match color { - ColorConfig::Auto | ColorConfig::Always => bufwtr.print(&mdbuf).is_ok(), - ColorConfig::Never => false, + // Try to print via the pager, pretty output if possible. + let pager_res: Option<()> = try { + let mut pager = cmd.stdin(Stdio::piped()).spawn().ok()?; + + let pager_stdin = pager.stdin.as_mut()?; + if pretty_on_pager && let Some((_, mdbuf)) = &pretty_data { + pager_stdin.write_all(mdbuf.as_slice()).ok()?; + } else { + pager_stdin.write_all(content.as_bytes()).ok()?; }; - if !fmt_success { - safe_print!("{content}"); - } + pager.wait().ok()?; + }; + if pager_res.is_some() { + return; } + + // The pager failed. Try to print pretty output to stdout. + if let Some((bufwtr, mdbuf)) = &pretty_data + && bufwtr.print(&mdbuf).is_ok() + { + return; + } + + // Everything failed. Print the raw markdown text. + safe_print!("{content}"); } fn process_rlink(sess: &Session, compiler: &interface::Compiler) {