Sanitize all text before printing

.vscode/launch.json

@@ -33,6 +33,8 @@
             //    "lldb-dap.environment": {
             //        "LLDB_LAUNCH_FLAG_LAUNCH_IN_TTY": "YES"
             //    }
+            // 3. Add the following to your ~/.lldbinit file (replace "stable" with "nightly" if needed):
+            //    command script import ~/.rustup/toolchains/stable-aarch64-apple-darwin/lib/rustlib/etc/lldb_lookup.py
             "name": "Launch edit (lldb-dap, macOS)",
             "preLaunchTask": "rust: cargo build",
             "type": "lldb-dap",

crates/edit/src/bin/edit/main.rs

@@ -11,7 +11,6 @@ mod localization;
 mod settings;
 mod state;
 
-use std::borrow::Cow;
 use std::path::Path;
 use std::time::Duration;
 use std::{env, process};
@@ -32,6 +31,7 @@ use state::*;
 use stdext::arena::{self, Arena, scratch_arena};
 use stdext::arena_format;
 use stdext::collections::{BString, BVec};
+use stdext::unicode::sanitize_control_chars;
 
 use crate::settings::Settings;
 
@@ -445,7 +445,9 @@ fn write_terminal_title<'a>(arena: &'a Arena, output: &mut BString<'a>, state: &
         if dirty {
             output.push_str(arena, "● ");
         }
-        output.push_str(arena, &sanitize_control_chars(filename));
+        let scratch = scratch_arena(Some(arena));
+        let sanitized = sanitize_control_chars(&scratch, filename);
+        output.push_str(arena, &sanitized);
         output.push_str(arena, " - ");
     }
     output.push_str(arena, "edit\x1b\\");
@@ -701,23 +703,3 @@ fn setup_terminal(tui: &mut Tui, state: &mut State, vt_parser: &mut vt::Parser)
 
     RestoreModes
 }
-
-/// Strips all C0 control characters from the string and replaces them with "_".
-///
-/// Jury is still out on whether this should also strip C1 control characters.
-/// That requires parsing UTF8 codepoints, which is annoying.
-fn sanitize_control_chars(text: &str) -> Cow<'_, str> {
-    if let Some(off) = text.bytes().position(|b| (..0x20).contains(&b)) {
-        let mut sanitized = text.to_string();
-        // SAFETY: We only search for ASCII and replace it with ASCII.
-        let vec = unsafe { sanitized.as_bytes_mut() };
-
-        for i in &mut vec[off..] {
-            *i = if (..0x20).contains(i) { b'_' } else { *i }
-        }
-
-        Cow::Owned(sanitized)
-    } else {
-        Cow::Borrowed(text)
-    }
-}

crates/edit/src/buffer/mod.rs

@@ -36,7 +36,6 @@ use std::str;
 pub use gap_buffer::GapBuffer;
 use stdext::arena::{Arena, scratch_arena};
 use stdext::collections::{BString, BVec};
-use stdext::unicode::Utf8Chars;
 use stdext::{ReplaceRange as _, arena_write_fmt, minmax, slice_as_uninit_mut, slice_copy_safe};
 
 use crate::cell::SemiRefCell;
@@ -54,13 +53,13 @@ use crate::{icu, simd};
 /// The margin template is used for line numbers.
 /// The max. line number we should ever expect is probably 64-bit,
 /// and so this template fits 19 digits, followed by " │ ".
-const MARGIN_TEMPLATE: &str = "                    │ ";
+const MARGIN_TEMPLATE: &[u8] = "                    │ ".as_bytes();
 /// Just a bunch of whitespace you can use for turning tabs into spaces.
 /// Happens to reuse MARGIN_TEMPLATE, because it has sufficient whitespace.
-const TAB_WHITESPACE: &str = MARGIN_TEMPLATE;
-const VISUAL_SPACE: &str = "･";
+const TAB_WHITESPACE: &[u8] = MARGIN_TEMPLATE;
+const VISUAL_SPACE: &[u8] = "･".as_bytes();
 const VISUAL_SPACE_PREFIX_ADD: usize = '･'.len_utf8() - 1;
-const VISUAL_TAB: &str = "￫       ";
+const VISUAL_TAB: &[u8] = "￫       ".as_bytes();
 const VISUAL_TAB_PREFIX_ADD: usize = '￫'.len_utf8() - 1;
 
 pub enum IoError {
@@ -1791,7 +1790,6 @@ impl TextBuffer {
         let height = destination.height();
         let line_number_width = self.margin_width.max(3) as usize - 3;
         let text_width = width - self.margin_width;
-        let mut visualizer_buf = [0xE2, 0x90, 0x80]; // U+2400 in UTF8
         let mut visual_pos_x_max = 0;
 
         // Pick the cursor closer to the `origin.y`.
@@ -1810,7 +1808,7 @@ impl TextBuffer {
 
         for y in 0..height {
             let scratch = scratch_arena(None);
-            let mut line = BString::empty();
+            let mut line = BVec::empty();
             line.reserve(&*scratch, width as usize * 2);
 
             let visual_line = origin.y + y;
@@ -1834,7 +1832,7 @@ impl TextBuffer {
                     // because `line_number_width` can't possibly be larger than 19.
                     let off = 19 - line_number_width;
                     unsafe { std::hint::assert_unchecked(off < MARGIN_TEMPLATE.len()) };
-                    line.push_str(&*scratch, &MARGIN_TEMPLATE[off..]);
+                    line.extend_from_slice(&*scratch, &MARGIN_TEMPLATE[off..]);
                 } else if self.word_wrap_column <= 0 || cursor_beg.logical_pos.x == 0 {
                     // Regular line? Place "123 | " in the margin.
                     arena_write_fmt!(
@@ -1939,7 +1937,7 @@ impl TextBuffer {
                     if cursor_next.visual_pos.x > origin.x {
                         let overlap = cursor_next.visual_pos.x - origin.x;
                         debug_assert!((1..=7).contains(&overlap));
-                        line.push_str(&*scratch, &TAB_WHITESPACE[..overlap as usize]);
+                        line.extend_from_slice(&*scratch, &TAB_WHITESPACE[..overlap as usize]);
                         cursor_beg = cursor_next;
                     }
                 }
@@ -1950,27 +1948,27 @@ impl TextBuffer {
                 while global_off < cursor_end.offset {
                     let chunk = self.read_forward(global_off);
                     let chunk = &chunk[..chunk.len().min(cursor_end.offset - global_off)];
-                    let mut it = Utf8Chars::new(chunk, 0);
-
-                    // TODO: Looping char-by-char is bad for performance.
-                    // >25% of the total rendering time is spent here.
-                    loop {
-                        let chunk_off = it.offset();
-                        let global_off = global_off + chunk_off;
-                        let Some(ch) = it.next() else {
-                            break;
-                        };
-
-                        if ch == ' ' || ch == '\t' {
-                            let is_tab = ch == '\t';
+                    let mut off = 0;
+
+                    while off < chunk.len() {
+                        let beg = off;
+                        off = memchr2(b' ', b'\t', chunk, off);
+
+                        // Anything that isn't whitespace is copied as-is.
+                        // The framebuffer takes care of sanitizing it.
+                        line.extend_from_slice(&*scratch, &chunk[beg..off]);
+
+                        while off < chunk.len() && matches!(chunk[off], b' ' | b'\t') {
+                            let is_tab = chunk[off] == b'\t';
+                            let global_off = global_off + off;
                             let visualize = selection_off.contains(&global_off);
                             let mut whitespace = TAB_WHITESPACE;
                             let mut prefix_add = 0;
 
                             if is_tab || visualize {
                                 // We need the character's visual position in order to either compute the tab size,
                                 // or set the foreground color of the visualizer, respectively.
-                                // TODO: Doing this char-by-char is of course also bad for performance.
+                                // TODO: Doing this char-by-char is bad for performance.
                                 cursor_line =
                                     self.cursor_move_to_offset_internal(cursor_line, global_off);
                             }
@@ -2002,38 +2000,11 @@ impl TextBuffer {
                                 );
                             }
 
-                            line.push_str(&*scratch, &whitespace[..prefix_add + tab_size as usize]);
-                        } else if ch <= '\x1f' || ('\u{7f}'..='\u{9f}').contains(&ch) {
-                            // Append a Unicode representation of the C0 or C1 control character.
-                            visualizer_buf[2] = if ch <= '\x1f' {
-                                0x80 | ch as u8 // U+2400..=U+241F
-                            } else if ch == '\x7f' {
-                                0xA1 // U+2421
-                            } else {
-                                0xA6 // U+2426, because there are no pictures for C1 control characters.
-                            };
-
-                            // Our manually constructed UTF8 is never going to be invalid. Trust.
-                            line.push_str(&*scratch, unsafe {
-                                str::from_utf8_unchecked(&visualizer_buf)
-                            });
-
-                            // Highlight the control character yellow.
-                            cursor_line =
-                                self.cursor_move_to_offset_internal(cursor_line, global_off);
-                            let visualizer_rect = {
-                                let left =
-                                    destination.left + self.margin_width + cursor_line.visual_pos.x
-                                        - origin.x;
-                                let top = destination.top + cursor_line.visual_pos.y - origin.y;
-                                Rect { left, top, right: left + 1, bottom: top + 1 }
-                            };
-                            let bg = fb.indexed(IndexedColor::Yellow);
-                            let fg = fb.contrasted(bg);
-                            fb.blend_bg(visualizer_rect, bg);
-                            fb.blend_fg(visualizer_rect, fg);
-                        } else {
-                            line.push(&*scratch, ch);
+                            line.extend_from_slice(
+                                &*scratch,
+                                &whitespace[..prefix_add + tab_size as usize],
+                            );
+                            off += 1;
                         }
                     }
 
@@ -2377,7 +2348,7 @@ impl TextBuffer {
                 // Now replace tabs with spaces.
                 while line_off < line.len() && line[line_off] == b'\t' {
                     let spaces = self.tab_size_eval(self.cursor.column);
-                    let spaces = &TAB_WHITESPACE.as_bytes()[..spaces as usize];
+                    let spaces = &TAB_WHITESPACE[..spaces as usize];
                     self.edit_write(spaces);
                     line_off += 1;
                 }

crates/edit/src/framebuffer.rs

@@ -8,10 +8,11 @@ use std::ops::{BitOr, BitXor};
 use std::ptr;
 use std::slice::ChunksExact;
 
-use stdext::arena::Arena;
-use stdext::arena_write_fmt;
+use stdext::arena::{Arena, scratch_arena};
 use stdext::collections::BString;
 use stdext::simd::memset;
+use stdext::unicode::{SanitizedControlChars, sanitize_control_chars};
+use stdext::{MaybeOwned, arena_write_fmt};
 
 use crate::helpers::{CoordType, Point, Rect, Size};
 use crate::oklab::StraightRgba;
@@ -195,15 +196,61 @@ impl Framebuffer {
     /// Replaces text contents in a single line of the framebuffer.
     /// All coordinates are in viewport coordinates.
     /// Assumes that control characters have been replaced or escaped.
+    #[inline]
     pub fn replace_text(
         &mut self,
         y: CoordType,
         origin_x: CoordType,
         clip_right: CoordType,
-        text: &str,
+        text: &(impl AsRef<[u8]> + ?Sized),
+    ) {
+        self.replace_text_impl(y, origin_x, clip_right, text.as_ref());
+    }
+
+    fn replace_text_impl(
+        &mut self,
+        y: CoordType,
+        origin_x: CoordType,
+        clip_right: CoordType,
+        text: &[u8],
     ) {
+        let scratch = scratch_arena(None);
+        let sanitized = sanitize_control_chars(&scratch, text);
+
         let back = &mut self.buffers[self.frame_counter & 1];
-        back.text.replace_text(y, origin_x, clip_right, text)
+        back.text.replace_text(y, origin_x, clip_right, &sanitized);
+
+        if let MaybeOwned::Owned(sanitized) = &sanitized {
+            self.highlight_sanitized(y, origin_x, clip_right, sanitized);
+        }
+    }
+
+    /// Highlights the replacements that [`sanitize_control_chars`] made in yellow.
+    #[cold]
+    fn highlight_sanitized(
+        &mut self,
+        y: CoordType,
+        origin_x: CoordType,
+        clip_right: CoordType,
+        sanitized: &SanitizedControlChars,
+    ) {
+        let bg = self.indexed(IndexedColor::Yellow);
+        let fg = self.contrasted(bg);
+        let text = sanitized.text.as_bytes();
+        let mut cfg = MeasurementConfig::new(&text);
+
+        for range in sanitized.unsane_ranges.iter() {
+            // The ranges are sorted, so once we're past the right edge we're done.
+            let left = origin_x + cfg.goto_offset(range.start).visual_pos.x;
+            if left >= clip_right {
+                break;
+            }
+
+            let right = origin_x + cfg.goto_offset(range.end).visual_pos.x;
+            let rect = Rect { left, top: y, right: right.min(clip_right), bottom: y + 1 };
+            self.blend_bg(rect, bg);
+            self.blend_fg(rect, fg);
+        }
     }
 
     /// Draws a scrollbar in the given `track` rectangle.
@@ -307,15 +354,11 @@ impl Framebuffer {
         let mut fract_buf = [0xE2, 0x96, 0x88];
         if top_fract != 0 {
             fract_buf[2] = (0x88 - top_fract) as u8;
-            self.replace_text(thumb_top - 1, track_clipped.left, track_clipped.right, unsafe {
-                std::str::from_utf8_unchecked(&fract_buf)
-            });
+            self.replace_text(thumb_top - 1, track_clipped.left, track_clipped.right, &fract_buf);
         }
         if bottom_fract != 0 {
             fract_buf[2] = (0x88 - bottom_fract) as u8;
-            self.replace_text(thumb_bottom, track_clipped.left, track_clipped.right, unsafe {
-                std::str::from_utf8_unchecked(&fract_buf)
-            });
+            self.replace_text(thumb_bottom, track_clipped.left, track_clipped.right, &fract_buf);
             let rect = Rect {
                 left: track_clipped.left,
                 top: thumb_bottom,

crates/stdext/src/collections/string.rs

@@ -1,9 +1,9 @@
 // Copyright (c) Microsoft Corporation.
 // Licensed under the MIT License.
 
+use std::borrow::Borrow;
 use std::fmt::{self};
 use std::ops::{Bound, Deref, DerefMut, RangeBounds};
-use std::slice;
 use std::str::Utf8Error;
 
 use crate::alloc::Allocator;
@@ -158,22 +158,16 @@ impl<'a> BString<'a> {
 
     /// Appends a single `char`, encoding it as UTF-8.
     pub fn push(&mut self, alloc: &'a dyn Allocator, ch: char) {
-        self.reserve(alloc, 4);
-        unsafe {
-            let len = self.vec.len();
-            let dst = self.vec.as_mut_ptr().add(len);
-            let add = ch.encode_utf8(slice::from_raw_parts_mut(dst, 4)).len();
-            self.vec.set_len(len + add);
-        }
+        self.vec.push_char(alloc, ch);
     }
 
     /// Empties the string. The allocation is kept.
     pub fn clear(&mut self) {
         self.vec.clear();
     }
 
-    /// Returns a [`BorrowedStringFormatter`] pairing this string with an allocator,
-    /// enabling use with `write!` and `fmt::Write`.
+    /// Pairs this instance with an allocator, making it possible to
+    /// use `write!` and `fmt::Write`, which are allocator-unaware.
     pub fn formatter<A>(&mut self, alloc: &'a A) -> BStringFormatter<'_, 'a, A>
     where
         A: Allocator,
@@ -287,6 +281,27 @@ impl DerefMut for BString<'_> {
     }
 }
 
+impl AsRef<str> for BString<'_> {
+    #[inline]
+    fn as_ref(&self) -> &str {
+        self.as_str()
+    }
+}
+
+impl AsRef<[u8]> for BString<'_> {
+    #[inline]
+    fn as_ref(&self) -> &[u8] {
+        self.as_bytes()
+    }
+}
+
+impl Borrow<str> for BString<'_> {
+    #[inline]
+    fn borrow(&self) -> &str {
+        self.as_str()
+    }
+}
+
 impl PartialEq<BString<'_>> for BString<'_> {
     #[inline]
     fn eq(&self, other: &BString) -> bool {