From ac7634000ef058464e47b2c0f7e542262caa15b2 Mon Sep 17 00:00:00 2001 From: Casey Rodarmor Date: Fri, 11 Nov 2016 15:18:42 -0800 Subject: [PATCH] Fix error messages with wide character Input may contain tabs and other characters whose byte widths do not correspond to their display widths. This causes error context underlining to be off when lines contain those characters Fixed by properly accounting for the display width of characters, as well as replacing tabs with spaces when printing error messages. --- Cargo.lock | 1 + Cargo.toml | 17 ++++----- justfile | 3 ++ src/integration.rs | 90 ++++++++++++++++++++++++++++++++++++++++++++++ src/lib.rs | 57 ++++++++++++++++++++++++----- 5 files changed, 151 insertions(+), 17 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 4d24137..f8a5f91 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -10,6 +10,7 @@ dependencies = [ "lazy_static 0.2.1 (registry+https://github.com/rust-lang/crates.io-index)", "regex 0.1.80 (registry+https://github.com/rust-lang/crates.io-index)", "tempdir 0.3.5 (registry+https://github.com/rust-lang/crates.io-index)", + "unicode-width 0.1.3 (registry+https://github.com/rust-lang/crates.io-index)", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index 8b25d66..b5f0d27 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -7,11 +7,12 @@ license = "WTFPL/MIT/Apache-2.0" homepage = "https://github.com/casey/just" [dependencies] -ansi_term = "^0.9.0" -atty = "^0.2.1" -brev = "^0.1.6" -clap = "^2.0.0" -itertools = "^0.5.5" -lazy_static = "^0.2.1" -regex = "^0.1.77" -tempdir = "^0.3.5" +ansi_term = "^0.9.0" +atty = "^0.2.1" +brev = "^0.1.6" +clap = "^2.0.0" +itertools = "^0.5.5" +lazy_static = "^0.2.1" +regex = "^0.1.77" +tempdir = "^0.3.5" +unicode-width = "^0.1.3" diff --git a/justfile b/justfile index da39868..a383c62 100644 --- a/justfile +++ b/justfile @@ -50,6 +50,9 @@ nop: fail: exit 1 +backtick-fail: + echo {{`exit 1`}} + # make a quine, compile it, and verify it quine: create cc tmp/gen0.c -o tmp/gen0 diff --git a/src/integration.rs b/src/integration.rs index 8844867..36cda75 100644 --- a/src/integration.rs +++ b/src/integration.rs @@ -372,6 +372,96 @@ fn backtick_code_interpolation() { ); } +#[test] +fn backtick_code_interpolation_tab() { + integration_test( + &[], + " +backtick-fail: +\techo {{`exit 1`}} +", + 1, + "", + "error: backtick failed with exit code 1 + | +3 | echo {{`exit 1`}} + | ^^^^^^^^ +", + ); +} + +#[test] +fn backtick_code_interpolation_tabs() { + integration_test( + &[], + " +backtick-fail: +\techo {{\t`exit 1`}} +", + 1, + "", + "error: backtick failed with exit code 1 + | +3 | echo {{ `exit 1`}} + | ^^^^^^^^ +", + ); +} + +#[test] +fn backtick_code_interpolation_inner_tab() { + integration_test( + &[], + " +backtick-fail: +\techo {{\t`exit\t\t1`}} +", + 1, + "", + "error: backtick failed with exit code 1 + | +3 | echo {{ `exit 1`}} + | ^^^^^^^^^^^^^^^ +", + ); +} + +#[test] +fn backtick_code_interpolation_leading_emoji() { + integration_test( + &[], + " +backtick-fail: +\techo šŸ˜¬{{`exit 1`}} +", + 1, + "", + "error: backtick failed with exit code 1 + | +3 | echo šŸ˜¬{{`exit 1`}} + | ^^^^^^^^ +", + ); +} + +#[test] +fn backtick_code_interpolation_unicode_hell() { + integration_test( + &[], + " +backtick-fail: +\techo \t\t\tšŸ˜¬éŽŒé¼¬{{\t\t`exit 1 # \t\t\tšŸ˜¬éŽŒé¼¬`}}\t\t\tšŸ˜¬éŽŒé¼¬ +", + 1, + "", + "error: backtick failed with exit code 1 + | +3 | echo šŸ˜¬éŽŒé¼¬{{ `exit 1 # šŸ˜¬éŽŒé¼¬`}} šŸ˜¬éŽŒé¼¬ + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^ +", + ); +} + #[test] fn backtick_code_long() { integration_test( diff --git a/src/lib.rs b/src/lib.rs index 34c297a..9140f37 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -14,6 +14,7 @@ extern crate regex; extern crate tempdir; extern crate itertools; extern crate ansi_term; +extern crate unicode_width; use std::io::prelude::*; @@ -786,12 +787,41 @@ fn write_error_context( let red = maybe_red(f.alternate()); match text.lines().nth(line) { Some(line) => { + let mut i = 0; + let mut space_column = 0; + let mut space_line = String::new(); + let mut space_width = 0; + for c in line.chars() { + if c == '\t' { + space_line.push_str(" "); + if i < column { + space_column += 4; + } + if i >= column && i < column + width.unwrap_or(1) { + space_width += 4; + } + } else { + if i < column { + space_column += unicode_width::UnicodeWidthChar::width(c).unwrap_or(0); + + } + if i >= column && i < column + width.unwrap_or(1) { + space_width += unicode_width::UnicodeWidthChar::width(c).unwrap_or(0); + } + space_line.push(c); + } + i += c.len_utf8(); + } let line_number_width = line_number.to_string().len(); try!(write!(f, "{0:1$} |\n", "", line_number_width)); - try!(write!(f, "{} | {}\n", line_number, line)); + try!(write!(f, "{} | {}\n", line_number, space_line)); try!(write!(f, "{0:1$} |", "", line_number_width)); - try!(write!(f, " {0:1$}{2}{3:^<4$}{5}", "", column, - red.prefix(), "", width.unwrap_or(1), red.suffix())); + if width == None { + try!(write!(f, " {0:1$}{2}^{3}", "", space_column, red.prefix(), red.suffix())); + } else { + try!(write!(f, " {0:1$}{2}{3:^<4$}{5}", "", space_column, + red.prefix(), "", space_width, red.suffix())); + } }, None => if index != text.len() { try!(write!(f, "internal error: Error has invalid line number: {}", line_number)) @@ -1091,7 +1121,10 @@ enum RunError<'a> { impl<'a> Display for RunError<'a> { fn fmt(&self, f: &mut fmt::Formatter) -> Result<(), fmt::Error> { let red = maybe_red(f.alternate()); - try!(write!(f, "{} ", red.paint("error:"))); + let bold = maybe_bold(f.alternate()); + try!(write!(f, "{} {}", red.paint("error:"), bold.prefix())); + + let mut error_token = None; match *self { RunError::UnknownRecipes{ref recipes} => { @@ -1140,15 +1173,15 @@ impl<'a> Display for RunError<'a> { recipe, io_error)), RunError::BacktickCode{code, ref token} => { try!(write!(f, "backtick failed with exit code {}\n", code)); - try!(write_token_error_context(f, token)); + error_token = Some(token); } RunError::BacktickSignal{ref token, signal} => { try!(write!(f, "backtick was terminated by signal {}", signal)); - try!(write_token_error_context(f, token)); + error_token = Some(token); } RunError::BacktickUnknownFailure{ref token} => { try!(write!(f, "backtick failed for an uknown reason")); - try!(write_token_error_context(f, token)); + error_token = Some(token); } RunError::BacktickIoError{ref token, ref io_error} => { try!(match io_error.kind() { @@ -1160,11 +1193,11 @@ impl<'a> Display for RunError<'a> { _ => write!(f, "backtick could not be run because of an IO \ error while launching `sh`:\n{}", io_error), }); - try!(write_token_error_context(f, token)); + error_token = Some(token); } RunError::BacktickUtf8Error{ref token, ref utf8_error} => { try!(write!(f, "backtick succeeded but stdout was not utf8: {}", utf8_error)); - try!(write_token_error_context(f, token)); + error_token = Some(token); } RunError::InternalError{ref message} => { try!(write!(f, "internal error, this may indicate a bug in just: {} @@ -1172,6 +1205,12 @@ consider filing an issue: https://github.com/casey/just/issues/new", message)); } } + try!(write!(f, "{}", bold.suffix())); + + if let Some(token) = error_token { + try!(write_token_error_context(f, token)); + } + Ok(()) } }