git.iko.soy/helix
on master help sign in
nit: Do less allocations in `ui::menu::Item::label` implementations
34389e1Alexis (Poliorcetics) Bourget
helix-term/src/commands.rs+12−13
compare with master
unifiedsplit
@@ -54,7 +54,7 @@ use crate::{
54
54
55use crate::job::{self, Jobs};
55use crate::job::{self, Jobs};
56use futures_util::StreamExt;
56use futures_util::StreamExt;
57−use std::{collections::HashMap, fmt, future::Future};
57+use std::{collections::HashMap, fmt, fmt::Write, future::Future};
58use std::{collections::HashSet, num::NonZeroUsize};
58use std::{collections::HashSet, num::NonZeroUsize};
59
59
60use std::{
60use std::{
@@ -2416,19 +2416,18 @@ impl ui::menu::Item for MappableCommand {
2416 type Data = ReverseKeymap;
2416 type Data = ReverseKeymap;
2417
2417
2418 fn label(&self, keymap: &Self::Data) -> Spans {
2418 fn label(&self, keymap: &Self::Data) -> Spans {
2419− // formats key bindings, multiple bindings are comma separated,
2420− // individual key presses are joined with `+`
2421 let fmt_binding = |bindings: &Vec<Vec<KeyEvent>>| -> String {
2419 let fmt_binding = |bindings: &Vec<Vec<KeyEvent>>| -> String {
2422− bindings
2420+ bindings.iter().fold(String::new(), |mut acc, bind| {
2423− .iter()
2421+ if !acc.is_empty() {
2424− .map(|bind| {
2422+ acc.push_str(", ");
2425− bind.iter()
2423+ }
2426− .map(|key| key.to_string())
2424+ bind.iter().fold(false, |needs_plus, key| {
2427− .collect::<Vec<String>>()
2425+ write!(&mut acc, "{}{}", if needs_plus { "+" } else { "" }, key)
2428− .join("+")
2426+ .expect("Writing to a string can only fail on an Out-Of-Memory error");
2427+ true
2429− })
2428+ });
2430− .collect::<Vec<String>>()
2429+ acc
2431− .join(", ")
2430+ })
2432 };
2431 };
2433
2432
2434 match self {
2433 match self {
helix-term/src/commands/lsp.rs+36−30
@@ -18,7 +18,9 @@ use crate::{
18 },
18 },
19};
19};
20
20
21+use std::{
21−use std::{borrow::Cow, cmp::Ordering, collections::BTreeMap, path::PathBuf, sync::Arc};
22+ borrow::Cow, cmp::Ordering, collections::BTreeMap, fmt::Write, path::PathBuf, sync::Arc,
23+};
22
24
23/// Gets the language server that is attached to a document, and
25/// Gets the language server that is attached to a document, and
24/// if it's not active displays a status message. Using this macro
26/// if it's not active displays a status message. Using this macro
@@ -43,23 +45,32 @@ impl ui::menu::Item for lsp::Location {
43 type Data = PathBuf;
45 type Data = PathBuf;
44
46
45 fn label(&self, cwdir: &Self::Data) -> Spans {
47 fn label(&self, cwdir: &Self::Data) -> Spans {
48+ // The preallocation here will overallocate a few characters since it will account for the
49+ // URL's scheme, which is not used most of the time since that scheme will be "file://".
50+ // Those extra chars will be used to avoid allocating when writing the line number (in the
51+ // common case where it has 5 digits or less, which should be enough for a cast majority
52+ // of usages).
53+ let mut res = String::with_capacity(self.uri.as_str().len());
54+
46− let file: Cow<'_, str> = (self.uri.scheme() == "file")
55+ if self.uri.scheme() == "file" {
47− .then(|| {
56+ // With the preallocation above and UTF-8 paths already, this closure will do one (1)
48− self.uri
57+ // allocation, for `to_file_path`, else there will be two (2), with `to_string_lossy`.
58+ let mut write_path_to_res = || -> Option<()> {
49− .to_file_path()
59+ let path = self.uri.to_file_path().ok()?;
50− .map(|path| {
51− // strip root prefix
52− path.strip_prefix(&cwdir)
60+ res.push_str(&path.strip_prefix(&cwdir).unwrap_or(&path).to_string_lossy());
53− .map(|path| path.to_path_buf())
61+ Some(())
54− .unwrap_or(path)
55− })
56− .map(|path| Cow::from(path.to_string_lossy().into_owned()))
57− .ok()
58− })
62+ };
59− .flatten()
63+ write_path_to_res();
64+ } else {
65+ // Never allocates since we declared the string with this capacity already.
60− .unwrap_or_else(|| self.uri.as_str().into());
66+ res.push_str(self.uri.as_str());
67+ }
68+
69+ // Most commonly, this will not allocate, especially on Unix systems where the root prefix
70+ // is a simple `/` and not `C:\` (with whatever drive letter)
61− let line = self.range.start.line;
71+ write!(&mut res, ":{}", self.range.start.line)
62− format!("{}:{}", file, line).into()
72+ .expect("Will only failed if allocating fail");
73+ res.into()
63 }
74 }
64}
75}
65
76
@@ -73,10 +84,8 @@ impl ui::menu::Item for lsp::SymbolInformation {
73 } else {
84 } else {
74 match self.location.uri.to_file_path() {
85 match self.location.uri.to_file_path() {
75 Ok(path) => {
86 Ok(path) => {
76− let relative_path = helix_core::path::get_relative_path(path.as_path())
87+ let get_relative_path = path::get_relative_path(path.as_path());
77− .to_string_lossy()
78− .into_owned();
79− format!("{} ({})", &self.name, relative_path).into()
88+ format!("{} ({})", &self.name, get_relative_path.to_string_lossy()).into()
80 }
89 }
81 Err(_) => format!("{} ({})", &self.name, &self.location.uri).into(),
90 Err(_) => format!("{} ({})", &self.name, &self.location.uri).into(),
82 }
91 }
@@ -115,24 +124,21 @@ impl ui::menu::Item for PickerDiagnostic {
115 // remove background as it is distracting in the picker list
124 // remove background as it is distracting in the picker list
116 style.bg = None;
125 style.bg = None;
117
126
118− let code = self
127+ let code: Cow<'_, str> = self
119 .diag
128 .diag
120 .code
129 .code
121 .as_ref()
130 .as_ref()
122 .map(|c| match c {
131 .map(|c| match c {
123− NumberOrString::Number(n) => n.to_string(),
132+ NumberOrString::Number(n) => n.to_string().into(),
124− NumberOrString::String(s) => s.to_string(),
133+ NumberOrString::String(s) => s.as_str().into(),
125 })
134 })
126− .map(|code| format!(" ({})", code))
127 .unwrap_or_default();
135 .unwrap_or_default();
128
136
129 let path = match format {
137 let path = match format {
130 DiagnosticsFormat::HideSourcePath => String::new(),
138 DiagnosticsFormat::HideSourcePath => String::new(),
131 DiagnosticsFormat::ShowSourcePath => {
139 DiagnosticsFormat::ShowSourcePath => {
132− let path = path::get_truncated_path(self.url.path())
140+ let path = path::get_truncated_path(self.url.path());
133− .to_string_lossy()
134− .into_owned();
135− format!("{}: ", path)
141+ format!("{}: ", path.to_string_lossy())
136 }
142 }
137 };
143 };
138
144