refactor(rust): cleanup path handling

- Avoids unnecessary string allocation when tracing paths
- Replaces `mut path & path.push()` with `path.join()`
- Avoids unncessary cloning of paths where applicable
- Use `dunce` crate to remove `UNC` prefix
- Improve performance of resolving `~` by avoiding unnecessary string allocations
- Resolve `~`, `$Env:USERPROFILE` and `$HOME` consistenly between different code paths
- Use `PathBuf` instead of `String` for paths in CLI args

I may have missed a couple of places but I think I covered 90% of path handling in the codebase
This commit is contained in:
amrbashir
2023-11-25 09:14:54 +02:00
committed by LGUG2Z
parent a68f3843b7
commit b39e65642b
18 changed files with 305 additions and 530 deletions
+2 -2
View File
@@ -15,11 +15,9 @@ komorebi-core = { path = "../komorebi-core" }
bitflags = "2"
clap = { version = "4", features = ["derive"] }
color-eyre = "0.6"
crossbeam-channel = "0.5"
crossbeam-utils = "0.8"
ctrlc = "3"
dirs = "5"
getset = "0.1"
hotwatch = "0.4"
lazy_static = "1"
@@ -45,6 +43,8 @@ winreg = "0.52"
windows-interface = { workspace = true }
windows-implement = { workspace = true }
windows = { workspace = true }
color-eyre = { workspace = true }
dirs = { workspace = true }
[features]
deadlock_detection = []
+8 -28
View File
@@ -22,7 +22,6 @@ use std::sync::Arc;
use std::time::Duration;
use clap::Parser;
use color_eyre::eyre::anyhow;
use color_eyre::Result;
use crossbeam_channel::Receiver;
use crossbeam_channel::Sender;
@@ -252,7 +251,7 @@ fn setup() -> Result<(WorkerGuard, WorkerGuard)> {
std::env::set_var("RUST_LOG", "info");
}
let appender = tracing_appender::rolling::never(DATA_DIR.clone(), "komorebi.log");
let appender = tracing_appender::rolling::never(&*DATA_DIR, "komorebi.log");
let color_appender = tracing_appender::rolling::never(std::env::temp_dir(), "komorebi.log");
let (non_blocking, guard) = tracing_appender::non_blocking(appender);
let (color_non_blocking, color_guard) = tracing_appender::non_blocking(color_appender);
@@ -305,13 +304,8 @@ fn setup() -> Result<(WorkerGuard, WorkerGuard)> {
}
pub fn load_configuration() -> Result<()> {
let home = HOME_DIR.clone();
let mut config_pwsh = home.clone();
config_pwsh.push("komorebi.ps1");
let mut config_ahk = home;
config_ahk.push("komorebi.ahk");
let config_pwsh = HOME_DIR.join("komorebi.ps1");
let config_ahk = HOME_DIR.join("komorebi.ahk");
if config_pwsh.exists() {
let powershell_exe = if which("pwsh.exe").is_ok() {
@@ -320,25 +314,13 @@ pub fn load_configuration() -> Result<()> {
"powershell.exe"
};
tracing::info!(
"loading configuration file: {}",
config_pwsh
.as_os_str()
.to_str()
.ok_or_else(|| anyhow!("cannot convert path to string"))?
);
tracing::info!("loading configuration file: {}", config_pwsh.display());
Command::new(powershell_exe)
.arg(config_pwsh.as_os_str())
.output()?;
} else if config_ahk.exists() && which(&*AHK_EXE).is_ok() {
tracing::info!(
"loading configuration file: {}",
config_ahk
.as_os_str()
.to_str()
.ok_or_else(|| anyhow!("cannot convert path to string"))?
);
tracing::info!("loading configuration file: {}", config_ahk.display());
Command::new(&*AHK_EXE)
.arg(config_ahk.as_os_str())
@@ -410,7 +392,7 @@ pub fn notify_subscribers(notification: &str) -> Result<()> {
let mut subscriptions = SUBSCRIPTION_PIPES.lock();
for (subscriber, pipe) in &mut *subscriptions {
match writeln!(pipe, "{notification}") {
Ok(_) => {
Ok(()) => {
tracing::debug!("pushed notification to subscriber: {}", subscriber);
}
Err(error) => {
@@ -528,7 +510,7 @@ fn main() -> Result<()> {
let wm = if let Some(config) = &opts.config {
tracing::info!(
"creating window manager from static configuration file: {}",
config.as_os_str().to_str().unwrap()
config.display()
);
Arc::new(Mutex::new(StaticConfig::preload(
@@ -557,9 +539,7 @@ fn main() -> Result<()> {
}
if opts.config.is_none() {
std::thread::spawn(|| {
load_configuration().expect("could not load configuration");
});
std::thread::spawn(|| load_configuration().expect("could not load configuration"));
if opts.await_configuration {
let backoff = Backoff::new();
+2 -3
View File
@@ -2,6 +2,7 @@ use std::collections::HashMap;
use std::collections::VecDeque;
use color_eyre::eyre::anyhow;
use color_eyre::eyre::bail;
use color_eyre::Result;
use getset::CopyGetters;
use getset::Getters;
@@ -120,9 +121,7 @@ impl Monitor {
.ok_or_else(|| anyhow!("there is no workspace"))?;
if workspace.maximized_window().is_some() {
return Err(anyhow!(
"cannot move native maximized window to another monitor or workspace"
));
bail!("cannot move native maximized window to another monitor or workspace");
}
let container = workspace
+18 -38
View File
@@ -472,10 +472,10 @@ impl WindowManager {
SocketMessage::ChangeLayout(layout) => self.change_workspace_layout_default(layout)?,
SocketMessage::CycleLayout(direction) => self.cycle_layout(direction)?,
SocketMessage::ChangeLayoutCustom(ref path) => {
self.change_workspace_custom_layout(path.clone())?;
self.change_workspace_custom_layout(path)?;
}
SocketMessage::WorkspaceLayoutCustom(monitor_idx, workspace_idx, ref path) => {
self.set_workspace_layout_custom(monitor_idx, workspace_idx, path.clone())?;
self.set_workspace_layout_custom(monitor_idx, workspace_idx, path)?;
}
SocketMessage::WorkspaceTiling(monitor_idx, workspace_idx, tile) => {
self.set_workspace_tiling(monitor_idx, workspace_idx, tile)?;
@@ -506,7 +506,7 @@ impl WindowManager {
monitor_idx,
workspace_idx,
at_container_count,
path.clone(),
path,
)?;
}
SocketMessage::ClearWorkspaceLayoutRules(monitor_idx, workspace_idx) => {
@@ -516,7 +516,7 @@ impl WindowManager {
if let Some((monitor_idx, workspace_idx)) =
self.monitor_workspace_index_by_name(workspace)
{
self.set_workspace_layout_custom(monitor_idx, workspace_idx, path.clone())?;
self.set_workspace_layout_custom(monitor_idx, workspace_idx, path)?;
}
}
SocketMessage::NamedWorkspaceTiling(ref workspace, tile) => {
@@ -557,7 +557,7 @@ impl WindowManager {
monitor_idx,
workspace_idx,
at_container_count,
path.clone(),
path,
)?;
}
}
@@ -691,10 +691,7 @@ impl WindowManager {
Err(error) => error.to_string(),
};
let mut socket = DATA_DIR.clone();
socket.push("komorebic.sock");
let socket = socket.as_path();
let socket = DATA_DIR.join("komorebic.sock");
let mut stream = UnixStream::connect(socket)?;
stream.write_all(state.as_bytes())?;
}
@@ -714,10 +711,7 @@ impl WindowManager {
}
.to_string();
let mut socket = DATA_DIR.clone();
socket.push("komorebic.sock");
let socket = socket.as_path();
let socket = DATA_DIR.join("komorebic.sock");
let mut stream = UnixStream::connect(socket)?;
stream.write_all(response.as_bytes())?;
}
@@ -1034,8 +1028,7 @@ impl WindowManager {
let workspace = self.focused_workspace()?;
let resize = workspace.resize_dimensions();
let mut quicksave_json = std::env::temp_dir();
quicksave_json.push("komorebi.quicksave.json");
let quicksave_json = std::env::temp_dir().join("komorebi.quicksave.json");
let file = OpenOptions::new()
.write(true)
@@ -1048,15 +1041,10 @@ impl WindowManager {
SocketMessage::QuickLoad => {
let workspace = self.focused_workspace_mut()?;
let mut quicksave_json = std::env::temp_dir();
quicksave_json.push("komorebi.quicksave.json");
let quicksave_json = std::env::temp_dir().join("komorebi.quicksave.json");
let file = File::open(&quicksave_json).map_err(|_| {
anyhow!(
"no quicksave found at {}",
quicksave_json.display().to_string()
)
})?;
let file = File::open(&quicksave_json)
.map_err(|_| anyhow!("no quicksave found at {}", quicksave_json.display()))?;
let resize: Vec<Option<Rect>> = serde_json::from_reader(file)?;
@@ -1071,15 +1059,15 @@ impl WindowManager {
.write(true)
.truncate(true)
.create(true)
.open(path.clone())?;
.open(path)?;
serde_json::to_writer_pretty(&file, &resize)?;
}
SocketMessage::Load(ref path) => {
let workspace = self.focused_workspace_mut()?;
let file = File::open(path)
.map_err(|_| anyhow!("no file found at {}", path.display().to_string()))?;
let file =
File::open(path).map_err(|_| anyhow!("no file found at {}", path.display()))?;
let resize: Vec<Option<Rect>> = serde_json::from_reader(file)?;
@@ -1191,9 +1179,7 @@ impl WindowManager {
SocketMessage::NotificationSchema => {
let notification = schema_for!(Notification);
let schema = serde_json::to_string_pretty(&notification)?;
let mut socket = DATA_DIR.clone();
socket.push("komorebic.sock");
let socket = socket.as_path();
let socket = DATA_DIR.join("komorebic.sock");
let mut stream = UnixStream::connect(socket)?;
stream.write_all(schema.as_bytes())?;
@@ -1201,9 +1187,7 @@ impl WindowManager {
SocketMessage::SocketSchema => {
let socket_message = schema_for!(SocketMessage);
let schema = serde_json::to_string_pretty(&socket_message)?;
let mut socket = DATA_DIR.clone();
socket.push("komorebic.sock");
let socket = socket.as_path();
let socket = DATA_DIR.join("komorebic.sock");
let mut stream = UnixStream::connect(socket)?;
stream.write_all(schema.as_bytes())?;
@@ -1211,18 +1195,14 @@ impl WindowManager {
SocketMessage::StaticConfigSchema => {
let socket_message = schema_for!(StaticConfig);
let schema = serde_json::to_string_pretty(&socket_message)?;
let mut socket = DATA_DIR.clone();
socket.push("komorebic.sock");
let socket = socket.as_path();
let socket = DATA_DIR.join("komorebic.sock");
let mut stream = UnixStream::connect(socket)?;
stream.write_all(schema.as_bytes())?;
}
SocketMessage::GenerateStaticConfig => {
let config = serde_json::to_string_pretty(&StaticConfig::from(&*self))?;
let mut socket = DATA_DIR.clone();
socket.push("komorebic.sock");
let socket = socket.as_path();
let socket = DATA_DIR.join("komorebic.sock");
let mut stream = UnixStream::connect(socket)?;
stream.write_all(config.as_bytes())?;
+1 -1
View File
@@ -31,7 +31,7 @@ pub fn listen_for_movements(wm: Arc<Mutex<WindowManager>>) {
Event::MouseMoveRelative { .. } => {
if !ignore_movement {
match wm.lock().raise_window_at_cursor_pos() {
Ok(_) => {}
Ok(()) => {}
Err(error) => tracing::error!("{}", error),
}
}
+4 -9
View File
@@ -29,13 +29,13 @@ use crate::TRAY_AND_MULTI_WINDOW_IDENTIFIERS;
use crate::WORKSPACE_RULES;
use color_eyre::Result;
use crossbeam_channel::Receiver;
use dirs::home_dir;
use hotwatch::notify::DebouncedEvent;
use hotwatch::Hotwatch;
use komorebi_core::config_generation::ApplicationConfigurationGenerator;
use komorebi_core::config_generation::ApplicationOptions;
use komorebi_core::config_generation::IdWithIdentifier;
use komorebi_core::config_generation::MatchingStrategy;
use komorebi_core::resolve_home_path;
use komorebi_core::ApplicationIdentifier;
use komorebi_core::DefaultLayout;
use komorebi_core::FocusFollowsMouseImplementation;
@@ -616,13 +616,8 @@ impl StaticConfig {
}
if let Some(path) = &self.app_specific_configuration_path {
let stringified = path.to_string_lossy();
let stringified = stringified.replace(
"$Env:USERPROFILE",
&home_dir().expect("no home dir").to_string_lossy(),
);
let content = std::fs::read_to_string(stringified)?;
let path = resolve_home_path(path)?;
let content = std::fs::read_to_string(path)?;
let asc = ApplicationConfigurationGenerator::load(&content)?;
for mut entry in asc {
@@ -762,7 +757,7 @@ impl StaticConfig {
let socket = DATA_DIR.join("komorebi.sock");
match std::fs::remove_file(&socket) {
Ok(_) => {}
Ok(()) => {}
Err(error) => match error.kind() {
// Doing this because ::exists() doesn't work reliably on Windows via IntelliJ
ErrorKind::NotFound => {}
+4 -4
View File
@@ -241,7 +241,7 @@ impl Window {
// Raise Window to foreground
match WindowsApi::set_foreground_window(self.hwnd()) {
Ok(_) => {}
Ok(()) => {}
Err(error) => {
tracing::error!(
"could not set as foreground window, but continuing execution of raise(): {}",
@@ -252,7 +252,7 @@ impl Window {
// This isn't really needed when the above command works as expected via AHK
match WindowsApi::set_focus(self.hwnd()) {
Ok(_) => {}
Ok(()) => {}
Err(error) => {
tracing::error!(
"could not set focus, but continuing execution of raise(): {}",
@@ -302,7 +302,7 @@ impl Window {
}
match WindowsApi::set_foreground_window(self.hwnd()) {
Ok(_) => {
Ok(()) => {
foregrounded = true;
}
Err(error) => {
@@ -334,7 +334,7 @@ impl Window {
// This isn't really needed when the above command works as expected via AHK
match WindowsApi::set_focus(self.hwnd()) {
Ok(_) => {}
Ok(()) => {}
Err(error) => {
tracing::error!(
"could not set focus, but continuing execution of focus(): {}",
+62 -75
View File
@@ -3,11 +3,13 @@ use std::collections::HashSet;
use std::collections::VecDeque;
use std::io::ErrorKind;
use std::num::NonZeroUsize;
use std::path::Path;
use std::path::PathBuf;
use std::sync::atomic::Ordering;
use std::sync::Arc;
use color_eyre::eyre::anyhow;
use color_eyre::eyre::bail;
use color_eyre::Result;
use crossbeam_channel::Receiver;
use hotwatch::notify::DebouncedEvent;
@@ -166,7 +168,7 @@ impl WindowManager {
let socket = DATA_DIR.join("komorebi.sock");
match std::fs::remove_file(&socket) {
Ok(_) => {}
Ok(()) => {}
Err(error) => match error.kind() {
// Doing this because ::exists() doesn't work reliably on Windows via IntelliJ
ErrorKind::NotFound => {}
@@ -247,13 +249,8 @@ impl WindowManager {
#[tracing::instrument(skip(self))]
pub fn watch_configuration(&mut self, enable: bool) -> Result<()> {
let home = HOME_DIR.clone();
let mut config_pwsh = home.clone();
config_pwsh.push("komorebi.ps1");
let mut config_ahk = home;
config_ahk.push("komorebi.ahk");
let config_pwsh = HOME_DIR.join("komorebi.ps1");
let config_ahk = HOME_DIR.join("komorebi.ahk");
if config_pwsh.exists() {
self.configure_watcher(enable, config_pwsh)?;
@@ -265,50 +262,39 @@ impl WindowManager {
}
fn configure_watcher(&mut self, enable: bool, config: PathBuf) -> Result<()> {
if config.exists() {
if enable {
tracing::info!(
"watching configuration for changes: {}",
config
.as_os_str()
.to_str()
.ok_or_else(|| anyhow!("cannot convert path to string"))?
);
// Always make absolutely sure that there isn't an already existing watch, because
// hotwatch allows multiple watches to be registered for the same path
match self.hotwatch.unwatch(config.clone()) {
Ok(_) => {}
Err(error) => match error {
hotwatch::Error::Notify(error) => match error {
hotwatch::notify::Error::WatchNotFound => {}
error => return Err(error.into()),
},
error @ hotwatch::Error::Io(_) => return Err(error.into()),
if enable {
tracing::info!("watching configuration for changes: {}", config.display());
// Always make absolutely sure that there isn't an already existing watch, because
// hotwatch allows multiple watches to be registered for the same path
match self.hotwatch.unwatch(&config) {
Ok(()) => {}
Err(error) => match error {
hotwatch::Error::Notify(error) => match error {
hotwatch::notify::Error::WatchNotFound => {}
error => return Err(error.into()),
},
error @ hotwatch::Error::Io(_) => return Err(error.into()),
},
}
self.hotwatch.watch(config, |event| match event {
// Editing in Notepad sends a NoticeWrite while editing in (Neo)Vim sends
// a NoticeRemove, presumably because of the use of swap files?
DebouncedEvent::NoticeWrite(_) | DebouncedEvent::NoticeRemove(_) => {
std::thread::spawn(|| {
load_configuration().expect("could not load configuration");
});
}
_ => {}
})?;
} else {
tracing::info!(
"no longer watching configuration for changes: {}",
config.display()
);
self.hotwatch.watch(config, |event| match event {
// Editing in Notepad sends a NoticeWrite while editing in (Neo)Vim sends
// a NoticeRemove, presumably because of the use of swap files?
DebouncedEvent::NoticeWrite(_) | DebouncedEvent::NoticeRemove(_) => {
std::thread::spawn(|| {
load_configuration().expect("could not load configuration");
});
}
_ => {}
})?;
} else {
tracing::info!(
"no longer watching configuration for changes: {}",
config
.as_os_str()
.to_str()
.ok_or_else(|| anyhow!("cannot convert path to string"))?
);
self.hotwatch.unwatch(config)?;
};
}
self.hotwatch.unwatch(config)?;
};
Ok(())
}
@@ -868,7 +854,7 @@ impl WindowManager {
// attach to the thread of the desktop window always seems to result in "Access is
// denied (os error 5)"
match WindowsApi::set_foreground_window(desktop_window.hwnd()) {
Ok(_) => {}
Ok(()) => {}
Err(error) => {
tracing::warn!("{} {}:{}", error, file!(), line!());
}
@@ -1002,9 +988,7 @@ impl WindowManager {
let workspace = self.focused_workspace()?;
let focused_hwnd = WindowsApi::foreground_window()?;
if !workspace.contains_managed_window(focused_hwnd) {
return Err(anyhow!(
"ignoring commands while active window is not managed by komorebi"
));
bail!("ignoring commands while active window is not managed by komorebi");
}
}
@@ -1120,9 +1104,7 @@ impl WindowManager {
.ok_or_else(|| anyhow!("there is no workspace"))?;
if workspace.maximized_window().is_some() {
return Err(anyhow!(
"cannot move native maximized window to another monitor or workspace"
));
bail!("cannot move native maximized window to another monitor or workspace");
}
let container = workspace
@@ -1232,9 +1214,7 @@ impl WindowManager {
// removing this messes up the monitor / container / window index somewhere
// and results in the wrong window getting moved across the monitor boundary
if workspace.is_focused_window_monocle_or_maximized()? {
return Err(anyhow!(
"ignoring command while active window is in monocle mode or maximized"
));
bail!("ignoring command while active window is in monocle mode or maximized");
}
tracing::info!("moving container");
@@ -1396,9 +1376,7 @@ impl WindowManager {
let workspace = self.focused_workspace_mut()?;
if workspace.is_focused_window_monocle_or_maximized()? {
return Err(anyhow!(
"ignoring command while active window is in monocle mode or maximized"
));
bail!("ignoring command while active window is in monocle mode or maximized");
}
tracing::info!("moving container");
@@ -1425,7 +1403,7 @@ impl WindowManager {
.ok_or_else(|| anyhow!("there must be at least one window in a container"))?;
if len.get() == 1 {
return Err(anyhow!("there is only one window in this container"));
bail!("there is only one window in this container");
}
let current_idx = container.focused_window_idx();
@@ -1510,7 +1488,7 @@ impl WindowManager {
tracing::info!("removing window");
if self.focused_container()?.windows().len() == 1 {
return Err(anyhow!("a container must have at least one window"));
bail!("a container must have at least one window");
}
let workspace = self.focused_workspace_mut()?;
@@ -1729,10 +1707,13 @@ impl WindowManager {
}
#[tracing::instrument(skip(self))]
pub fn change_workspace_custom_layout(&mut self, path: PathBuf) -> Result<()> {
pub fn change_workspace_custom_layout<P>(&mut self, path: P) -> Result<()>
where
P: AsRef<Path> + std::fmt::Debug,
{
tracing::info!("changing layout");
let layout = CustomLayout::from_path_buf(path)?;
let layout = CustomLayout::from_path(path)?;
let workspace = self.focused_workspace_mut()?;
match workspace.layout() {
@@ -1854,13 +1835,16 @@ impl WindowManager {
}
#[tracing::instrument(skip(self))]
pub fn add_workspace_layout_custom_rule(
pub fn add_workspace_layout_custom_rule<P>(
&mut self,
monitor_idx: usize,
workspace_idx: usize,
at_container_count: usize,
path: PathBuf,
) -> Result<()> {
path: P,
) -> Result<()>
where
P: AsRef<Path> + std::fmt::Debug,
{
tracing::info!("setting workspace layout");
let invisible_borders = self.invisible_borders;
@@ -1885,7 +1869,7 @@ impl WindowManager {
.get_mut(workspace_idx)
.ok_or_else(|| anyhow!("there is no monitor"))?;
let layout = CustomLayout::from_path_buf(path)?;
let layout = CustomLayout::from_path(path)?;
let rules: &mut Vec<(usize, Layout)> = workspace.layout_rules_mut();
rules.retain(|pair| pair.0 != at_container_count);
@@ -1986,14 +1970,17 @@ impl WindowManager {
}
#[tracing::instrument(skip(self))]
pub fn set_workspace_layout_custom(
pub fn set_workspace_layout_custom<P>(
&mut self,
monitor_idx: usize,
workspace_idx: usize,
path: PathBuf,
) -> Result<()> {
path: P,
) -> Result<()>
where
P: AsRef<Path> + std::fmt::Debug,
{
tracing::info!("setting workspace layout");
let layout = CustomLayout::from_path_buf(path)?;
let layout = CustomLayout::from_path(path)?;
let invisible_borders = self.invisible_borders;
let offset = self.work_area_offset;
let focused_monitor_idx = self.focused_monitor_idx();
@@ -2164,7 +2151,7 @@ impl WindowManager {
if self.monitors().get(idx).is_some() {
self.monitors.focus(idx);
} else {
return Err(anyhow!("this is not a valid monitor index"));
bail!("this is not a valid monitor index");
}
Ok(())
+1 -1
View File
@@ -368,7 +368,7 @@ impl WindowsApi {
pub fn close_window(hwnd: HWND) -> Result<()> {
match Self::post_message(hwnd, WM_CLOSE, WPARAM(0), LPARAM(0)) {
Ok(_) => Ok(()),
Ok(()) => Ok(()),
Err(_) => Err(anyhow!("could not close window")),
}
}
+1 -1
View File
@@ -61,7 +61,7 @@ impl WinEventListener {
MessageLoop::start(10, |_msg| {
if let Ok(event) = WINEVENT_CALLBACK_CHANNEL.lock().1.try_recv() {
match outgoing.send(event) {
Ok(_) => {}
Ok(()) => {}
Err(error) => {
tracing::error!("{}", error);
}
+2 -2
View File
@@ -108,7 +108,7 @@ impl Workspace {
}
if let Some(pathbuf) = &config.custom_layout {
let layout = CustomLayout::from_path_buf(pathbuf.clone())?;
let layout = CustomLayout::from_path(pathbuf)?;
self.layout = Layout::Custom(layout);
self.tile = true;
}
@@ -129,7 +129,7 @@ impl Workspace {
if let Some(layout_rules) = &config.custom_layout_rules {
let rules = self.layout_rules_mut();
for (count, pathbuf) in layout_rules {
let rule = CustomLayout::from_path_buf(pathbuf.clone())?;
let rule = CustomLayout::from_path(pathbuf)?;
rules.push((*count, Layout::Custom(rule)));
}
}