From 702b1d0a0fc483935882cbd451a6898fa8f895eb Mon Sep 17 00:00:00 2001
From: Michael Davis <mcarsondavis@gmail.com>
Date: Wed, 14 May 2025 18:27:46 -0400
Subject: [PATCH] statusline: Avoid unnecessary allocations for `&'static str`
 spans
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Previously the statusline `write` function only accepted a string
and optional Style, so all rendering functions converted text to
strings. Some elements write spans with `&'static str`s, however, making
this unnecessary since `Span<'a>` is a wrapper around `Cow<'a, str>` and
style, and a `Span<'static>` would outlive all required lifetimes.

Moreover many elements could produce `Span<'a>` according to the
lifetime in `RenderContext` in the future, potentially re-borrowing from
the Editor borrow, so this change could save allocations for many
file-type elements (with more future changes). This is not explored in
this patch since the statusline functions currently add bespoke padding
per-element, but with a future refactor to make spacing consistent this
could be possible.

This change refactors the write function to accept a `Span<'a>` and
rewrites some related code to fit the codebase better (preferring `for`
to iterator's `for_each` for example). The new code is more complicated
lifetime-wise but avoids allocations in these cases:

* spacer for mode name when a pane is not focused
* LSP spinner frames
* '●' (workspace) diagnostic indicators
* " W " workspace diagnostic prefix
* file modification indicators
* read-only indicators
* spacer element

... and opens the door to avoid allocation for file name elements in the
future.
---
 helix-term/src/ui/statusline.rs | 291 ++++++++++++++------------------
 1 file changed, 126 insertions(+), 165 deletions(-)

diff --git a/helix-term/src/ui/statusline.rs b/helix-term/src/ui/statusline.rs
index 117483e26..4d4b9f2fe 100644
--- a/helix-term/src/ui/statusline.rs
+++ b/helix-term/src/ui/statusline.rs
@@ -1,3 +1,5 @@
+use std::borrow::Cow;
+
 use helix_core::{coords_at_pos, encoding, Position};
 use helix_lsp::lsp::DiagnosticSeverity;
 use helix_view::document::DEFAULT_LANGUAGE_NAME;
@@ -58,25 +60,16 @@ pub fn render(context: &mut RenderContext, viewport: Rect, surface: &mut Surface
 
     surface.set_style(viewport.with_height(1), base_style);
 
-    let write_left = |context: &mut RenderContext, text, style| {
-        append(&mut context.parts.left, text, &base_style, style)
-    };
-    let write_center = |context: &mut RenderContext, text, style| {
-        append(&mut context.parts.center, text, &base_style, style)
-    };
-    let write_right = |context: &mut RenderContext, text, style| {
-        append(&mut context.parts.right, text, &base_style, style)
-    };
-
     // Left side of the status line.
 
     let config = context.editor.config();
 
-    let element_ids = &config.statusline.left;
-    element_ids
-        .iter()
-        .map(|element_id| get_render_function(*element_id))
-        .for_each(|render| render(context, write_left));
+    for element_id in &config.statusline.left {
+        let render = get_render_function(*element_id);
+        (render)(context, |context, span| {
+            append(&mut context.parts.left, span, base_style)
+        });
+    }
 
     surface.set_spans(
         viewport.x,
@@ -87,11 +80,12 @@ pub fn render(context: &mut RenderContext, viewport: Rect, surface: &mut Surface
 
     // Right side of the status line.
 
-    let element_ids = &config.statusline.right;
-    element_ids
-        .iter()
-        .map(|element_id| get_render_function(*element_id))
-        .for_each(|render| render(context, write_right));
+    for element_id in &config.statusline.right {
+        let render = get_render_function(*element_id);
+        (render)(context, |context, span| {
+            append(&mut context.parts.right, span, base_style)
+        })
+    }
 
     surface.set_spans(
         viewport.x
@@ -105,11 +99,12 @@ pub fn render(context: &mut RenderContext, viewport: Rect, surface: &mut Surface
 
     // Center of the status line.
 
-    let element_ids = &config.statusline.center;
-    element_ids
-        .iter()
-        .map(|element_id| get_render_function(*element_id))
-        .for_each(|render| render(context, write_center));
+    for element_id in &config.statusline.center {
+        let render = get_render_function(*element_id);
+        (render)(context, |context, span| {
+            append(&mut context.parts.center, span, base_style)
+        })
+    }
 
     // Width of the empty space between the left and center area and between the center and right area.
     let spacing = 1u16;
@@ -126,16 +121,14 @@ pub fn render(context: &mut RenderContext, viewport: Rect, surface: &mut Surface
     );
 }
 
-fn append(buffer: &mut Spans, text: String, base_style: &Style, style: Option<Style>) {
-    buffer.0.push(Span::styled(
-        text,
-        style.map_or(*base_style, |s| (*base_style).patch(s)),
-    ));
+fn append<'a>(buffer: &mut Spans<'a>, mut span: Span<'a>, base_style: Style) {
+    span.style = base_style.patch(span.style);
+    buffer.0.push(span);
 }
 
-fn get_render_function<F>(element_id: StatusLineElementID) -> impl Fn(&mut RenderContext, F)
+fn get_render_function<'a, F>(element_id: StatusLineElementID) -> impl Fn(&mut RenderContext<'a>, F)
 where
-    F: Fn(&mut RenderContext, String, Option<Style>) + Copy,
+    F: Fn(&mut RenderContext<'a>, Span<'a>) + Copy,
 {
     match element_id {
         helix_view::editor::StatusLineElement::Mode => render_mode,
@@ -166,44 +159,42 @@ where
     }
 }
 
-fn render_mode<F>(context: &mut RenderContext, write: F)
+fn render_mode<'a, F>(context: &mut RenderContext<'a>, write: F)
 where
-    F: Fn(&mut RenderContext, String, Option<Style>) + Copy,
+    F: Fn(&mut RenderContext<'a>, Span<'a>) + Copy,
 {
     let visible = context.focused;
     let config = context.editor.config();
     let modenames = &config.statusline.mode;
-    write(
-        context,
-        format!(
+    let content = if visible {
+        Cow::Owned(format!(
             " {} ",
-            if visible {
-                match context.editor.mode() {
-                    Mode::Insert => &modenames.insert,
-                    Mode::Select => &modenames.select,
-                    Mode::Normal => &modenames.normal,
-                }
-            } else {
-                // If not focused, explicitly leave an empty space instead of returning None.
-                "   "
-            }
-        ),
-        if visible && config.color_modes {
             match context.editor.mode() {
-                Mode::Insert => Some(context.editor.theme.get("ui.statusline.insert")),
-                Mode::Select => Some(context.editor.theme.get("ui.statusline.select")),
-                Mode::Normal => Some(context.editor.theme.get("ui.statusline.normal")),
+                Mode::Insert => &modenames.insert,
+                Mode::Select => &modenames.select,
+                Mode::Normal => &modenames.normal,
             }
-        } else {
-            None
-        },
-    );
+        ))
+    } else {
+        // If not focused, explicitly leave an empty space instead of returning None.
+        Cow::Borrowed("     ")
+    };
+    let style = if visible && config.color_modes {
+        match context.editor.mode() {
+            Mode::Insert => context.editor.theme.get("ui.statusline.insert"),
+            Mode::Select => context.editor.theme.get("ui.statusline.select"),
+            Mode::Normal => context.editor.theme.get("ui.statusline.normal"),
+        }
+    } else {
+        Style::default()
+    };
+    write(context, Span::styled(content, style));
 }
 
 // TODO think about handling multiple language servers
-fn render_lsp_spinner<F>(context: &mut RenderContext, write: F)
+fn render_lsp_spinner<'a, F>(context: &mut RenderContext<'a>, write: F)
 where
-    F: Fn(&mut RenderContext, String, Option<Style>) + Copy,
+    F: Fn(&mut RenderContext<'a>, Span<'a>) + Copy,
 {
     let language_server = context.doc.language_servers().next();
     write(
@@ -217,14 +208,13 @@ where
             })
             // Even if there's no spinner; reserve its space to avoid elements frequently shifting.
             .unwrap_or(" ")
-            .to_string(),
-        None,
+            .into(),
     );
 }
 
-fn render_diagnostics<F>(context: &mut RenderContext, write: F)
+fn render_diagnostics<'a, F>(context: &mut RenderContext<'a>, write: F)
 where
-    F: Fn(&mut RenderContext, String, Option<Style>) + Copy,
+    F: Fn(&mut RenderContext<'a>, Span<'a>) + Copy,
 {
     use helix_core::diagnostic::Severity;
     let (hints, info, warnings, errors) =
@@ -245,45 +235,35 @@ where
     for sev in &context.editor.config().statusline.diagnostics {
         match sev {
             Severity::Hint if hints > 0 => {
-                write(
-                    context,
-                    "●".to_string(),
-                    Some(context.editor.theme.get("hint")),
-                );
-                write(context, format!(" {} ", hints), None);
+                write(context, Span::styled("●", context.editor.theme.get("hint")));
+                write(context, format!(" {} ", hints).into());
             }
             Severity::Info if info > 0 => {
-                write(
-                    context,
-                    "●".to_string(),
-                    Some(context.editor.theme.get("info")),
-                );
-                write(context, format!(" {} ", info), None);
+                write(context, Span::styled("●", context.editor.theme.get("info")));
+                write(context, format!(" {} ", info).into());
             }
             Severity::Warning if warnings > 0 => {
                 write(
                     context,
-                    "●".to_string(),
-                    Some(context.editor.theme.get("warning")),
+                    Span::styled("●", context.editor.theme.get("warning")),
                 );
-                write(context, format!(" {} ", warnings), None);
+                write(context, format!(" {} ", warnings).into());
             }
             Severity::Error if errors > 0 => {
                 write(
                     context,
-                    "●".to_string(),
-                    Some(context.editor.theme.get("error")),
+                    Span::styled("●", context.editor.theme.get("error")),
                 );
-                write(context, format!(" {} ", errors), None);
+                write(context, format!(" {} ", errors).into());
             }
             _ => {}
         }
     }
 }
 
-fn render_workspace_diagnostics<F>(context: &mut RenderContext, write: F)
+fn render_workspace_diagnostics<'a, F>(context: &mut RenderContext<'a>, write: F)
 where
-    F: Fn(&mut RenderContext, String, Option<Style>) + Copy,
+    F: Fn(&mut RenderContext<'a>, Span<'a>) + Copy,
 {
     use helix_core::diagnostic::Severity;
     let (hints, info, warnings, errors) = context.editor.diagnostics.values().flatten().fold(
@@ -317,68 +297,56 @@ where
         return;
     }
 
-    write(context, " W ".into(), None);
+    write(context, " W ".into());
 
     for sev in sevs_to_show {
         match sev {
             Severity::Hint if hints > 0 => {
-                write(
-                    context,
-                    "●".to_string(),
-                    Some(context.editor.theme.get("hint")),
-                );
-                write(context, format!(" {} ", hints), None);
+                write(context, Span::styled("●", context.editor.theme.get("hint")));
+                write(context, format!(" {} ", hints).into());
             }
             Severity::Info if info > 0 => {
-                write(
-                    context,
-                    "●".to_string(),
-                    Some(context.editor.theme.get("info")),
-                );
-                write(context, format!(" {} ", info), None);
+                write(context, Span::styled("●", context.editor.theme.get("info")));
+                write(context, format!(" {} ", info).into());
             }
             Severity::Warning if warnings > 0 => {
                 write(
                     context,
-                    "●".to_string(),
-                    Some(context.editor.theme.get("warning")),
+                    Span::styled("●", context.editor.theme.get("warning")),
                 );
-                write(context, format!(" {} ", warnings), None);
+                write(context, format!(" {} ", warnings).into());
             }
             Severity::Error if errors > 0 => {
                 write(
                     context,
-                    "●".to_string(),
-                    Some(context.editor.theme.get("error")),
+                    Span::styled("●", context.editor.theme.get("error")),
                 );
-                write(context, format!(" {} ", errors), None);
+                write(context, format!(" {} ", errors).into());
             }
             _ => {}
         }
     }
 }
 
-fn render_selections<F>(context: &mut RenderContext, write: F)
+fn render_selections<'a, F>(context: &mut RenderContext<'a>, write: F)
 where
-    F: Fn(&mut RenderContext, String, Option<Style>) + Copy,
+    F: Fn(&mut RenderContext<'a>, Span<'a>) + Copy,
 {
     let count = context.doc.selection(context.view.id).len();
     write(
         context,
-        format!(" {} sel{} ", count, if count == 1 { "" } else { "s" }),
-        None,
+        format!(" {} sel{} ", count, if count == 1 { "" } else { "s" }).into(),
     );
 }
 
-fn render_primary_selection_length<F>(context: &mut RenderContext, write: F)
+fn render_primary_selection_length<'a, F>(context: &mut RenderContext<'a>, write: F)
 where
-    F: Fn(&mut RenderContext, String, Option<Style>) + Copy,
+    F: Fn(&mut RenderContext<'a>, Span<'a>) + Copy,
 {
     let tot_sel = context.doc.selection(context.view.id).primary().len();
     write(
         context,
-        format!(" {} char{} ", tot_sel, if tot_sel == 1 { "" } else { "s" }),
-        None,
+        format!(" {} char{} ", tot_sel, if tot_sel == 1 { "" } else { "s" }).into(),
     );
 }
 
@@ -393,54 +361,52 @@ fn get_position(context: &RenderContext) -> Position {
     )
 }
 
-fn render_position<F>(context: &mut RenderContext, write: F)
+fn render_position<'a, F>(context: &mut RenderContext<'a>, write: F)
 where
-    F: Fn(&mut RenderContext, String, Option<Style>) + Copy,
+    F: Fn(&mut RenderContext<'a>, Span<'a>) + Copy,
 {
     let position = get_position(context);
     write(
         context,
-        format!(" {}:{} ", position.row + 1, position.col + 1),
-        None,
+        format!(" {}:{} ", position.row + 1, position.col + 1).into(),
     );
 }
 
-fn render_total_line_numbers<F>(context: &mut RenderContext, write: F)
+fn render_total_line_numbers<'a, F>(context: &mut RenderContext<'a>, write: F)
 where
-    F: Fn(&mut RenderContext, String, Option<Style>) + Copy,
+    F: Fn(&mut RenderContext<'a>, Span<'a>) + Copy,
 {
     let total_line_numbers = context.doc.text().len_lines();
 
-    write(context, format!(" {} ", total_line_numbers), None);
+    write(context, format!(" {} ", total_line_numbers).into());
 }
 
-fn render_position_percentage<F>(context: &mut RenderContext, write: F)
+fn render_position_percentage<'a, F>(context: &mut RenderContext<'a>, write: F)
 where
-    F: Fn(&mut RenderContext, String, Option<Style>) + Copy,
+    F: Fn(&mut RenderContext<'a>, Span<'a>) + Copy,
 {
     let position = get_position(context);
     let maxrows = context.doc.text().len_lines();
     write(
         context,
-        format!("{}%", (position.row + 1) * 100 / maxrows),
-        None,
+        format!("{}%", (position.row + 1) * 100 / maxrows).into(),
     );
 }
 
-fn render_file_encoding<F>(context: &mut RenderContext, write: F)
+fn render_file_encoding<'a, F>(context: &mut RenderContext<'a>, write: F)
 where
-    F: Fn(&mut RenderContext, String, Option<Style>) + Copy,
+    F: Fn(&mut RenderContext<'a>, Span<'a>) + Copy,
 {
     let enc = context.doc.encoding();
 
     if enc != encoding::UTF_8 {
-        write(context, format!(" {} ", enc.name()), None);
+        write(context, format!(" {} ", enc.name()).into());
     }
 }
 
-fn render_file_line_ending<F>(context: &mut RenderContext, write: F)
+fn render_file_line_ending<'a, F>(context: &mut RenderContext<'a>, write: F)
 where
-    F: Fn(&mut RenderContext, String, Option<Style>) + Copy,
+    F: Fn(&mut RenderContext<'a>, Span<'a>) + Copy,
 {
     use helix_core::LineEnding::*;
     let line_ending = match context.doc.line_ending {
@@ -460,21 +426,21 @@ where
         PS => "PS", // U+2029 -- ParagraphSeparator
     };
 
-    write(context, format!(" {} ", line_ending), None);
+    write(context, format!(" {} ", line_ending).into());
 }
 
-fn render_file_type<F>(context: &mut RenderContext, write: F)
+fn render_file_type<'a, F>(context: &mut RenderContext<'a>, write: F)
 where
-    F: Fn(&mut RenderContext, String, Option<Style>) + Copy,
+    F: Fn(&mut RenderContext<'a>, Span<'a>) + Copy,
 {
     let file_type = context.doc.language_name().unwrap_or(DEFAULT_LANGUAGE_NAME);
 
-    write(context, format!(" {} ", file_type), None);
+    write(context, format!(" {} ", file_type).into());
 }
 
-fn render_file_name<F>(context: &mut RenderContext, write: F)
+fn render_file_name<'a, F>(context: &mut RenderContext<'a>, write: F)
 where
-    F: Fn(&mut RenderContext, String, Option<Style>) + Copy,
+    F: Fn(&mut RenderContext<'a>, Span<'a>) + Copy,
 {
     let title = {
         let rel_path = context.doc.relative_path();
@@ -485,12 +451,12 @@ where
         format!(" {} ", path)
     };
 
-    write(context, title, None);
+    write(context, title.into());
 }
 
-fn render_file_absolute_path<F>(context: &mut RenderContext, write: F)
+fn render_file_absolute_path<'a, F>(context: &mut RenderContext<'a>, write: F)
 where
-    F: Fn(&mut RenderContext, String, Option<Style>) + Copy,
+    F: Fn(&mut RenderContext<'a>, Span<'a>) + Copy,
 {
     let title = {
         let path = context.doc.path();
@@ -501,39 +467,37 @@ where
         format!(" {} ", path)
     };
 
-    write(context, title, None);
+    write(context, title.into());
 }
 
-fn render_file_modification_indicator<F>(context: &mut RenderContext, write: F)
+fn render_file_modification_indicator<'a, F>(context: &mut RenderContext<'a>, write: F)
 where
-    F: Fn(&mut RenderContext, String, Option<Style>) + Copy,
+    F: Fn(&mut RenderContext<'a>, Span<'a>) + Copy,
 {
-    let title = (if context.doc.is_modified() {
+    let title = if context.doc.is_modified() {
         "[+]"
     } else {
         "   "
-    })
-    .to_string();
+    };
 
-    write(context, title, None);
+    write(context, title.into());
 }
 
-fn render_read_only_indicator<F>(context: &mut RenderContext, write: F)
+fn render_read_only_indicator<'a, F>(context: &mut RenderContext<'a>, write: F)
 where
-    F: Fn(&mut RenderContext, String, Option<Style>) + Copy,
+    F: Fn(&mut RenderContext<'a>, Span<'a>) + Copy,
 {
     let title = if context.doc.readonly {
         " [readonly] "
     } else {
         ""
-    }
-    .to_string();
-    write(context, title, None);
+    };
+    write(context, title.into());
 }
 
-fn render_file_base_name<F>(context: &mut RenderContext, write: F)
+fn render_file_base_name<'a, F>(context: &mut RenderContext<'a>, write: F)
 where
-    F: Fn(&mut RenderContext, String, Option<Style>) + Copy,
+    F: Fn(&mut RenderContext<'a>, Span<'a>) + Copy,
 {
     let title = {
         let rel_path = context.doc.relative_path();
@@ -544,32 +508,29 @@ where
         format!(" {} ", path)
     };
 
-    write(context, title, None);
+    write(context, title.into());
 }
 
-fn render_separator<F>(context: &mut RenderContext, write: F)
+fn render_separator<'a, F>(context: &mut RenderContext<'a>, write: F)
 where
-    F: Fn(&mut RenderContext, String, Option<Style>) + Copy,
+    F: Fn(&mut RenderContext<'a>, Span<'a>) + Copy,
 {
     let sep = &context.editor.config().statusline.separator;
+    let style = context.editor.theme.get("ui.statusline.separator");
 
-    write(
-        context,
-        sep.to_string(),
-        Some(context.editor.theme.get("ui.statusline.separator")),
-    );
+    write(context, Span::styled(sep.to_string(), style));
 }
 
-fn render_spacer<F>(context: &mut RenderContext, write: F)
+fn render_spacer<'a, F>(context: &mut RenderContext<'a>, write: F)
 where
-    F: Fn(&mut RenderContext, String, Option<Style>) + Copy,
+    F: Fn(&mut RenderContext<'a>, Span<'a>) + Copy,
 {
-    write(context, String::from(" "), None);
+    write(context, " ".into());
 }
 
-fn render_version_control<F>(context: &mut RenderContext, write: F)
+fn render_version_control<'a, F>(context: &mut RenderContext<'a>, write: F)
 where
-    F: Fn(&mut RenderContext, String, Option<Style>) + Copy,
+    F: Fn(&mut RenderContext<'a>, Span<'a>) + Copy,
 {
     let head = context
         .doc
@@ -577,14 +538,14 @@ where
         .unwrap_or_default()
         .to_string();
 
-    write(context, head, None);
+    write(context, head.into());
 }
 
-fn render_register<F>(context: &mut RenderContext, write: F)
+fn render_register<'a, F>(context: &mut RenderContext<'a>, write: F)
 where
-    F: Fn(&mut RenderContext, String, Option<Style>) + Copy,
+    F: Fn(&mut RenderContext<'a>, Span<'a>) + Copy,
 {
     if let Some(reg) = context.editor.selected_register {
-        write(context, format!(" reg={} ", reg), None)
+        write(context, format!(" reg={} ", reg).into())
     }
 }