Skip to content

Commit

Permalink
minor: Use more exact allocations in insert_newline
Browse files Browse the repository at this point in the history
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<String>` 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.
  • Loading branch information
the-mikedavis committed Jan 15, 2025
1 parent 4bd17e5 commit 3318953
Showing 1 changed file with 25 additions and 18 deletions.
43 changes: 25 additions & 18 deletions helix-term/src/commands.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3980,15 +3980,25 @@ 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();
let mut ranges = SmallVec::with_capacity(selection.len());

// 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.
Expand All @@ -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()
Expand All @@ -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,
Expand All @@ -4045,27 +4048,29 @@ 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(' ');
new_text.chars().count()
} 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()
Expand All @@ -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)
};
Expand All @@ -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()));
Expand Down

0 comments on commit 3318953

Please sign in to comment.