Eliminate code duplication with unified action functions
All checks were successful
Build and Release / build-and-release (push) Successful in 55s

Create action functions for stop, volume, and seek operations and
use them consistently across keyboard handlers, mouse handlers, and
API handlers. This eliminates duplicate logic and ensures consistent
behavior across all input methods.

Also fixes stop command triggering auto-advance by setting the
skip_position_update flag to prevent the Playing→Stopped transition
from being interpreted as a natural track ending.
This commit is contained in:
Christoffer Martinsson 2025-12-12 16:19:56 +01:00
parent be9ee8c005
commit d53542afa6
2 changed files with 50 additions and 54 deletions

View File

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

View File

@ -262,14 +262,44 @@ fn action_toggle_play_pause(state: &mut AppState, player: &mut player::Player) -
Ok(())
}
fn action_stop(state: &mut AppState, player: &mut player::Player) -> Result<()> {
fn action_stop(state: &mut AppState, player: &mut player::Player, skip_position_update: &mut bool) -> Result<()> {
player.stop()?;
state.current_position = 0.0;
state.current_duration = 0.0;
*skip_position_update = true; // Prevent auto-advance on manual stop
tracing::info!("Stopped");
Ok(())
}
fn action_volume_up(state: &mut AppState, player: &mut player::Player) -> Result<()> {
state.volume = (state.volume + 5).min(100);
player.set_volume(state.volume)?;
tracing::info!("Volume: {}%", state.volume);
Ok(())
}
fn action_volume_down(state: &mut AppState, player: &mut player::Player) -> Result<()> {
state.volume = (state.volume - 5).max(0);
player.set_volume(state.volume)?;
tracing::info!("Volume: {}%", state.volume);
Ok(())
}
fn action_volume_set(state: &mut AppState, player: &mut player::Player, volume: i64) -> Result<()> {
state.volume = volume.clamp(0, 100);
player.set_volume(state.volume)?;
tracing::info!("Volume: {}%", state.volume);
Ok(())
}
fn action_seek(player: &mut player::Player, seconds: f64) -> Result<()> {
if player.get_player_state() != Some(PlayerState::Stopped) {
player.seek(seconds)?;
tracing::info!("Seek {}s", seconds);
}
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;
let was_playing = player.get_player_state() == Some(PlayerState::Playing);
@ -415,7 +445,7 @@ fn handle_context_menu_action(menu_type: state::ContextMenuType, selected: usize
}
state::ContextMenuType::TitleBar => {
match selected {
0 => action_stop(state, player)?,
0 => action_stop(state, player, skip_position_update)?,
1 => {
state.cycle_play_mode();
tracing::info!("Play mode: {:?}", state.play_mode);
@ -453,26 +483,11 @@ fn run_app<B: ratatui::backend::Backend>(
tracing::debug!("Processing API command: {:?}", cmd);
match cmd {
api::ApiCommand::PlayPause => {
if let Some(player_state) = player.get_player_state() {
match player_state {
PlayerState::Stopped => {
// Play current file or first in playlist
if state.current_file.is_none() && !state.playlist.is_empty() {
state.current_file = Some(state.playlist[0].clone());
}
if let Some(ref file) = state.current_file {
player.play(file)?;
}
}
PlayerState::Playing => player.pause()?,
PlayerState::Paused => player.resume()?,
}
state_changed = true;
}
action_toggle_play_pause(state, player)?;
state_changed = true;
}
api::ApiCommand::Stop => {
player.stop()?;
state.current_file = None;
action_stop(state, player, &mut skip_position_update)?;
state_changed = true;
}
api::ApiCommand::Next => {
@ -484,26 +499,23 @@ fn run_app<B: ratatui::backend::Backend>(
state_changed = true;
}
api::ApiCommand::VolumeUp => {
state.volume = (state.volume + 5).min(100);
player.set_volume(state.volume)?;
action_volume_up(state, player)?;
state_changed = true;
}
api::ApiCommand::VolumeDown => {
state.volume = (state.volume - 5).max(0);
player.set_volume(state.volume)?;
action_volume_down(state, player)?;
state_changed = true;
}
api::ApiCommand::VolumeSet { volume } => {
state.volume = volume.clamp(0, 100);
player.set_volume(state.volume)?;
action_volume_set(state, player, volume)?;
state_changed = true;
}
api::ApiCommand::SeekForward { seconds } => {
player.seek(seconds)?;
action_seek(player, seconds)?;
state_changed = true;
}
api::ApiCommand::SeekBackward { seconds } => {
player.seek(-seconds)?;
action_seek(player, -seconds)?;
state_changed = true;
}
api::ApiCommand::GetStatus => {
@ -554,8 +566,10 @@ fn run_app<B: ratatui::backend::Backend>(
// Check if track ended and play next
// When MPV finishes playing a file, it goes to idle (Stopped state)
// Detect Playing → Stopped transition = track ended, play next
// But skip this check if we just manually stopped (skip_position_update flag)
if previous_player_state == Some(PlayerState::Playing)
&& player_state == PlayerState::Stopped
&& !skip_position_update
{
let should_continue = state.play_next();
// play_next() returns true if should continue playing, false if should stop
@ -894,7 +908,7 @@ fn handle_key_event<B: ratatui::backend::Backend>(terminal: &mut Terminal<B>, st
}
}
(KeyCode::Char('s'), _) => {
action_stop(state, player)?;
action_stop(state, player, skip_position_update)?;
}
(KeyCode::Char('m'), _) => {
state.cycle_play_mode();
@ -908,28 +922,16 @@ fn handle_key_event<B: ratatui::backend::Backend>(terminal: &mut Terminal<B>, st
action_toggle_play_pause(state, player)?;
}
(KeyCode::Char('H'), KeyModifiers::SHIFT) => {
if player.get_player_state() != Some(PlayerState::Stopped) {
player.seek(-10.0)?;
tracing::info!("Seek backward 10s");
}
action_seek(player, -10.0)?;
}
(KeyCode::Char('L'), KeyModifiers::SHIFT) => {
if player.get_player_state() != Some(PlayerState::Stopped) {
player.seek(10.0)?;
tracing::info!("Seek forward 10s");
}
action_seek(player, 10.0)?;
}
(KeyCode::Char('+'), _) | (KeyCode::Char('='), _) => {
let new_volume = (state.volume + 5).min(100);
state.volume = new_volume;
player.set_volume(new_volume)?;
tracing::info!("Volume: {}%", new_volume);
action_volume_up(state, player)?;
}
(KeyCode::Char('-'), _) => {
let new_volume = (state.volume - 5).max(0);
state.volume = new_volume;
player.set_volume(new_volume)?;
tracing::info!("Volume: {}%", new_volume);
action_volume_down(state, player)?;
}
(KeyCode::Char('r'), _) => {
state.show_refresh_confirm = true;
@ -1032,10 +1034,7 @@ fn handle_mouse_event(state: &mut AppState, mouse: MouseEvent, title_bar_area: r
&& y < title_bar_area.y + title_bar_area.height
{
// Scroll on title bar = decrease volume
let new_volume = (state.volume - 5).max(0);
state.volume = new_volume;
player.set_volume(new_volume)?;
tracing::info!("Volume: {}%", new_volume);
action_volume_down(state, player)?;
} else if x >= playlist_area.x
&& x < playlist_area.x + playlist_area.width
&& y >= playlist_area.y
@ -1058,10 +1057,7 @@ fn handle_mouse_event(state: &mut AppState, mouse: MouseEvent, title_bar_area: r
&& y < title_bar_area.y + title_bar_area.height
{
// Scroll on title bar = increase volume
let new_volume = (state.volume + 5).min(100);
state.volume = new_volume;
player.set_volume(new_volume)?;
tracing::info!("Volume: {}%", new_volume);
action_volume_up(state, player)?;
} else if x >= playlist_area.x
&& x < playlist_area.x + playlist_area.width
&& y >= playlist_area.y