sed: underline the offending script character at a terminal

When stderr is a terminal (or UUTILS_DIAG=always), quote the offending
script line and underline the faulty character, in the layout uucore's
diagnostics use. Pipes and the test suite still get the single-line
message.

Both error constructors handle this, so no call site changes.
ScriptLocation now carries the line text, captured only when diagnostics
are on and shared between commands from the same line, so semantic
errors raised after compilation can still be drawn.

Anything that cannot be drawn (non-UTF-8 script, empty line, no line
recorded) leaves the message unchanged.

Cargo.lock

@@ -85,6 +85,16 @@ dependencies = [
  "num-traits",
 ]
 
+[[package]]
+name = "ariadne"
+version = "0.6.0"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "8454c8a44ce2cb9cc7e7fae67fc6128465b343b92c6631e94beca3c8d1524ea5"
+dependencies = [
+ "unicode-width",
+ "yansi",
+]
+
 [[package]]
 name = "assert_fs"
 version = "1.1.4"
@@ -1103,7 +1113,7 @@ dependencies = [
 
 [[package]]
 name = "sed"
-version = "0.2.0"
+version = "0.3.0"
 dependencies = [
  "assert_fs",
  "chrono",
@@ -1129,6 +1139,7 @@ dependencies = [
  "tempfile",
  "terminal_size",
  "textwrap",
+ "unicode-width",
  "uucore",
  "uutests",
  "xattr",
@@ -1453,6 +1464,7 @@ version = "0.12.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "f24a5910ebbf2c0baf486a9125705f8e026a8f7741a3ea8cf5eca3c9257c9bdf"
 dependencies = [
+ "ariadne",
  "clap",
  "dns-lookup",
  "fluent",

Cargo.toml

@@ -1,11 +1,12 @@
 # sed (uutils)
 # * see the repository LICENSE, README, and CONTRIBUTING files for more information
 
-# spell-checker:ignore (libs) mangen sed
+# spell-checker:ignore (libs) mangen sed binstall chrono codspeed libc memchr memmap rlimit rustix textwrap uucore uutests xattr
+# spell-checker:ignore (misc) clippy codegen msvc startswith uninlined uutils
 
 [package]
 name = "sed"
-version = "0.2.0"
+version = "0.3.0"
 license = "MIT"
 description = "sed ~ implemented as universal (cross-platform) utils, written in Rust"
 default-run = "sed"
@@ -62,7 +63,8 @@ sha2 = { version = "0.11.0", default-features = false, features = ["alloc"] }
 tempfile = "3.10.1"
 terminal_size = "0.4.2"
 textwrap = { version = "0.16.1", features = ["terminal_size"] }
-uucore = { version = "0.12.0", features = ["libc"] }
+unicode-width = "0.2.2"
+uucore = { version = "0.12.0", features = ["diagnostics", "libc"] }
 rustix = "1.1.4"
 xattr = "1.3.1"
 
@@ -80,6 +82,7 @@ regex = { workspace = true }
 tempfile = { workspace = true }
 terminal_size = { workspace = true }
 textwrap = { workspace = true }
+unicode-width = { workspace = true }
 uucore = { workspace = true }
 rustix = { workspace = true }
 

docs/src/extensions.md

@@ -39,6 +39,17 @@ Below is a list of these extensions and incompatibilities.
 ## New extensions
 * Unicode characters can be specified in regular expression pattern, replacement
   and transliteration sequences using `\uXXXX` or `\UXXXXXXXX` sequences.
+* On a terminal, script errors also quote the script line and underline the
+  character at fault (`UUTILS_DIAG=always` or `never` overrides this):
+  ```
+  $ sed 's/a/b/q'
+  sed: <script argument 1>:1:7: error: invalid substitute flag: 'q'
+     ╭─[ <script argument 1>:1:7 ]
+     │
+   1 │ s/a/b/q
+     │       ─
+  ───╯
+  ```
 
 ## Incompatible extensions
 The `-U` or `--uutil-extensions` option enables useful extensions or bug fixes

fuzz/Cargo.lock

@@ -897,7 +897,7 @@ checksum = "b39cdef0fa800fc44525c84ccb54a029961a8215f9619753635a9c0d2538d46d"
 
 [[package]]
 name = "sed"
-version = "0.2.0"
+version = "0.3.0"
 dependencies = [
  "clap",
  "clap_complete",

src/sed/compiler.rs

@@ -359,7 +359,7 @@ fn compile_address_range(
         let is_step_match = line.current() == '~'; // E.g. 0~2: Pick even-numbered lines
         line.advance();
         line.eat_spaces();
-        let is_step_end = if line.current() == '~' {
+        let is_step_end = if !line.eol() && line.current() == '~' {
             // E.g. /foo/,~10: Start at foo, include all lines until multiple of 10 is reached.
             line.advance();
             line.eat_spaces();
@@ -373,39 +373,46 @@ fn compile_address_range(
             return compilation_error(lines, line, "~step is invalid in POSIX mode");
         }
 
-        // Look for second address.
-        if !line.eol() {
-            let addr2 = compile_address(lines, line, context)?;
-            // Set step_n to the number specified in the (required numeric) address.
-            let step_n = if is_step_match || is_step_end {
-                match addr2 {
-                    Address::Line(n) => n,
-                    _ => {
-                        return compilation_error(
-                            lines,
-                            line,
-                            "~step can only be specified through numeric values",
-                        );
-                    }
-                }
-            } else {
-                0 // dummy, not used
-            };
-
-            if is_line0 && !matches!(addr2, Address::Re(_)) && !is_step_match {
-                return compilation_error(lines, line, ERR_ADDRESS_0_USAGE);
-            }
-
-            // If needed, transform Address::Line into Address::Step*.
-            cmd.addr2 = if is_step_match {
-                Some(Address::StepMatch(step_n))
-            } else if is_step_end {
-                Some(Address::StepEnd(step_n))
-            } else {
-                Some(addr2)
-            };
-            n_addr += 1;
+        // Look for second address; a missing ~step counts as 0.
+        let has_addr2 = !line.eol() && (is_address_char(line.current()) || line.current() == '+');
+        if !(has_addr2 || is_step_match || is_step_end) {
+            return compilation_error(lines, line, "unexpected `,'");
         }
+        let addr2 = if has_addr2 {
+            compile_address(lines, line, context)?
+        } else {
+            Address::Line(0)
+        };
+        // Set step_n to the number specified in the (required numeric) address.
+        let step_n = if is_step_match || is_step_end {
+            match addr2 {
+                Address::Line(n) => n,
+                _ => {
+                    return compilation_error(
+                        lines,
+                        line,
+                        "~step can only be specified through numeric values",
+                    );
+                }
+            }
+        } else {
+            0 // dummy, not used
+        };
+
+        // Address 0 needs a regex end or a positive ~step (0~0 selects no line).
+        if is_line0 && !matches!(addr2, Address::Re(_)) && !(is_step_match && step_n > 0) {
+            return compilation_error(lines, line, ERR_ADDRESS_0_USAGE);
+        }
+
+        // If needed, transform Address::Line into Address::Step*.
+        cmd.addr2 = if is_step_match {
+            Some(Address::StepMatch(step_n))
+        } else if is_step_end {
+            Some(Address::StepEnd(step_n))
+        } else {
+            Some(addr2)
+        };
+        n_addr += 1;
     }
 
     // Zero-address read command check
@@ -496,7 +503,7 @@ fn compile_address(
             let number = parse_number(lines, line, true)?.unwrap();
             Ok(Address::Line(number))
         }
-        _ => panic!("invalid context address"),
+        _ => compilation_error(lines, line, "expected context address"),
     }
 }
 

src/sed/error_handling.rs

@@ -12,8 +12,13 @@ use crate::sed::command::ProcessingContext;
 use crate::sed::script_char_provider::ScriptCharProvider;
 use crate::sed::script_line_provider::ScriptLineProvider;
 
+use std::env;
+use std::io::{IsTerminal, stderr};
 use std::rc::Rc;
+use std::sync::OnceLock;
 
+use unicode_width::UnicodeWidthChar;
+use uucore::diagnostics;
 use uucore::display::Quotable;
 use uucore::error::{UResult, USimpleError};
 
@@ -23,6 +28,9 @@ pub struct ScriptLocation {
     pub input_name: Rc<str>,  // Shared input name
     pub line_number: usize,   // 1-based line number
     pub column_number: usize, // 1-based column number
+    /// The script line, for errors raised after it was consumed.
+    /// `None` when diagnostics are off.
+    pub line_text: Option<Rc<str>>,
 }
 
 impl Default for ScriptLocation {
@@ -31,6 +39,7 @@ impl Default for ScriptLocation {
             input_name: Rc::from("<unknown>"),
             line_number: 1,
             column_number: 1,
+            line_text: None,
         }
     }
 }
@@ -42,10 +51,123 @@ impl ScriptLocation {
             line_number: lines.get_line_number(),
             column_number: line.get_pos() + 1,
             input_name: Rc::from(lines.get_input_name()),
+            line_text: if diagnostics_enabled() {
+                line.line_text()
+            } else {
+                None
+            },
         }
     }
 }
 
+/// Whether script errors get a snippet. Evaluated once: a location is
+/// recorded for every compiled command, and the terminal check is a syscall.
+fn diagnostics_enabled() -> bool {
+    static ENABLED: OnceLock<bool> = OnceLock::new();
+    *ENABLED.get_or_init(diagnostics::enabled)
+}
+
+/// Format `msg` as an error at the given script position, followed by the
+/// offending script line, underlined, when diagnostics are enabled.
+fn script_error(
+    input_name: &str,
+    line_number: usize,
+    column_number: usize,
+    line_text: Option<&str>,
+    msg: impl ToString,
+) -> String {
+    let message = format!(
+        "{input_name}:{line_number}:{column_number}: error: {}",
+        msg.to_string()
+    );
+    // Line 0: the script was exhausted, so there is no line to show.
+    if !diagnostics_enabled() || line_number == 0 {
+        return message;
+    }
+    // A script line that is not valid UTF-8 cannot be drawn.
+    let Some(text) = line_text.filter(|text| !text.is_empty()) else {
+        return message;
+    };
+
+    // `column_number` is a 1-based byte column; errors at end of line point
+    // one past the last character.
+    let Some(prefix) = text.get(..(column_number - 1).min(text.len())) else {
+        return message;
+    };
+    let drawn = render_snippet(
+        input_name,
+        line_number,
+        column_number,
+        text,
+        prefix,
+        use_color(),
+    );
+    format!("{message}\n{drawn}")
+}
+
+/// Whether the underline is colored. uucore's diagnostics make the same
+/// check but keep it private.
+fn use_color() -> bool {
+    env::var_os("NO_COLOR").is_none() && stderr().is_terminal()
+}
+
+/// Draw `text` as line `line_number` of `input_name`, underlining the
+/// character that follows `prefix`, in the layout uucore's diagnostics use:
+///
+/// ```text
+///    ╭─[ <script argument 1>:1:7 ]
+///    │
+///  1 │ s/a/b/q
+///    │       ─
+/// ───╯
+/// ```
+fn render_snippet(
+    input_name: &str,
+    line_number: usize,
+    column_number: usize,
+    text: &str,
+    prefix: &str,
+    color: bool,
+) -> String {
+    let number = line_number.to_string();
+    let margin = " ".repeat(number.len() + 2);
+    // Pad by display width so the underline lines up with the character
+    // above it; tabs are kept as they are, since their width depends on the
+    // terminal.
+    let indent: String = prefix
+        .chars()
+        .flat_map(|c| {
+            let (fill, width) = if c == '\t' {
+                ('\t', 1)
+            } else {
+                (' ', c.width().unwrap_or(0))
+            };
+            std::iter::repeat_n(fill, width)
+        })
+        .collect();
+    // As wide as the offending character; one column past the end of line.
+    let width = text[prefix.len()..]
+        .chars()
+        .next()
+        .and_then(UnicodeWidthChar::width)
+        .unwrap_or(1)
+        .max(1);
+    let underline = "─".repeat(width);
+    let underline = if color {
+        format!("\x1b[31m{underline}\x1b[0m")
+    } else {
+        underline
+    };
+    format!(
+        "{margin}╭─[ {input_name}:{line_number}:{column_number} ]\n\
+         {margin}│\n\
+         \x20{number} │ {text}\n\
+         {margin}│ {indent}{underline}\n\
+         {rule}╯",
+        rule = "─".repeat(margin.len()),
+    )
+}
+
 /// Fail with msg as a compile error at the provider location.
 /// The error's exit code is 1 (compilation phase).
 pub fn compilation_error<T>(
@@ -55,12 +177,12 @@ pub fn compilation_error<T>(
 ) -> UResult<T> {
     Err(USimpleError::new(
         1,
-        format!(
-            "{}:{}:{}: error: {}",
+        script_error(
             lines.get_input_name(),
             lines.get_line_number(),
             line.get_pos() + 1,
-            msg.to_string()
+            str::from_utf8(line.get_line()).ok(),
+            msg,
         ),
     ))
 }
@@ -70,12 +192,12 @@ pub fn compilation_error<T>(
 fn location_error<T>(location: &ScriptLocation, msg: impl ToString, exit_code: i32) -> UResult<T> {
     Err(USimpleError::new(
         exit_code,
-        format!(
-            "{}:{}:{}: error: {}",
-            location.input_name,
+        script_error(
+            &location.input_name,
             location.line_number,
             location.column_number,
-            msg.to_string()
+            location.line_text.as_deref(),
+            msg,
         ),
     ))
 }
@@ -115,3 +237,25 @@ pub fn input_runtime_error<T>(
         ),
     ))
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    // The integration tests check the layout; they cannot get color, as
+    // their stderr is not a terminal.
+    #[test]
+    fn the_underline_is_red_when_colored() {
+        let drawn = render_snippet("f.sed", 1, 7, "s/a/b/q", "s/a/b/", true);
+        assert_eq!(
+            drawn,
+            concat!(
+                "   ╭─[ f.sed:1:7 ]\n",
+                "   │\n",
+                " 1 │ s/a/b/q\n",
+                "   │       \x1b[31m─\x1b[0m\n",
+                "───╯",
+            )
+        );
+    }
+}

src/sed/processor.rs

@@ -129,7 +129,7 @@ fn applies(
                     Address::Line(n) if linenum >= *n => {
                         context.last_address = true;
                     }
-                    Address::RelLine(n) if *n == 0 => {
+                    Address::RelLine(0) | Address::StepMatch(0) | Address::StepEnd(0) => {
                         context.last_address = true;
                     }
                     _ => {

src/sed/script_char_provider.rs

@@ -8,10 +8,15 @@
 // For the full copyright and license information, please view the LICENSE
 // file that was distributed with this source code.
 
+use std::cell::OnceCell;
+use std::rc::Rc;
+
 #[derive(Debug)]
 pub struct ScriptCharProvider {
     line: Vec<u8>,
     pos: usize,
+    /// The line as text, shared by the commands compiled from it.
+    text: OnceCell<Option<Rc<str>>>,
 }
 
 impl ScriptCharProvider {
@@ -20,6 +25,7 @@ impl ScriptCharProvider {
         Self {
             line: line.as_ref().to_vec(),
             pos: 0,
+            text: OnceCell::new(),
         }
     }
 
@@ -66,6 +72,18 @@ impl ScriptCharProvider {
     pub fn get_pos(&self) -> usize {
         self.pos
     }
+
+    /// Return the whole line being scanned.
+    pub fn get_line(&self) -> &[u8] {
+        &self.line
+    }
+
+    /// Return the line as shared text, or `None` if it is not valid UTF-8.
+    pub fn line_text(&self) -> Option<Rc<str>> {
+        self.text
+            .get_or_init(|| str::from_utf8(&self.line).ok().map(Rc::from))
+            .clone()
+    }
 }
 
 #[cfg(test)]

tests/by-util/test_sed.rs

@@ -1183,6 +1183,166 @@ fn pattern_clear_with_z_is_non_posix() {
 }
 check_output!(trans_newline, ["-e", r"1N;2y/\n/X/", LINES1]);
 
+////////////////////////////////////////////////////////////
+// Rich diagnostics, drawn for a terminal and controlled by UUTILS_DIAG
+#[test]
+fn diagnostics_stay_out_of_the_way_of_scripts() {
+    // stderr is not a terminal here, so only the plain message is printed.
+    for value in [None, Some(""), Some("never")] {
+        let mut command = new_ucmd!();
+        if let Some(value) = value {
+            command.env("UUTILS_DIAG", value);
+        }
+        command
+            .args(&["--posix", "z"])
+            .fails()
+            .code_is(1)
+            .stderr_is("sed: <script argument 1>:1:1: error: invalid command code `z'\n");
+    }
+}
+
+#[test]
+fn diagnostics_underline_the_offending_character() {
+    new_ucmd!()
+        .env("UUTILS_DIAG", "always")
+        .args(&["s/a/b/q"])
+        .fails()
+        .code_is(1)
+        .stderr_is(concat!(
+            "sed: <script argument 1>:1:7: error: invalid substitute flag: 'q'\n",
+            "   ╭─[ <script argument 1>:1:7 ]\n",
+            "   │\n",
+            " 1 │ s/a/b/q\n",
+            "   │       ─\n",
+            "───╯\n",
+        ));
+}
+
+#[test]
+fn diagnostics_can_point_past_the_end_of_the_line() {
+    new_ucmd!()
+        .env("UUTILS_DIAG", "always")
+        .args(&["/adrift"])
+        .fails()
+        .code_is(1)
+        .stderr_is(concat!(
+            "sed: <script argument 1>:1:8: error: unterminated regular expression\n",
+            "   ╭─[ <script argument 1>:1:8 ]\n",
+            "   │\n",
+            " 1 │ /adrift\n",
+            "   │        ─\n",
+            "───╯\n",
+        ));
+}
+
+#[test]
+fn diagnostics_count_columns_in_bytes_like_the_message() {
+    // `é` is two bytes: the header must use byte column 8, not 7.
+    new_ucmd!()
+        .env("UUTILS_DIAG", "always")
+        .args(&["s/é/e/z"])
+        .fails()
+        .code_is(1)
+        .stderr_is(concat!(
+            "sed: <script argument 1>:1:8: error: invalid substitute flag: 'z'\n",
+            "   ╭─[ <script argument 1>:1:8 ]\n",
+            "   │\n",
+            " 1 │ s/é/e/z\n",
+            "   │       ─\n",
+            "───╯\n",
+        ));
+}
+
+#[test]
+fn diagnostics_reach_errors_raised_after_the_line_was_read() {
+    // Undefined labels are found after compilation; the gutter shows line 2.
+    new_ucmd!()
+        .env("UUTILS_DIAG", "always")
+        .args(&["-e", "p\nb nowhere"])
+        .fails()
+        .code_is(1)
+        .stderr_is(concat!(
+            "sed: <script argument 1>:2:1: error: undefined label `nowhere'\n",
+            "   ╭─[ <script argument 1>:2:1 ]\n",
+            "   │\n",
+            " 2 │ b nowhere\n",
+            "   │ ─\n",
+            "───╯\n",
+        ));
+}
+
+#[test]
+fn diagnostics_line_up_under_wide_characters() {
+    // `日本` takes four columns: the underline must sit under `q`.
+    new_ucmd!()
+        .env("UUTILS_DIAG", "always")
+        .args(&["s/日本/x/q"])
+        .fails()
+        .code_is(1)
+        .stderr_is(concat!(
+            "sed: <script argument 1>:1:12: error: invalid substitute flag: 'q'\n",
+            "   ╭─[ <script argument 1>:1:12 ]\n",
+            "   │\n",
+            " 1 │ s/日本/x/q\n",
+            "   │          ─\n",
+            "───╯\n",
+        ));
+}
+
+#[test]
+fn diagnostics_underline_a_wide_character_across_its_columns() {
+    new_ucmd!()
+        .env("UUTILS_DIAG", "always")
+        .args(&["p日"])
+        .fails()
+        .code_is(1)
+        .stderr_is(concat!(
+            "sed: <script argument 1>:1:2: error: extra characters at the end of the p command\n",
+            "   ╭─[ <script argument 1>:1:2 ]\n",
+            "   │\n",
+            " 1 │ p日\n",
+            "   │  ──\n",
+            "───╯\n",
+        ));
+}
+
+#[test]
+fn diagnostics_keep_tabs_and_widen_the_gutter() {
+    // Line 10 starts with a tab, kept in the indent so the underline lines up.
+    new_ucmd!()
+        .env("UUTILS_DIAG", "always")
+        .args(&[format!("{}\tpz", "p\n".repeat(9))])
+        .fails()
+        .code_is(1)
+        .stderr_is(concat!(
+            "sed: <script argument 1>:10:3: error: extra characters at the end of the p command\n",
+            "    ╭─[ <script argument 1>:10:3 ]\n",
+            "    │\n",
+            " 10 │ \tpz\n",
+            "    │ \t ─\n",
+            "────╯\n",
+        ));
+}
+
+#[test]
+fn diagnostics_skip_scripts_that_are_not_utf8() {
+    // A line that is not valid UTF-8 cannot be quoted: only the message.
+    let mut script = NamedTempFile::new().expect("create temporary sed script");
+    script
+        .write_all(b"s/\xff/x/q\n")
+        .expect("write temporary sed script");
+    let script_path = script.path().to_str().expect("temporary path is UTF-8");
+
+    new_ucmd!()
+        .env("UUTILS_DIAG", "always")
+        .args(&["-f", script_path])
+        .fails()
+        .code_is(1)
+        .stderr_is(format!(
+            "sed: {script_path}:1:7: error: invalid substitute flag: 'q'\n"
+        ));
+}
+
 ////////////////////////////////////////////////////////////
 // Pattern space manipulation: D, d, H, h, N, n, P, p, q, x
 check_output!(pattern_print_to_newline, ["-n", r"1{;N;P;P;p;}", LINES1]);
@@ -2546,6 +2706,74 @@ fn test_step_end_non_posix() {
         .stderr_is("sed: <script argument 1>:1:4: error: ~step is invalid in POSIX mode\n");
 }
 
+#[test]
+fn test_step_without_number_is_zero() {
+    // `N~` and `N,~` select line N alone, like a step of 0.
+    for script in ["3~=", "3,~=", "3,~0="] {
+        new_ucmd!()
+            .args(&["-n", script])
+            .pipe_in("q\nr\ns\nt\n")
+            .succeeds()
+            .stdout_is("3\n");
+    }
+}
+
+#[test]
+fn test_step_match_zero_closes_range() {
+    // `3~0` selects line 3 alone, so `c` prints its text there.
+    new_ucmd!()
+        .args(&["3~0c\\X"])
+        .pipe_in("1\n2\n3\n4\n5\n")
+        .succeeds()
+        .stdout_is("1\n2\nX\n4\n5\n");
+}
+
+#[test]
+fn test_step_non_numeric() {
+    for (script, column) in [("1~/x/p", 6), ("1,~/x/p", 7)] {
+        new_ucmd!()
+            .args(&[script])
+            .fails()
+            .code_is(1)
+            .stderr_is(format!(
+                "sed: <script argument 1>:1:{column}: error: ~step can only be specified through numeric values\n"
+            ));
+    }
+}
+
+#[test]
+fn test_step_without_number_at_end_of_line() {
+    new_ucmd!()
+        .args(&["7~"])
+        .fails()
+        .code_is(1)
+        .stderr_is("sed: <script argument 1>:1:3: error: command expected\n");
+}
+
+#[test]
+fn test_addr0_step_zero() {
+    for script in ["0~0p", "0~p"] {
+        new_ucmd!()
+            .args(&[script])
+            .fails()
+            .code_is(1)
+            .stderr_contains("address 0 can only be used with ~step");
+    }
+}
+
+#[test]
+fn test_comma_without_second_address() {
+    for (script, column) in [("5,", 3), ("5,=", 3), ("5, x", 4)] {
+        new_ucmd!()
+            .args(&[script])
+            .fails()
+            .code_is(1)
+            .stderr_is(format!(
+                "sed: <script argument 1>:1:{column}: error: unexpected `,'\n"
+            ));
+    }
+}
+
 // The following test diverse ways in which regexes are matched.
 // Search for 'regex\.' to find them in the code.
 #[test]