refactor: Eliminate unnecessary empty Vec constructions.

This commit is contained in:
2026-03-14 11:43:48 -04:00
parent 0ebb5e79dd
commit e89c6cb16f
7 changed files with 40 additions and 49 deletions

View File

@@ -1,4 +1,4 @@
# pikl-menu # pikl
Pipe stuff in, pick stuff out. Pipe stuff in, pick stuff out.

View File

@@ -29,7 +29,7 @@ pub enum DebounceMode {
/// behavior. Each event kind can have its own mode. /// behavior. Each event kind can have its own mode.
pub struct DebouncedDispatcher { pub struct DebouncedDispatcher {
handler: Arc<dyn HookHandler>, handler: Arc<dyn HookHandler>,
action_tx: mpsc::Sender<Action>, _action_tx: mpsc::Sender<Action>,
modes: HashMap<HookEventKind, DebounceMode>, modes: HashMap<HookEventKind, DebounceMode>,
in_flight: HashMap<HookEventKind, JoinHandle<()>>, in_flight: HashMap<HookEventKind, JoinHandle<()>>,
} }
@@ -41,7 +41,7 @@ impl DebouncedDispatcher {
) -> Self { ) -> Self {
Self { Self {
handler, handler,
action_tx, _action_tx: action_tx,
modes: HashMap::new(), modes: HashMap::new(),
in_flight: HashMap::new(), in_flight: HashMap::new(),
} }
@@ -95,18 +95,10 @@ impl DebouncedDispatcher {
fn fire_now(&self, event: HookEvent) { fn fire_now(&self, event: HookEvent) {
let handler = Arc::clone(&self.handler); let handler = Arc::clone(&self.handler);
let action_tx = self.action_tx.clone();
tokio::spawn(async move { tokio::spawn(async move {
match handler.handle(event) { if let Err(e) = handler.handle(event) {
Ok(responses) => {
for resp in responses {
let _ = action_tx.send(Action::ProcessHookResponse(resp)).await;
}
}
Err(e) => {
tracing::warn!(error = %e, "hook handler error"); tracing::warn!(error = %e, "hook handler error");
} }
}
}); });
} }
@@ -117,19 +109,11 @@ impl DebouncedDispatcher {
} }
let handler = Arc::clone(&self.handler); let handler = Arc::clone(&self.handler);
let action_tx = self.action_tx.clone();
let handle = tokio::spawn(async move { let handle = tokio::spawn(async move {
tokio::time::sleep(delay).await; tokio::time::sleep(delay).await;
match handler.handle(event) { if let Err(e) = handler.handle(event) {
Ok(responses) => {
for resp in responses {
let _ = action_tx.send(Action::ProcessHookResponse(resp)).await;
}
}
Err(e) => {
tracing::warn!(error = %e, "hook handler error"); tracing::warn!(error = %e, "hook handler error");
} }
}
}); });
self.in_flight.insert(kind, handle); self.in_flight.insert(kind, handle);
@@ -165,11 +149,11 @@ mod tests {
} }
impl HookHandler for RecordingHandler { impl HookHandler for RecordingHandler {
fn handle(&self, event: HookEvent) -> Result<Vec<HookResponse>, PiklError> { fn handle(&self, event: HookEvent) -> Result<(), PiklError> {
if let Ok(mut events) = self.events.lock() { if let Ok(mut events) = self.events.lock() {
events.push(event.kind()); events.push(event.kind());
} }
Ok(vec![]) Ok(())
} }
} }

View File

@@ -65,16 +65,15 @@ pub enum HookResponse {
} }
/// Handler trait for lifecycle hooks. Implementations /// Handler trait for lifecycle hooks. Implementations
/// receive events and optionally return responses. /// receive events and may produce side effects (spawning
/// Exec hooks return empty vecs. Handler hooks send /// processes, sending to channels). Responses flow through
/// responses back through the action channel asynchronously /// the action channel, not the return value.
/// and also return empty vecs.
/// ///
/// This is deliberately synchronous for dyn-compatibility. /// This is deliberately synchronous for dyn-compatibility.
/// Implementations that need async work (spawning processes, /// Implementations that need async work (spawning processes,
/// writing to channels) should use `tokio::spawn` internally. /// writing to channels) should use `tokio::spawn` internally.
pub trait HookHandler: Send + Sync { pub trait HookHandler: Send + Sync {
fn handle(&self, event: HookEvent) -> Result<Vec<HookResponse>, PiklError>; fn handle(&self, event: HookEvent) -> Result<(), PiklError>;
} }
/// Parse a single line of JSON as a [`HookResponse`]. /// Parse a single line of JSON as a [`HookResponse`].

View File

@@ -11,7 +11,7 @@ use crate::debounce::{hook_response_to_action, DebouncedDispatcher};
use crate::error::PiklError; use crate::error::PiklError;
use crate::event::{Action, MenuEvent, MenuResult, Mode, ViewState, VisibleItem}; use crate::event::{Action, MenuEvent, MenuResult, Mode, ViewState, VisibleItem};
use crate::hook::{HookEvent, HookHandler}; use crate::hook::{HookEvent, HookHandler};
use crate::model::traits::Menu; use crate::model::traits::MutableMenu;
use crate::navigation::Viewport; use crate::navigation::Viewport;
use serde_json::Value; use serde_json::Value;
@@ -37,7 +37,7 @@ pub enum ActionOutcome {
/// drives it with an action/event channel loop. Create one, /// drives it with an action/event channel loop. Create one,
/// grab the action sender and event subscriber, then call /// grab the action sender and event subscriber, then call
/// [`MenuRunner::run`] to start the event loop. /// [`MenuRunner::run`] to start the event loop.
pub struct MenuRunner<M: Menu> { pub struct MenuRunner<M: MutableMenu> {
menu: M, menu: M,
viewport: Viewport, viewport: Viewport,
filter_text: Arc<str>, filter_text: Arc<str>,
@@ -46,9 +46,10 @@ pub struct MenuRunner<M: Menu> {
event_tx: broadcast::Sender<MenuEvent>, event_tx: broadcast::Sender<MenuEvent>,
dispatcher: Option<DebouncedDispatcher>, dispatcher: Option<DebouncedDispatcher>,
previous_cursor: Option<usize>, previous_cursor: Option<usize>,
generation: u64,
} }
impl<M: Menu> MenuRunner<M> { impl<M: MutableMenu> MenuRunner<M> {
/// Create a menu runner wrapping the given menu backend. /// Create a menu runner wrapping the given menu backend.
/// Returns the runner and an action sender. Call /// Returns the runner and an action sender. Call
/// [`subscribe`](Self::subscribe) to get an event handle, /// [`subscribe`](Self::subscribe) to get an event handle,
@@ -69,6 +70,7 @@ impl<M: Menu> MenuRunner<M> {
event_tx, event_tx,
dispatcher: None, dispatcher: None,
previous_cursor: None, previous_cursor: None,
generation: 0,
}; };
(runner, action_tx) (runner, action_tx)
} }
@@ -105,7 +107,8 @@ impl<M: Menu> MenuRunner<M> {
/// Build a [`ViewState`] snapshot from the current filter /// Build a [`ViewState`] snapshot from the current filter
/// results and viewport position. /// results and viewport position.
fn build_view_state(&self) -> ViewState { fn build_view_state(&mut self) -> ViewState {
self.generation += 1;
let range = self.viewport.visible_range(); let range = self.viewport.visible_range();
let visible_items: Vec<VisibleItem> = range let visible_items: Vec<VisibleItem> = range
.clone() .clone()
@@ -134,14 +137,14 @@ impl<M: Menu> MenuRunner<M> {
total_items: self.menu.total(), total_items: self.menu.total(),
total_filtered: self.menu.filtered_count(), total_filtered: self.menu.filtered_count(),
mode: self.mode, mode: self.mode,
generation: self.generation,
} }
} }
/// Send the current view state to all subscribers. /// Send the current view state to all subscribers.
fn broadcast_state(&self) { fn broadcast_state(&mut self) {
let _ = self let vs = self.build_view_state();
.event_tx let _ = self.event_tx.send(MenuEvent::StateChanged(vs));
.send(MenuEvent::StateChanged(self.build_view_state()));
} }
/// Emit a hook event through the dispatcher, if one is set. /// Emit a hook event through the dispatcher, if one is set.
@@ -390,6 +393,7 @@ mod tests {
use super::*; use super::*;
use crate::event::MenuEvent; use crate::event::MenuEvent;
use crate::item::Item; use crate::item::Item;
use crate::model::traits::Menu;
use crate::runtime::json_menu::JsonMenu; use crate::runtime::json_menu::JsonMenu;
fn test_menu() -> (MenuRunner<JsonMenu>, mpsc::Sender<Action>) { fn test_menu() -> (MenuRunner<JsonMenu>, mpsc::Sender<Action>) {
@@ -1099,16 +1103,16 @@ mod tests {
#[tokio::test] #[tokio::test]
async fn hook_events_fire_on_lifecycle() { async fn hook_events_fire_on_lifecycle() {
use crate::hook::{HookEvent, HookEventKind, HookHandler, HookResponse}; use crate::hook::{HookEvent, HookEventKind, HookHandler};
use std::sync::Mutex; use std::sync::Mutex;
struct Recorder(Mutex<Vec<HookEventKind>>); struct Recorder(Mutex<Vec<HookEventKind>>);
impl HookHandler for Recorder { impl HookHandler for Recorder {
fn handle(&self, event: HookEvent) -> Result<Vec<HookResponse>, PiklError> { fn handle(&self, event: HookEvent) -> Result<(), PiklError> {
if let Ok(mut v) = self.0.lock() { if let Ok(mut v) = self.0.lock() {
v.push(event.kind()); v.push(event.kind());
} }
Ok(vec![]) Ok(())
} }
} }

View File

@@ -13,7 +13,7 @@ use tokio::sync::mpsc;
use pikl_core::error::PiklError; use pikl_core::error::PiklError;
use pikl_core::event::Action; use pikl_core::event::Action;
use pikl_core::hook::{ use pikl_core::hook::{
parse_hook_response, HookEvent, HookEventKind, HookHandler, HookResponse, parse_hook_response, HookEvent, HookEventKind, HookHandler,
}; };
/// A persistent handler hook process. Spawns a child process, /// A persistent handler hook process. Spawns a child process,
@@ -77,13 +77,13 @@ impl ShellHandlerHook {
} }
impl HookHandler for ShellHandlerHook { impl HookHandler for ShellHandlerHook {
fn handle(&self, event: HookEvent) -> Result<Vec<HookResponse>, PiklError> { fn handle(&self, event: HookEvent) -> Result<(), PiklError> {
let kind = event.kind(); let kind = event.kind();
if let Some(tx) = self.event_txs.get(&kind) { if let Some(tx) = self.event_txs.get(&kind) {
// Non-blocking send. If the channel is full, drop the event. // Non-blocking send. If the channel is full, drop the event.
let _ = tx.try_send(event); let _ = tx.try_send(event);
} }
Ok(vec![]) Ok(())
} }
} }

View File

@@ -9,7 +9,7 @@ use tokio::io::AsyncWriteExt;
use tokio::process::Command; use tokio::process::Command;
use pikl_core::error::PiklError; use pikl_core::error::PiklError;
use pikl_core::hook::{HookEvent, HookEventKind, HookHandler, HookResponse}; use pikl_core::hook::{HookEvent, HookEventKind, HookHandler};
/// Duplicate stderr as a [`Stdio`] handle for use as a /// Duplicate stderr as a [`Stdio`] handle for use as a
/// child process's stdout. Keeps hook output on stderr /// child process's stdout. Keeps hook output on stderr
@@ -18,8 +18,12 @@ fn stderr_as_stdio() -> std::process::Stdio {
#[cfg(unix)] #[cfg(unix)]
{ {
use std::os::unix::io::FromRawFd; use std::os::unix::io::FromRawFd;
// SAFETY: STDERR_FILENO is a valid open fd in any running process.
// dup() returns a new fd or -1, which we check below.
let fd = unsafe { libc::dup(libc::STDERR_FILENO) }; let fd = unsafe { libc::dup(libc::STDERR_FILENO) };
if fd >= 0 { if fd >= 0 {
// SAFETY: fd is valid (checked >= 0 above) and we take exclusive
// ownership here (no aliasing). The File will close it on drop.
return unsafe { std::process::Stdio::from(std::fs::File::from_raw_fd(fd)) }; return unsafe { std::process::Stdio::from(std::fs::File::from_raw_fd(fd)) };
} }
} }
@@ -112,7 +116,7 @@ impl ShellExecHandler {
} }
impl HookHandler for ShellExecHandler { impl HookHandler for ShellExecHandler {
fn handle(&self, event: HookEvent) -> Result<Vec<HookResponse>, PiklError> { fn handle(&self, event: HookEvent) -> Result<(), PiklError> {
let kind = event.kind(); let kind = event.kind();
if let Some(cmd) = self.commands.get(&kind) { if let Some(cmd) = self.commands.get(&kind) {
let cmd = cmd.clone(); let cmd = cmd.clone();
@@ -125,7 +129,7 @@ impl HookHandler for ShellExecHandler {
} }
}); });
} }
Ok(vec![]) Ok(())
} }
} }

View File

@@ -310,14 +310,14 @@ impl HookHandler for CompositeHookHandler {
fn handle( fn handle(
&self, &self,
event: pikl_core::hook::HookEvent, event: pikl_core::hook::HookEvent,
) -> Result<Vec<pikl_core::hook::HookResponse>, PiklError> { ) -> Result<(), PiklError> {
// Both fire. Exec is fire-and-forget, handler may // Both fire. Exec is fire-and-forget, handler may
// send responses through action_tx. // send responses through action_tx.
let _ = self.exec.handle(event.clone()); let _ = self.exec.handle(event.clone());
if let Some(ref h) = self.handler { if let Some(ref h) = self.handler {
h.handle(event)?; h.handle(event)?;
} }
Ok(vec![]) Ok(())
} }
} }