Improve truncate efficiency and fix OBOE in truncate_and_remove_front (#22591)

Michael Sloan created 1 year ago

* Skip walking string for truncate when byte len is <= char limit

* Fix `truncate_and_remove_front` returning string that is `max_chars +
1` in length. Now more consistent with `truncate_and_trailoff` behavior.

* Fix `truncate_and_remove_front` adding ellipsis when max_chars == char
length

Release Notes:

- N/A

Change summary

crates/task/src/task_template.rs |  2 
crates/tasks_ui/src/modal.rs     |  4 +-
crates/util/src/util.rs          | 38 +++++++++++++++++++++++++++++----
3 files changed, 36 insertions(+), 8 deletions(-)

Detailed changes

crates/task/src/task_template.rs

@@ -570,7 +570,7 @@ mod tests {
                 spawn_in_terminal.label,
                 format!(
                     "test label for 1234 and …{}",
-                    &long_value[..=MAX_DISPLAY_VARIABLE_LENGTH]
+                    &long_value[long_value.len() - MAX_DISPLAY_VARIABLE_LENGTH..]
                 ),
                 "Human-readable label should have long substitutions trimmed"
             );

crates/tasks_ui/src/modal.rs

@@ -791,7 +791,7 @@ mod tests {
         assert_eq!(
             task_names(&tasks_picker, cx),
             vec![
-                "hello from …th.odd_extension:1:1".to_string(),
+                "hello from …h.odd_extension:1:1".to_string(),
                 "opened now: /dir".to_string()
             ],
             "Second opened buffer should fill the context, labels should be trimmed if long enough"
@@ -820,7 +820,7 @@ mod tests {
         assert_eq!(
             task_names(&tasks_picker, cx),
             vec![
-                "hello from …ithout_extension:2:3".to_string(),
+                "hello from …thout_extension:2:3".to_string(),
                 "opened now: /dir".to_string()
             ],
             "Opened buffer should fill the context, labels should be trimmed if long enough"

crates/util/src/util.rs

@@ -49,10 +49,15 @@ pub fn truncate(s: &str, max_chars: usize) -> &str {
 pub fn truncate_and_trailoff(s: &str, max_chars: usize) -> String {
     debug_assert!(max_chars >= 5);
 
+    // If the string's byte length is <= max_chars, walking the string can be skipped since the
+    // number of chars is <= the number of bytes.
+    if s.len() <= max_chars {
+        return s.to_string();
+    }
     let truncation_ix = s.char_indices().map(|(i, _)| i).nth(max_chars);
     match truncation_ix {
-        Some(length) => s[..length].to_string() + "…",
-        None => s.to_string(),
+        Some(index) => s[..index].to_string() + "…",
+        _ => s.to_string(),
     }
 }
 
@@ -61,10 +66,19 @@ pub fn truncate_and_trailoff(s: &str, max_chars: usize) -> String {
 pub fn truncate_and_remove_front(s: &str, max_chars: usize) -> String {
     debug_assert!(max_chars >= 5);
 
-    let truncation_ix = s.char_indices().map(|(i, _)| i).nth_back(max_chars);
+    // If the string's byte length is <= max_chars, walking the string can be skipped since the
+    // number of chars is <= the number of bytes.
+    if s.len() <= max_chars {
+        return s.to_string();
+    }
+    let suffix_char_length = max_chars.saturating_sub(1);
+    let truncation_ix = s
+        .char_indices()
+        .map(|(i, _)| i)
+        .nth_back(suffix_char_length);
     match truncation_ix {
-        Some(length) => "…".to_string() + &s[length..],
-        None => s.to_string(),
+        Some(index) if index > 0 => "…".to_string() + &s[index..],
+        _ => s.to_string(),
     }
 }
 
@@ -795,11 +809,25 @@ mod tests {
     #[test]
     fn test_truncate_and_trailoff() {
         assert_eq!(truncate_and_trailoff("", 5), "");
+        assert_eq!(truncate_and_trailoff("aaaaaa", 7), "aaaaaa");
+        assert_eq!(truncate_and_trailoff("aaaaaa", 6), "aaaaaa");
+        assert_eq!(truncate_and_trailoff("aaaaaa", 5), "aaaaa…");
         assert_eq!(truncate_and_trailoff("èèèèèè", 7), "èèèèèè");
         assert_eq!(truncate_and_trailoff("èèèèèè", 6), "èèèèèè");
         assert_eq!(truncate_and_trailoff("èèèèèè", 5), "èèèèè…");
     }
 
+    #[test]
+    fn test_truncate_and_remove_front() {
+        assert_eq!(truncate_and_remove_front("", 5), "");
+        assert_eq!(truncate_and_remove_front("aaaaaa", 7), "aaaaaa");
+        assert_eq!(truncate_and_remove_front("aaaaaa", 6), "aaaaaa");
+        assert_eq!(truncate_and_remove_front("aaaaaa", 5), "…aaaaa");
+        assert_eq!(truncate_and_remove_front("èèèèèè", 7), "èèèèèè");
+        assert_eq!(truncate_and_remove_front("èèèèèè", 6), "èèèèèè");
+        assert_eq!(truncate_and_remove_front("èèèèèè", 5), "…èèèèè");
+    }
+
     #[test]
     fn test_numeric_prefix_str_method() {
         let target = "1a";