Refactor to eliminate keyboard/mouse handler disconnects
All checks were successful
Build and Release / build-and-release (push) Successful in 53s

Extract duplicate logic into shared action functions to ensure
consistent behavior between keyboard and mouse interactions:

- action_remove_from_playlist: Unified playlist removal logic
- action_play_from_playlist: Unified playlist playback with optional
  pause state preservation
- handle_context_menu_action: Unified context menu execution

Fixes:
- Remove from playlist now checks index before removal (was broken
  in keyboard 'd' handler)
- Mouse double-click on playlist now preserves pause state
- Context menu handling no longer duplicated across 400+ lines

All keyboard and mouse actions now use identical code paths,
eliminating state bugs from inconsistent implementations.
This commit is contained in:
Christoffer Martinsson 2025-12-11 15:16:57 +01:00
parent ffe7cd0090
commit f1412b4f8c
2 changed files with 104 additions and 176 deletions

View File

@ -1,6 +1,6 @@
[package]
name = "cm-player"
version = "0.1.18"
version = "0.1.19"
edition = "2021"
[dependencies]

View File

@ -144,6 +144,102 @@ fn action_stop(state: &mut AppState, player: &mut player::Player) -> Result<()>
Ok(())
}
fn action_remove_from_playlist(state: &mut AppState, player: &mut player::Player) -> Result<()> {
let was_playing_removed = state.playlist_index == state.selected_playlist_index;
state.remove_selected_playlist_item();
if state.playlist.is_empty() {
state.player_state = PlayerState::Stopped;
state.current_file = None;
player.stop()?;
} else if was_playing_removed && state.player_state == PlayerState::Playing {
state.current_file = Some(state.playlist[state.playlist_index].clone());
if let Some(ref path) = state.current_file {
player.play(path)?;
player.update_metadata();
}
}
Ok(())
}
fn action_play_from_playlist(state: &mut AppState, player: &mut player::Player, preserve_pause: bool) -> Result<()> {
state.playlist_index = state.selected_playlist_index;
state.current_file = Some(state.playlist[state.playlist_index].clone());
if preserve_pause {
match state.player_state {
PlayerState::Playing => {
if let Some(ref path) = state.current_file {
player.play(path)?;
player.update_metadata();
tracing::info!("Jumped to track: {:?}", path);
}
}
PlayerState::Paused => {
if let Some(ref path) = state.current_file {
player.play(path)?;
player.update_metadata();
player.pause()?;
tracing::info!("Jumped to track (paused): {:?}", path);
}
}
PlayerState::Stopped => {
state.player_state = PlayerState::Playing;
if let Some(ref path) = state.current_file {
player.play(path)?;
player.update_metadata();
tracing::info!("Started playing track: {:?}", path);
}
}
}
} else {
state.player_state = PlayerState::Playing;
if let Some(ref path) = state.current_file {
player.play(path)?;
player.update_metadata();
tracing::info!("Playing from playlist: {:?}", path);
}
}
Ok(())
}
fn handle_context_menu_action(menu_type: state::ContextMenuType, selected: usize, state: &mut AppState, player: &mut player::Player) -> Result<()> {
match menu_type {
state::ContextMenuType::FilePanel => {
match selected {
0 => action_play_selection(state, player)?,
1 => state.add_to_playlist(),
_ => {}
}
}
state::ContextMenuType::Playlist => {
match selected {
0 => action_remove_from_playlist(state, player)?,
1 => {
state.shuffle_playlist();
tracing::info!("Playlist randomised from context menu");
}
_ => {}
}
}
state::ContextMenuType::TitleBar => {
match selected {
0 => action_stop(state, player)?,
1 => {
state.cycle_play_mode();
tracing::info!("Play mode: {:?}", state.play_mode);
}
2 => {
state.show_refresh_confirm = true;
tracing::info!("Refresh requested from context menu");
}
_ => {}
}
}
}
Ok(())
}
async fn run_app<B: ratatui::backend::Backend>(
terminal: &mut Terminal<B>,
state: &mut AppState,
@ -361,64 +457,7 @@ async fn handle_key_event<B: ratatui::backend::Backend>(terminal: &mut Terminal<
let menu_type = menu.menu_type;
let selected = menu.selected_index;
state.context_menu = None;
match menu_type {
ContextMenuType::FilePanel => {
match selected {
0 => action_play_selection(state, player)?,
1 => state.add_to_playlist(),
_ => {}
}
}
ContextMenuType::Playlist => {
match selected {
0 => {
// Remove
let was_playing_removed = state.playlist_index == state.selected_playlist_index;
state.remove_selected_playlist_item();
// Handle edge cases after removal
if state.playlist.is_empty() {
state.player_state = PlayerState::Stopped;
state.current_file = None;
player.stop()?;
} else if was_playing_removed && state.player_state == PlayerState::Playing {
// Removed currently playing track, start new one at same index
state.current_file = Some(state.playlist[state.playlist_index].clone());
if let Some(ref path) = state.current_file {
player.play(path)?;
player.update_metadata();
}
}
}
1 => {
// Randomise
state.shuffle_playlist();
tracing::info!("Playlist randomised from context menu");
}
_ => {}
}
}
ContextMenuType::TitleBar => {
match selected {
0 => {
// Stop
action_stop(state, player)?;
}
1 => {
// Toggle Loop
state.cycle_play_mode();
tracing::info!("Play mode: {:?}", state.play_mode);
}
2 => {
// Refresh
state.show_refresh_confirm = true;
tracing::info!("Refresh requested from context menu");
}
_ => {}
}
}
}
handle_context_menu_action(menu_type, selected, state, player)?;
}
KeyCode::Esc => {
state.context_menu = None;
@ -604,39 +643,13 @@ async fn handle_key_event<B: ratatui::backend::Backend>(terminal: &mut Terminal<
}
(KeyCode::Char('d'), _) => {
if state.focus_playlist {
// Remove selected track from playlist
state.remove_selected_playlist_item();
// If removed currently playing track, handle it
if state.playlist.is_empty() {
state.player_state = PlayerState::Stopped;
state.current_file = None;
player.stop()?;
} else if state.playlist_index == state.selected_playlist_index {
// Removed currently playing track, play next one
if state.playlist_index < state.playlist.len() {
state.current_file = Some(state.playlist[state.playlist_index].clone());
if state.player_state == PlayerState::Playing {
if let Some(ref path) = state.current_file {
player.play(path)?;
player.update_metadata();
}
}
}
}
action_remove_from_playlist(state, player)?;
}
}
(KeyCode::Enter, _) => {
if state.focus_playlist {
// Play selected track from playlist
if state.selected_playlist_index < state.playlist.len() {
state.playlist_index = state.selected_playlist_index;
state.current_file = Some(state.playlist[state.playlist_index].clone());
state.player_state = PlayerState::Playing;
if let Some(ref path) = state.current_file {
player.play(path)?;
player.update_metadata();
tracing::info!("Playing from playlist: {:?}", path);
}
action_play_from_playlist(state, player, false)?;
}
} else {
action_play_selection(state, player)?;
@ -758,63 +771,7 @@ fn handle_mouse_event(state: &mut AppState, mouse: MouseEvent, title_bar_area: r
let menu_type = menu.menu_type;
let selected = relative_y;
state.context_menu = None;
match menu_type {
ContextMenuType::FilePanel => {
match selected {
0 => action_play_selection(state, player)?,
1 => state.add_to_playlist(),
_ => {}
}
}
ContextMenuType::Playlist => {
match selected {
0 => {
// Remove
let was_playing_removed = state.playlist_index == state.selected_playlist_index;
state.remove_selected_playlist_item();
// Handle edge cases after removal
if state.playlist.is_empty() {
state.player_state = PlayerState::Stopped;
state.current_file = None;
player.stop()?;
} else if was_playing_removed && state.player_state == PlayerState::Playing {
state.current_file = Some(state.playlist[state.playlist_index].clone());
if let Some(ref path) = state.current_file {
player.play(path)?;
player.update_metadata();
}
}
}
1 => {
// Randomise
state.shuffle_playlist();
tracing::info!("Playlist randomised from context menu (mouse)");
}
_ => {}
}
}
ContextMenuType::TitleBar => {
match selected {
0 => {
// Stop
action_stop(state, player)?;
}
1 => {
// Toggle Loop
state.cycle_play_mode();
tracing::info!("Play mode: {:?} (mouse)", state.play_mode);
}
2 => {
// Refresh
state.show_refresh_confirm = true;
tracing::info!("Refresh requested from context menu (mouse)");
}
_ => {}
}
}
}
handle_context_menu_action(menu_type, selected, state, player)?;
}
return Ok(());
} else {
@ -994,38 +951,9 @@ fn handle_mouse_event(state: &mut AppState, mouse: MouseEvent, title_bar_area: r
};
if is_double_click {
// Double click = play the track
state.playlist_index = actual_track;
state.current_file = Some(state.playlist[state.playlist_index].clone());
match state.player_state {
PlayerState::Playing => {
// Keep playing
if let Some(ref path) = state.current_file {
player.play(path)?;
player.update_metadata();
tracing::info!("Jumped to track: {:?}", path);
}
}
PlayerState::Paused => {
// Load but stay paused
if let Some(ref path) = state.current_file {
player.play(path)?;
player.update_metadata();
player.pause()?;
tracing::info!("Jumped to track (paused): {:?}", path);
}
}
PlayerState::Stopped => {
// Start playing from clicked track
state.player_state = PlayerState::Playing;
if let Some(ref path) = state.current_file {
player.play(path)?;
player.update_metadata();
tracing::info!("Started playing track: {:?}", path);
}
}
}
// Double click = play the track (preserve pause state)
state.selected_playlist_index = actual_track;
action_play_from_playlist(state, player, true)?;
// Reset click tracking after action
state.last_click_time = None;
state.last_click_index = None;