From c0991528100d72fa05e9f37196e280fef3995da6 Mon Sep 17 00:00:00 2001
From: Sylvestre Ledru <sylvestre@debian.org>
Date: Sun, 2 Aug 2026 09:12:57 +0200
Subject: [PATCH] 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                      |  12 +++
 Cargo.toml                      |   4 +-
 docs/src/extensions.md          |  11 +++
 src/sed/error_handling.rs       | 158 +++++++++++++++++++++++++++++--
 src/sed/script_char_provider.rs |  18 ++++
 tests/by-util/test_sed.rs       | 160 ++++++++++++++++++++++++++++++++
 6 files changed, 355 insertions(+), 8 deletions(-)

diff --git a/Cargo.lock b/Cargo.lock
index 53506ad1..d0f92a88 100644
--- a/Cargo.lock
+++ b/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"
@@ -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",
diff --git a/Cargo.toml b/Cargo.toml
index 52a7ab02..cb7e28f2 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -63,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"
 
@@ -81,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 }
 
diff --git a/docs/src/extensions.md b/docs/src/extensions.md
index c81b4b53..3b2a1adf 100644
--- a/docs/src/extensions.md
+++ b/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
diff --git a/src/sed/error_handling.rs b/src/sed/error_handling.rs
index 8f2bb250..e5c2e25d 100644
--- a/src/sed/error_handling.rs
+++ b/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",
+                "───╯",
+            )
+        );
+    }
+}
diff --git a/src/sed/script_char_provider.rs b/src/sed/script_char_provider.rs
index 5c697e7c..7dcc86eb 100644
--- a/src/sed/script_char_provider.rs
+++ b/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)]
diff --git a/tests/by-util/test_sed.rs b/tests/by-util/test_sed.rs
index c604c5a4..8a64ee3d 100644
--- a/tests/by-util/test_sed.rs
+++ b/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]);