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

panic when piping hx --health through head #1841

Closed
the-mikedavis opened this issue Mar 18, 2022 · 5 comments · Fixed by #1876
Closed

panic when piping hx --health through head #1841

the-mikedavis opened this issue Mar 18, 2022 · 5 comments · Fixed by #1876
Labels
A-helix-term Area: Helix term improvements C-bug Category: This is a bug E-easy Call for participation: Experience needed to fix: Easy / not much E-good-first-issue Call for participation: Issues suitable for new contributors

Comments

@the-mikedavis
Copy link
Member

the-mikedavis commented Mar 18, 2022

Summary

Piping hx --health through head gives a panic. I'm not sure if it's something to do with my terminal (kitty). Piping through less works perfectly, even with colors.

Reproduction Steps

asciicast

$ hx --health | head -n 10

Helix log

N/A: no log lines were produced.

But here's the backtrace...
thread 'main' panicked at 'failed printing to stdout: Broken pipe (os error 32)', library/std/src/io/stdio.rs:1187:9
stack backtrace:
   0:     0x560d47de313c - <std::sys_common::backtrace::_print::DisplayBacktrace as core::fmt::Display>::fmt::h3e2b509ce2ce6007
   1:     0x560d4773a1ac - core::fmt::write::h753c7571fa063ecb
   2:     0x560d47ddb565 - std::io::Write::write_fmt::h2815c0519c99ba09
   3:     0x560d47de62e5 - std::panicking::default_hook::{{closure}}::h78d3e6cf97fc623d
   4:     0x560d47de5f63 - std::panicking::default_hook::hda898f8d3ad1a5ae
   5:     0x560d47de696c - std::panicking::rust_panic_with_hook::h1a5ea2d6c23051aa
   6:     0x560d47de665a - std::panicking::begin_panic_handler::{{closure}}::h07f549390938b73f
   7:     0x560d47de3664 - std::sys_common::backtrace::__rust_end_short_backtrace::h5ec3758a92cfb00d
   8:     0x560d47de63bd - rust_begin_unwind
   9:     0x560d476c1d01 - core::panicking::panic_fmt::h3a79a6a99affe1d5
  10:     0x560d47dda7d1 - std::io::stdio::_print::h4dabb72d0b79d2de
  11:     0x560d47bba8d5 - helix_term::health::languages_all::h64ed386eb51c8a24
  12:     0x560d47c84923 - <core::future::from_generator::GenFuture<T> as core::future::future::Future>::poll::h45a31bbdd9e955af
  13:     0x560d47c543ce - tokio::park::thread::CachedParkThread::block_on::h8bfc4daa5e5d5900
  14:     0x560d47cbc559 - tokio::runtime::thread_pool::ThreadPool::block_on::hfbbc83b50318e9de
  15:     0x560d47c81e83 - tokio::runtime::Runtime::block_on::hdab0bdfd50a45238
  16:     0x560d47c52eaa - hx::main::h7b50b857af503774
  17:     0x560d47c55fe3 - std::sys_common::backtrace::__rust_begin_short_backtrace::ha19c1e7ad046c834
  18:     0x560d47c68c8d - std::rt::lang_start::{{closure}}::ha7afa477cb4d71de
  19:     0x560d47de279e - std::rt::lang_start_internal::h52e73755f77c7dd9
  20:     0x560d47c562e2 - main
  21:     0x7fb18ef68790 - __libc_start_main
  22:     0x560d476f041a - _start
  23:                0x0 - <unknown>

Platform

Linux (NixOS 21.11)

Terminal Emulator

kitty 0.23.1

Helix Version

master at 533cca7

@the-mikedavis the-mikedavis added the C-bug Category: This is a bug label Mar 18, 2022
@dead10ck
Copy link
Member

@sudormrfbin looks like rather than using println! we'll have to handle writing to stdout ourselves and explicitly handle a broken pipe error to just quit without error.

@the-mikedavis the-mikedavis added the A-helix-term Area: Helix term improvements label Mar 18, 2022
@sudormrfbin sudormrfbin self-assigned this Mar 19, 2022
@sudormrfbin sudormrfbin removed their assignment Mar 27, 2022
@sudormrfbin sudormrfbin added E-easy Call for participation: Experience needed to fix: Easy / not much E-good-first-issue Call for participation: Issues suitable for new contributors labels Mar 27, 2022
@sudormrfbin
Copy link
Member

If anyone wants to take this up, here is an SO post describing the current behavior and how to fix it, and these are the functions where the diagnostics are printed:

pub fn general() {
let config_file = helix_loader::config_file();
let lang_file = helix_loader::lang_config_file();
let log_file = helix_loader::log_file();
let rt_dir = helix_loader::runtime_dir();
if config_file.exists() {
println!("Config file: {}", config_file.display());
} else {
println!("Config file: default")
}
if lang_file.exists() {
println!("Language file: {}", lang_file.display());
} else {
println!("Language file: default")
}
println!("Log file: {}", log_file.display());
println!("Runtime directory: {}", rt_dir.display());
if let Ok(path) = std::fs::read_link(&rt_dir) {
let msg = format!("Runtime directory is symlinked to {}", path.display());
println!("{}", msg.yellow());
}
if !rt_dir.exists() {
println!("{}", "Runtime directory does not exist.".red());
}
if rt_dir.read_dir().ok().map(|it| it.count()) == Some(0) {
println!("{}", "Runtime directory is empty.".red());
}
}
pub fn languages_all() {
let mut syn_loader_conf = user_syntax_loader().unwrap_or_else(|err| {
eprintln!("{}: {}", "Error parsing user language config".red(), err);
eprintln!("{}", "Using default language config".yellow());
default_syntax_loader()
});
let mut headings = vec!["Language", "LSP", "DAP"];
for feat in TsFeature::all() {
headings.push(feat.short_title())
}
let terminal_cols = crossterm::terminal::size().map(|(c, _)| c).unwrap_or(80);
let column_width = terminal_cols as usize / headings.len();
let column = |item: &str, color: Color| {
let data = format!(
"{:column_width$}",
item.get(..column_width - 2)
.map(|s| format!("{s}…"))
.unwrap_or_else(|| item.to_string())
)
.stylize()
.with(color);
// We can't directly use println!() because of
// https://github.com/crossterm-rs/crossterm/issues/589
let _ = crossterm::execute!(std::io::stdout(), Print(data));
};
for heading in headings {
column(heading, Color::White);
}
println!();
syn_loader_conf
.language
.sort_unstable_by_key(|l| l.language_id.clone());
let check_binary = |cmd: Option<String>| match cmd {
Some(cmd) => match which::which(&cmd) {
Ok(_) => column(&cmd, Color::Green),
Err(_) => column(&cmd, Color::Red),
},
None => column("None", Color::Yellow),
};
for lang in &syn_loader_conf.language {
column(&lang.language_id, Color::Reset);
let lsp = lang
.language_server
.as_ref()
.map(|lsp| lsp.command.to_string());
check_binary(lsp);
let dap = lang.debugger.as_ref().map(|dap| dap.command.to_string());
check_binary(dap);
for ts_feat in TsFeature::all() {
match load_runtime_file(&lang.language_id, ts_feat.runtime_filename()).is_ok() {
true => column("Found", Color::Green),
false => column("Not Found", Color::Red),
}
}
println!();
}
}
/// Display diagnostics pertaining to a particular language (LSP,
/// highlight queries, etc).
pub fn language(lang_str: String) {
let syn_loader_conf = user_syntax_loader().unwrap_or_else(|err| {
eprintln!("{}: {}", "Error parsing user language config".red(), err);
eprintln!("{}", "Using default language config".yellow());
default_syntax_loader()
});
let lang = match syn_loader_conf
.language
.iter()
.find(|l| l.language_id == lang_str)
{
Some(l) => l,
None => {
let msg = format!("Language '{lang_str}' not found");
println!("{}", msg.red());
let suggestions: Vec<&str> = syn_loader_conf
.language
.iter()
.filter(|l| l.language_id.starts_with(lang_str.chars().next().unwrap()))
.map(|l| l.language_id.as_str())
.collect();
if !suggestions.is_empty() {
let suggestions = suggestions.join(", ");
println!("Did you mean one of these: {} ?", suggestions.yellow());
}
return;
}
};
probe_protocol(
"language server",
lang.language_server
.as_ref()
.map(|lsp| lsp.command.to_string()),
);
probe_protocol(
"debug adapter",
lang.debugger.as_ref().map(|dap| dap.command.to_string()),
);
for ts_feat in TsFeature::all() {
probe_treesitter_feature(&lang_str, *ts_feat)
}
}
/// Display diagnostics about LSP and DAP.
fn probe_protocol(protocol_name: &str, server_cmd: Option<String>) {
let cmd_name = match server_cmd {
Some(ref cmd) => cmd.as_str().green(),
None => "None".yellow(),
};
println!("Configured {}: {}", protocol_name, cmd_name);
if let Some(cmd) = server_cmd {
let path = match which::which(&cmd) {
Ok(path) => path.display().to_string().green(),
Err(_) => "Not found in $PATH".to_string().red(),
};
println!("Binary for {}: {}", protocol_name, path);
}
}
/// Display diagnostics about a feature that requires tree-sitter
/// query files (highlights, textobjects, etc).
fn probe_treesitter_feature(lang: &str, feature: TsFeature) {
let found = match load_runtime_file(lang, feature.runtime_filename()).is_ok() {
true => "Found".green(),
false => "Not found".red(),
};
println!("{} queries: {}", feature.short_title(), found);
}

The functions will have to return an io::Result<()> and all the println!() calls will have to be replaced by writeln!()?, and then in the calling code io::ErrorKind::BrokenPipe will have to be handled.

@nirmal-j-patel
Copy link
Contributor

I would like to attempt to fix this. Thanks.

@sudormrfbin
Copy link
Member

@npate012 Go for it !

nirmal-j-patel added a commit to nirmal-j-patel/helix that referenced this issue Mar 29, 2022
Functions in health.rs now return std:io::Result<()>. All instances of println!
and eprintln! have been replaced with writeln!. This allows detection of errors
while printing messages and errors. BrokenPipe errors are ignored. All other
errors are returned.

Fixes helix-editor#1841

Signed-off-by: Nirmal Patel <[email protected]>
nirmal-j-patel added a commit to nirmal-j-patel/helix that referenced this issue Mar 29, 2022
Functions in health.rs now return std:io::Result<()>. All instances of println!
and eprintln! have been replaced with writeln!. This allows detection of errors
while printing messages and errors. BrokenPipe errors are ignored. All other
errors are returned.

Fixes helix-editor#1841

Signed-off-by: Nirmal Patel <[email protected]>
@nirmal-j-patel
Copy link
Contributor

I have created #1876 for this. I hope it looks good. Thanks.

nirmal-j-patel added a commit to nirmal-j-patel/helix that referenced this issue Mar 30, 2022
Functions in health.rs now return std:io::Result<()>. All instances of println!
and eprintln! have been replaced with writeln!. This allows detection of errors
while printing messages and errors. BrokenPipe errors are ignored. All other
errors are returned.

Fixes helix-editor#1841

Signed-off-by: Nirmal Patel <[email protected]>
sudormrfbin pushed a commit to nirmal-j-patel/helix that referenced this issue Mar 30, 2022
Functions in health.rs now return std:io::Result<()>. All instances of println!
and eprintln! have been replaced with writeln!. This allows detection of errors
while printing messages and errors. BrokenPipe errors are ignored. All other
errors are returned.

Fixes helix-editor#1841

Signed-off-by: Nirmal Patel <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-helix-term Area: Helix term improvements C-bug Category: This is a bug E-easy Call for participation: Experience needed to fix: Easy / not much E-good-first-issue Call for participation: Issues suitable for new contributors
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants