Fix zero-width regex replace (#815)

As a drive-by this also puts all replace-all
changes into a single undo group.

Tested by:
* Running Replace-All with `$` and "x": Every line gets a trailing "x",
  the final line gets a lone "x" and a new trailing newline is added.
* Running Replace with `$` and "x" repeatedly: New "x"s are added to
  each line incrementally, and it wraps around the buffer.

Closes #775

Co-authored-by: Leonard Hecker <leonard@hecker.io>

Author: MihneaTeodorStoica

crates/edit/src/buffer/mod.rs

@@ -1152,15 +1152,15 @@ impl TextBuffer {
 
         // If the user moved the cursor since the last search, but the needle remained the same,
         // we still need to move the start of the search to the new cursor position.
-        let next_search_offset = match self.selection {
-            Some(TextBufferSelection { beg, end }) => {
-                if self.selection_generation == search.selection_generation {
-                    search.next_search_offset
-                } else {
+        let next_search_offset = if self.selection_generation == search.selection_generation {
+            search.next_search_offset
+        } else {
+            match self.selection {
+                Some(TextBufferSelection { beg, end }) => {
                     self.cursor_move_to_logical_internal(self.cursor, beg.min(end)).offset
                 }
+                _ => self.cursor.offset,
             }
-            _ => self.cursor.offset,
         };
 
         self.find_select_next(search, next_search_offset, true);
@@ -1175,15 +1175,23 @@ impl TextBuffer {
         replacement: &[u8],
     ) -> icu::Result<()> {
         // Editors traditionally replace the previous search hit, not the next possible one.
-        if let (Some(search), Some(..)) = (&self.search, &self.selection) {
+        if let Some(search) = &self.search {
             let search = unsafe { &mut *search.get() };
             if search.selection_generation == self.selection_generation {
                 let scratch = scratch_arena(None);
+                let zero_width = self.selection.is_none();
                 let parsed_replacements =
                     Self::find_parse_replacement(&scratch, &mut *search, replacement);
                 let replacement =
                     self.find_fill_replacement(&mut *search, replacement, &parsed_replacements);
-                self.write(&replacement, self.cursor, true);
+                self.write_raw(&replacement);
+
+                // After replacing a zero-width match, advance past it so that find_and_select wraps to the
+                // next match rather than finding the same anchor (e.g. `$`) again at the same line end.
+                if zero_width {
+                    search.next_search_offset =
+                        self.find_advance_past_zero_width(self.active_edit_off).unwrap_or(0);
+                }
             }
         }
 
@@ -1197,26 +1205,48 @@ impl TextBuffer {
         options: SearchOptions,
         replacement: &[u8],
     ) -> icu::Result<()> {
+        self.edit_begin_grouping();
+
         let scratch = scratch_arena(None);
         let mut search = self.find_construct_search(pattern, options)?;
         let mut offset = 0;
         let parsed_replacements = Self::find_parse_replacement(&scratch, &mut search, replacement);
 
-        loop {
-            self.find_select_next(&mut search, offset, false);
-            if !self.has_selection() {
-                break;
-            }
-
+        while let Some(range) = self.find_select_next(&mut search, offset, false) {
             let replacement =
                 self.find_fill_replacement(&mut search, replacement, &parsed_replacements);
-            self.write(&replacement, self.cursor, true);
-            offset = self.cursor.offset;
+            self.write_raw(&replacement);
+
+            // The `active_edit_off` points to the end of the last edit made by `write_raw()`.
+            // This differs from the self.cursor.offset, if `write_raw()` did an `insert_final_newline`.
+            offset = self.active_edit_off;
+
+            // Avoid infinite loops when hitting zero-length matches
+            // by advancing past the zero-length match location.
+            //
+            // This is technically not entirely correct. For instance imagine replacing
+            // "^|f" with "x" in "foo". It should technically produce "xxoo", but I
+            // found that other editors also do it wrong, so it can't matter too much.
+            if range.is_empty() {
+                offset = match self.find_advance_past_zero_width(offset) {
+                    Some(next) => next,
+                    None => break,
+                };
+            }
         }
 
+        self.edit_end_grouping();
         Ok(())
     }
 
+    /// After replacing a zero-width match, compute the offset to resume
+    /// searching from. Returns `None` if we're at the end of the buffer.
+    fn find_advance_past_zero_width(&self, offset: usize) -> Option<usize> {
+        let cursor = self.cursor_move_to_offset_internal(self.cursor, offset);
+        let next = self.cursor_move_delta_internal(cursor, CursorMovement::Grapheme, 1);
+        (next.offset > offset).then_some(next.offset)
+    }
+
     fn find_construct_search(
         &self,
         pattern: &str,
@@ -1277,7 +1307,12 @@ impl TextBuffer {
         })
     }
 
-    fn find_select_next(&mut self, search: &mut ActiveSearch, offset: usize, wrap: bool) {
+    fn find_select_next(
+        &mut self,
+        search: &mut ActiveSearch,
+        offset: usize,
+        wrap: bool,
+    ) -> Option<Range<usize>> {
         if search.buffer_generation != self.buffer.generation() {
             unsafe { search.regex.set_text(&mut search.text, offset) };
             search.buffer_generation = self.buffer.generation();
@@ -1297,7 +1332,7 @@ impl TextBuffer {
             hit = search.regex.next();
         }
 
-        search.selection_generation = if let Some(range) = hit {
+        search.selection_generation = if let Some(range) = &hit {
             // Now the search offset is no more at the start of the buffer.
             search.next_search_offset = range.end;
 
@@ -1316,6 +1351,8 @@ impl TextBuffer {
             search.no_matches = true;
             self.set_selection(None)
         };
+
+        hit
     }
 
     fn find_parse_replacement<'a>(
@@ -3095,3 +3132,51 @@ fn detect_bom(bytes: &[u8]) -> Option<&'static str> {
     }
     None
 }
+
+#[cfg(test)]
+mod tests {
+    use super::{SearchOptions, TextBuffer};
+
+    fn buffer_contents(buf: &mut TextBuffer) -> String {
+        let mut str = String::new();
+        buf.save_as_string(&mut str);
+        str
+    }
+
+    #[test]
+    fn replace_one_zero_width() {
+        let mut buf = TextBuffer::new(false).unwrap();
+        buf.set_crlf(false);
+        buf.set_insert_final_newline(true);
+        buf.write_raw(b"a\nb\n");
+        buf.cursor_move_to_logical(Default::default());
+
+        for _ in 0..6 {
+            buf.find_and_replace(
+                "$",
+                SearchOptions { use_regex: true, ..Default::default() },
+                b"x",
+            )
+            .unwrap();
+        }
+
+        assert_eq!(buffer_contents(&mut buf), "axx\nbxx\nx\n");
+    }
+
+    #[test]
+    fn replace_all_zero_width() {
+        let mut buf = TextBuffer::new(false).unwrap();
+        buf.set_crlf(false);
+        buf.set_insert_final_newline(true);
+        buf.write_raw(b"a\nb\n");
+
+        buf.find_and_replace_all(
+            "$",
+            SearchOptions { use_regex: true, ..Default::default() },
+            b"x",
+        )
+        .unwrap();
+
+        assert_eq!(buffer_contents(&mut buf), "ax\nbx\nx\n");
+    }
+}

crates/edit/src/icu.rs

@@ -8,6 +8,7 @@ use std::ffi::{CStr, c_char};
 use std::mem::MaybeUninit;
 use std::ops::Range;
 use std::ptr::{null, null_mut};
+use std::sync::OnceLock;
 use std::{fmt, mem};
 
 use stdext::arena::{Arena, scratch_arena};
@@ -995,28 +996,18 @@ const LIBICUI18N_PROC_NAMES: [*const c_char; 12] = [
     proc_name!("uregex_end64"),
 ];
 
-enum LibraryFunctionsState {
-    Uninitialized,
-    Failed,
-    Loaded(LibraryFunctions),
-}
-
-static mut LIBRARY_FUNCTIONS: LibraryFunctionsState = LibraryFunctionsState::Uninitialized;
+static LIBRARY_FUNCTIONS: OnceLock<Option<LibraryFunctions>> = OnceLock::new();
 
 pub fn init() -> Result<()> {
     init_if_needed()?;
     Ok(())
 }
 
-#[allow(static_mut_refs)]
 fn init_if_needed() -> Result<&'static LibraryFunctions> {
-    #[cold]
-    fn load() {
+    fn load() -> Option<LibraryFunctions> {
         unsafe {
-            LIBRARY_FUNCTIONS = LibraryFunctionsState::Failed;
-
             let Ok(icu) = sys::load_icu() else {
-                return;
+                return None;
             };
 
             type TransparentFunction = unsafe extern "C" fn() -> *const ();
@@ -1060,35 +1051,28 @@ fn init_if_needed() -> Result<&'static LibraryFunctions> {
                             "Failed to load ICU function: {:?}",
                             CStr::from_ptr(name)
                         );
-                        return;
+                        return None;
                     };
 
                     ptr.write(func);
                     ptr = ptr.add(1);
                 }
             }
 
-            LIBRARY_FUNCTIONS = LibraryFunctionsState::Loaded(funcs.assume_init());
-        }
-    }
-
-    unsafe {
-        if matches!(&LIBRARY_FUNCTIONS, LibraryFunctionsState::Uninitialized) {
-            load();
+            Some(funcs.assume_init())
         }
     }
 
-    match unsafe { &LIBRARY_FUNCTIONS } {
-        LibraryFunctionsState::Loaded(f) => Ok(f),
-        _ => Err(ICU_MISSING_ERROR),
+    match LIBRARY_FUNCTIONS.get_or_init(load) {
+        Some(f) => Ok(f),
+        None => Err(ICU_MISSING_ERROR),
     }
 }
 
-#[allow(static_mut_refs)]
 fn assume_loaded() -> &'static LibraryFunctions {
-    match unsafe { &LIBRARY_FUNCTIONS } {
-        LibraryFunctionsState::Loaded(f) => f,
-        _ => unreachable!(),
+    match LIBRARY_FUNCTIONS.get() {
+        Some(Some(f)) => f,
+        _ => unsafe { std::hint::unreachable_unchecked() },
     }
 }