From 3318953bf6b3f7c054e971aec61bf22fd2a05104 Mon Sep 17 00:00:00 2001 From: Michael Davis Date: Wed, 15 Jan 2025 10:57:03 -0500 Subject: [PATCH] minor: Use more exact allocations in `insert_newline` This is partially a style commit: * Pull more bindings out the `change_by_selection` closure like the line-ending string and the comment tokens used for continuation. * Prefer `Editor::config` to `Document`'s config. The rest is changes to places where `insert_newline` may allocate. The first is to move `new_text` out of the `change_by_selection` closure, reusing it between iterations. This is not necessarily always an improvement as we need to clone the text for the return type of the closure. `SmartString`'s `From` implementation reuses the allocation when the string is too long to inline and drops it if it is short enough to inline though which can be wasteful. `From<&String>` clones the string's allocation only when it is too long to be inlined, so we save on allocations for any `new_text` short enough to be inlined. The rest is changes to `new_text.reserve_exact`. Previously calls to this function in this block mixed up character and byte indexing by treating the length of the line-ending as 1. `reserve_exact` takes a number of bytes to reserve and that may be 2 when `line_ending` is a CRLF. A call to `reserve_exact` is also added to the branch used when continuing line comments. --- helix-term/src/commands.rs | 43 ++++++++++++++++++++++---------------- 1 file changed, 25 insertions(+), 18 deletions(-) diff --git a/helix-term/src/commands.rs b/helix-term/src/commands.rs index 9ef1fe1533ba..ecaa18a0ef41 100644 --- a/helix-term/src/commands.rs +++ b/helix-term/src/commands.rs @@ -3980,8 +3980,10 @@ pub mod insert { } pub fn insert_newline(cx: &mut Context) { + let config = cx.editor.config(); let (view, doc) = current_ref!(cx.editor); let text = doc.text().slice(..); + let line_ending = doc.line_ending.as_str(); let contents = doc.text(); let selection = doc.selection(view.id).clone(); @@ -3989,6 +3991,14 @@ pub mod insert { // TODO: this is annoying, but we need to do it to properly calculate pos after edits let mut global_offs = 0; + let mut new_text = String::new(); + + let continue_comment_tokens = if config.continue_comments { + doc.language_config() + .and_then(|config| config.comment_tokens.as_ref()) + } else { + None + }; let mut transaction = Transaction::change_by_selection(contents, &selection, |range| { // Tracks the number of trailing whitespace characters deleted by this selection. @@ -4005,15 +4015,8 @@ pub mod insert { let current_line = text.char_to_line(pos); let line_start = text.line_to_char(current_line); - let mut new_text = String::new(); - - let continue_comment_token = if doc.config.load().continue_comments { - doc.language_config() - .and_then(|config| config.comment_tokens.as_ref()) - .and_then(|tokens| comment::get_comment_token(text, tokens, current_line)) - } else { - None - }; + let continue_comment_token = continue_comment_tokens + .and_then(|tokens| comment::get_comment_token(text, tokens, current_line)); let (from, to, local_offs) = if let Some(idx) = text.slice(line_start..pos).last_non_whitespace_char() @@ -4026,7 +4029,7 @@ pub mod insert { _ => indent::indent_for_newline( doc.language_config(), doc.syntax(), - &doc.config.load().indent_heuristic, + &config.indent_heuristic, &doc.indent_style, doc.tab_width(), text, @@ -4045,7 +4048,8 @@ pub mod insert { .map_or(false, |pair| pair.open == prev && pair.close == curr); let local_offs = if let Some(token) = continue_comment_token { - new_text.push_str(doc.line_ending.as_str()); + new_text.reserve_exact(line_ending.len() + indent.len() + token.len() + 1); + new_text.push_str(line_ending); new_text.push_str(&indent); new_text.push_str(token); new_text.push(' '); @@ -4053,19 +4057,20 @@ pub mod insert { } else if on_auto_pair { // line where the cursor will be let inner_indent = indent.clone() + doc.indent_style.as_str(); - new_text.reserve_exact(2 + indent.len() + inner_indent.len()); - new_text.push_str(doc.line_ending.as_str()); + new_text + .reserve_exact(line_ending.len() * 2 + indent.len() + inner_indent.len()); + new_text.push_str(line_ending); new_text.push_str(&inner_indent); // line where the matching pair will be let local_offs = new_text.chars().count(); - new_text.push_str(doc.line_ending.as_str()); + new_text.push_str(line_ending); new_text.push_str(&indent); local_offs } else { - new_text.reserve_exact(1 + indent.len()); - new_text.push_str(doc.line_ending.as_str()); + new_text.reserve_exact(line_ending.len() + indent.len()); + new_text.push_str(line_ending); new_text.push_str(&indent); new_text.chars().count() @@ -4084,7 +4089,7 @@ pub mod insert { // If the current line is all whitespace, insert a line ending at the beginning of // the current line. This makes the current line empty and the new line contain the // indentation of the old line. - new_text.push_str(doc.line_ending.as_str()); + new_text.push_str(line_ending); (line_start, line_start, new_text.chars().count() as isize) }; @@ -4108,8 +4113,10 @@ pub mod insert { // can be used with cx.mode to do replace or extend on most changes ranges.push(new_range); global_offs += new_text.chars().count() as isize - chars_deleted as isize; + let tendril = Tendril::from(&new_text); + new_text.clear(); - (from, to, Some(new_text.into())) + (from, to, Some(tendril)) }); transaction = transaction.with_selection(Selection::new(ranges, selection.primary_index()));