Merge pull request #1922 from zed-industries/dont-panic-clip-instead

Julia created 3 years ago

Dont panic in point conversion, clip instead

Change summary

Cargo.lock              |  1 +
crates/rope/Cargo.toml  |  2 +-
crates/rope/src/rope.rs | 39 +++++++++++++++++++++++----------------
crates/util/Cargo.toml  |  1 +
crates/util/src/lib.rs  | 13 +++++++++++++
5 files changed, 39 insertions(+), 17 deletions(-)

Detailed changes

Cargo.lock

@@ -6785,6 +6785,7 @@ name = "util"
 version = "0.1.0"
 dependencies = [
  "anyhow",
+ "backtrace",
  "futures 0.3.24",
  "git2",
  "lazy_static",

crates/rope/Cargo.toml

@@ -12,7 +12,7 @@ smallvec = { version = "1.6", features = ["union"] }
 sum_tree = { path = "../sum_tree" }
 arrayvec = "0.7.1"
 log = { version = "0.4.16", features = ["kv_unstable_serde"] }
-
+util = { path = "../util" }
 
 [dev-dependencies]
 rand = "0.8.3"

crates/rope/src/rope.rs

@@ -12,6 +12,7 @@ use std::{
     str,
 };
 use sum_tree::{Bias, Dimension, SumTree};
+use util::debug_panic;
 
 pub use offset_utf16::OffsetUtf16;
 pub use point::Point;
@@ -679,28 +680,33 @@ impl Chunk {
     fn point_to_offset(&self, target: Point) -> usize {
         let mut offset = 0;
         let mut point = Point::new(0, 0);
+
         for ch in self.0.chars() {
             if point >= target {
                 if point > target {
-                    panic!("point {:?} is inside of character {:?}", target, ch);
+                    debug_panic!("point {target:?} is inside of character {ch:?}");
                 }
                 break;
             }
 
             if ch == '\n' {
                 point.row += 1;
+                point.column = 0;
+
                 if point.row > target.row {
-                    panic!(
-                        "point {:?} is beyond the end of a line with length {}",
-                        target, point.column
+                    debug_panic!(
+                        "point {target:?} is beyond the end of a line with length {}",
+                        point.column
                     );
+                    break;
                 }
-                point.column = 0;
             } else {
                 point.column += ch.len_utf8() as u32;
             }
+
             offset += ch.len_utf8();
         }
+
         offset
     }
 
@@ -737,26 +743,27 @@ impl Chunk {
             if ch == '\n' {
                 point.row += 1;
                 point.column = 0;
+
                 if point.row > target.row {
-                    if clip {
-                        // Return the offset of the newline
-                        return offset;
+                    if !clip {
+                        debug_panic!(
+                            "point {target:?} is beyond the end of a line with length {}",
+                            point.column
+                        );
                     }
-                    panic!(
-                        "point {:?} is beyond the end of a line with length {}",
-                        target, point.column
-                    );
+                    // Return the offset of the newline
+                    return offset;
                 }
             } else {
                 point.column += ch.len_utf16() as u32;
             }
 
             if point > target {
-                if clip {
-                    // Return the offset of the codepoint which we have landed within, bias left
-                    return offset;
+                if !clip {
+                    debug_panic!("point {target:?} is inside of codepoint {ch:?}");
                 }
-                panic!("point {:?} is inside of codepoint {:?}", target, ch);
+                // Return the offset of the codepoint which we have landed within, bias left
+                return offset;
             }
 
             offset += ch.len_utf8();

crates/util/Cargo.toml

@@ -11,6 +11,7 @@ test-support = ["serde_json", "tempdir", "git2"]
 
 [dependencies]
 anyhow = "1.0.38"
+backtrace = "0.3"
 futures = "0.3"
 log = { version = "0.4.16", features = ["kv_unstable_serde"] }
 lazy_static = "1.4.0"

crates/util/src/lib.rs

@@ -1,6 +1,7 @@
 #[cfg(any(test, feature = "test-support"))]
 pub mod test;
 
+pub use backtrace::Backtrace;
 use futures::Future;
 use rand::{seq::SliceRandom, Rng};
 use std::{
@@ -10,6 +11,18 @@ use std::{
     task::{Context, Poll},
 };
 
+#[macro_export]
+macro_rules! debug_panic {
+    ( $($fmt_arg:tt)* ) => {
+        if cfg!(debug_assertions) {
+            panic!( $($fmt_arg)* );
+        } else {
+            let backtrace = $crate::Backtrace::new();
+            log::error!("{}\n{:?}", format_args!($($fmt_arg)*), backtrace);
+        }
+    };
+}
+
 pub fn truncate(s: &str, max_chars: usize) -> &str {
     match s.char_indices().nth(max_chars) {
         None => s,