nit: Do less allocations in `ui::menu::Item::label` implementations

This complicates the code a little but it often divides by two the number of allocations done by
the functions. LSP labels especially can easily be called dozens of time in a single menu popup,
when listing references for example.
pull/4406/head
Alexis (Poliorcetics) Bourget 2 years ago committed by Michael Davis
parent 3aea33a415
commit 34389e1d54

@ -54,7 +54,7 @@ use crate::{
use crate::job::{self, Jobs}; use crate::job::{self, Jobs};
use futures_util::StreamExt; use futures_util::StreamExt;
use std::{collections::HashMap, fmt, future::Future}; use std::{collections::HashMap, fmt, fmt::Write, future::Future};
use std::{collections::HashSet, num::NonZeroUsize}; use std::{collections::HashSet, num::NonZeroUsize};
use std::{ use std::{
@ -2416,19 +2416,18 @@ impl ui::menu::Item for MappableCommand {
type Data = ReverseKeymap; type Data = ReverseKeymap;
fn label(&self, keymap: &Self::Data) -> Spans { fn label(&self, keymap: &Self::Data) -> Spans {
// formats key bindings, multiple bindings are comma separated,
// individual key presses are joined with `+`
let fmt_binding = |bindings: &Vec<Vec<KeyEvent>>| -> String { let fmt_binding = |bindings: &Vec<Vec<KeyEvent>>| -> String {
bindings bindings.iter().fold(String::new(), |mut acc, bind| {
.iter() if !acc.is_empty() {
.map(|bind| { acc.push_str(", ");
bind.iter() }
.map(|key| key.to_string()) bind.iter().fold(false, |needs_plus, key| {
.collect::<Vec<String>>() write!(&mut acc, "{}{}", if needs_plus { "+" } else { "" }, key)
.join("+") .expect("Writing to a string can only fail on an Out-Of-Memory error");
true
});
acc
}) })
.collect::<Vec<String>>()
.join(", ")
}; };
match self { match self {

@ -18,7 +18,9 @@ use crate::{
}, },
}; };
use std::{borrow::Cow, cmp::Ordering, collections::BTreeMap, path::PathBuf, sync::Arc}; use std::{
borrow::Cow, cmp::Ordering, collections::BTreeMap, fmt::Write, path::PathBuf, sync::Arc,
};
/// Gets the language server that is attached to a document, and /// Gets the language server that is attached to a document, and
/// if it's not active displays a status message. Using this macro /// if it's not active displays a status message. Using this macro
@ -43,23 +45,32 @@ impl ui::menu::Item for lsp::Location {
type Data = PathBuf; type Data = PathBuf;
fn label(&self, cwdir: &Self::Data) -> Spans { fn label(&self, cwdir: &Self::Data) -> Spans {
let file: Cow<'_, str> = (self.uri.scheme() == "file") // The preallocation here will overallocate a few characters since it will account for the
.then(|| { // URL's scheme, which is not used most of the time since that scheme will be "file://".
self.uri // Those extra chars will be used to avoid allocating when writing the line number (in the
.to_file_path() // common case where it has 5 digits or less, which should be enough for a cast majority
.map(|path| { // of usages).
// strip root prefix let mut res = String::with_capacity(self.uri.as_str().len());
path.strip_prefix(&cwdir)
.map(|path| path.to_path_buf()) if self.uri.scheme() == "file" {
.unwrap_or(path) // With the preallocation above and UTF-8 paths already, this closure will do one (1)
}) // allocation, for `to_file_path`, else there will be two (2), with `to_string_lossy`.
.map(|path| Cow::from(path.to_string_lossy().into_owned())) let mut write_path_to_res = || -> Option<()> {
.ok() let path = self.uri.to_file_path().ok()?;
}) res.push_str(&path.strip_prefix(&cwdir).unwrap_or(&path).to_string_lossy());
.flatten() Some(())
.unwrap_or_else(|| self.uri.as_str().into()); };
let line = self.range.start.line; write_path_to_res();
format!("{}:{}", file, line).into() } else {
// Never allocates since we declared the string with this capacity already.
res.push_str(self.uri.as_str());
}
// Most commonly, this will not allocate, especially on Unix systems where the root prefix
// is a simple `/` and not `C:\` (with whatever drive letter)
write!(&mut res, ":{}", self.range.start.line)
.expect("Will only failed if allocating fail");
res.into()
} }
} }
@ -73,10 +84,8 @@ impl ui::menu::Item for lsp::SymbolInformation {
} else { } else {
match self.location.uri.to_file_path() { match self.location.uri.to_file_path() {
Ok(path) => { Ok(path) => {
let relative_path = helix_core::path::get_relative_path(path.as_path()) let get_relative_path = path::get_relative_path(path.as_path());
.to_string_lossy() format!("{} ({})", &self.name, get_relative_path.to_string_lossy()).into()
.into_owned();
format!("{} ({})", &self.name, relative_path).into()
} }
Err(_) => format!("{} ({})", &self.name, &self.location.uri).into(), Err(_) => format!("{} ({})", &self.name, &self.location.uri).into(),
} }
@ -115,24 +124,21 @@ impl ui::menu::Item for PickerDiagnostic {
// remove background as it is distracting in the picker list // remove background as it is distracting in the picker list
style.bg = None; style.bg = None;
let code = self let code: Cow<'_, str> = self
.diag .diag
.code .code
.as_ref() .as_ref()
.map(|c| match c { .map(|c| match c {
NumberOrString::Number(n) => n.to_string(), NumberOrString::Number(n) => n.to_string().into(),
NumberOrString::String(s) => s.to_string(), NumberOrString::String(s) => s.as_str().into(),
}) })
.map(|code| format!(" ({})", code))
.unwrap_or_default(); .unwrap_or_default();
let path = match format { let path = match format {
DiagnosticsFormat::HideSourcePath => String::new(), DiagnosticsFormat::HideSourcePath => String::new(),
DiagnosticsFormat::ShowSourcePath => { DiagnosticsFormat::ShowSourcePath => {
let path = path::get_truncated_path(self.url.path()) let path = path::get_truncated_path(self.url.path());
.to_string_lossy() format!("{}: ", path.to_string_lossy())
.into_owned();
format!("{}: ", path)
} }
}; };

Loading…
Cancel
Save