From 6a7050e0c76506d1830d3886cbffdd77c6cd9e72 Mon Sep 17 00:00:00 2001 From: devmobasa <4170275+devmobasa@users.noreply.github.com> Date: Sat, 15 Aug 2026 23:16:50 +0200 Subject: [PATCH] feat(session): add catalog-only rename command --- src/app/mod.rs | 3 +- src/app/session.rs | 20 +++++++++ src/app/usage.rs | 3 ++ src/cli.rs | 80 +++++++++++++++++++++++++++++++--- src/cli/tests.rs | 84 ++++++++++++++++++++++++++++++++++-- src/session/catalog.rs | 24 +++++++++++ src/session/catalog/tests.rs | 29 +++++++++++++ tests/cli.rs | 49 +++++++++++++++++++++ 8 files changed, 280 insertions(+), 12 deletions(-) diff --git a/src/app/mod.rs b/src/app/mod.rs index 6eeec2f0..3282ede0 100644 --- a/src/app/mod.rs +++ b/src/app/mod.rs @@ -223,7 +223,8 @@ pub fn run(cli: Cli) -> anyhow::Result<()> { return Ok(()); } - if cli.clear_session || cli.clear_tool_state || cli.session_info { + if cli.clear_session || cli.clear_tool_state || cli.session_info || cli.rename_session.is_some() + { run_session_cli_commands(&cli)?; return Ok(()); } diff --git a/src/app/session.rs b/src/app/session.rs index 6819cffb..5f57cc25 100644 --- a/src/app/session.rs +++ b/src/app/session.rs @@ -2,6 +2,26 @@ use crate::cli::Cli; use crate::env_vars::WAYLAND_DISPLAY_ENV; pub(crate) fn run_session_cli_commands(cli: &Cli) -> anyhow::Result<()> { + if let Some(display_name) = cli.rename_session.as_deref() { + let raw_path = cli + .session_file + .as_ref() + .ok_or_else(|| anyhow::anyhow!("--rename-session requires --session-file"))?; + let raw = raw_path + .to_str() + .ok_or_else(|| anyhow::anyhow!("--session-file path must be valid UTF-8"))?; + let path = crate::session::normalize_named_session_file_arg(raw); + match crate::session::catalog::rename_session_display_name_by_path(&path, display_name)? { + Some(entry) => { + println!("Renamed session to {}.", entry.display_name); + } + None => { + anyhow::bail!("session is not in the named-session catalog"); + } + } + return Ok(()); + } + let loaded = crate::config::Config::load()?; // [session] configures which files these commands act on. A load that // fell back to defaults for that section would silently retarget a diff --git a/src/app/usage.rs b/src/app/usage.rs index 25c432f4..a5a07660 100644 --- a/src/app/usage.rs +++ b/src/app/usage.rs @@ -182,6 +182,9 @@ pub(crate) fn print_usage() { println!(" wayscriber --active --session-file PATH Use a named session file"); println!(" wayscriber --freeze --session-file PATH Use a named session file"); println!(" wayscriber --session-info [--session-file PATH] Inspect saved session data"); + println!( + " wayscriber --rename-session NAME --session-file PATH Rename a catalog display name" + ); println!(" wayscriber --clear-session [--session-file PATH] Remove saved session data"); println!( " wayscriber --clear-tool-state [--session-file PATH] Reset saved tool defaults only" diff --git a/src/cli.rs b/src/cli.rs index 8734885e..b55dd734 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -47,6 +47,9 @@ pub struct Cli { /// Show session persistence status and file paths pub session_info: bool, + /// Rename a named session's catalog display name (session files are untouched) + pub rename_session: Option, + /// Use a named session file for active/freeze/info/clear operations pub session_file: Option, @@ -132,6 +135,10 @@ impl Cli { "--clear-session" => cli.clear_session = true, "--clear-tool-state" => cli.clear_tool_state = true, "--session-info" => cli.session_info = true, + "--rename-session" => { + index += 1; + cli.rename_session = Some(value_after(&args, index, "--rename-session")?); + } "--session-file" => { index += 1; cli.session_file = @@ -157,6 +164,9 @@ impl Cli { cli.session_file = Some(PathBuf::from(value_from_equals(arg, "--session-file")?)); } + _ if arg.starts_with("--rename-session=") => { + cli.rename_session = Some(value_from_equals(arg, "--rename-session")?); + } _ if is_short_option_cluster(arg) => { match parse_short_option_cluster(&args, index, &mut cli)? { ShortOptionOutcome::Continue(next_index) => index = next_index, @@ -214,6 +224,7 @@ impl Cli { || self.clear_session || self.clear_tool_state || self.session_info + || self.rename_session.is_some() || self.session_file.is_some() || self.freeze || self.exit_after_capture @@ -222,6 +233,29 @@ impl Cli { || self.no_resume_session } + /// Whether an option belongs to an overlay launch or daemon interaction. + /// + /// Catalog-only commands must reject these options because they return + /// before any overlay or daemon behavior can honor them. + fn selects_overlay_option(&self) -> bool { + self.daemon + || self.daemon_toggle + || self.daemon_action.is_some() + || self.light_toggle + || self.light_draw_toggle + || self.light_draw_on + || self.light_draw_off + || self.active + || self.mode.is_some() + || self.no_tray + || self.freeze_on_show + || self.freeze + || self.exit_after_capture + || self.no_exit_after_capture + || self.resume_session + || self.no_resume_session + } + fn validate(&self) -> Result<(), String> { if self.runtime_capabilities && (self.selects_a_launch_command() || self.about || self.check_update) @@ -244,6 +278,22 @@ impl Cli { if self.clear_tool_state && self.session_info { return Err(conflict("--clear-tool-state", "--session-info")); } + if self.rename_session.is_some() && self.session_info { + return Err(conflict("--rename-session", "--session-info")); + } + if self.rename_session.is_some() && self.clear_session { + return Err(conflict("--rename-session", "--clear-session")); + } + if self.rename_session.is_some() && self.clear_tool_state { + return Err(conflict("--rename-session", "--clear-tool-state")); + } + + if self.rename_session.is_some() && self.session_file.is_none() { + return Err("--rename-session requires --session-file".to_string()); + } + if self.rename_session.is_some() && self.selects_overlay_option() { + return Err("--rename-session conflicts with overlay/daemon options".to_string()); + } if self.freeze_on_show && !self.daemon { return Err("--freeze-on-show requires --daemon".to_string()); @@ -282,10 +332,11 @@ impl Cli { || self.daemon_toggle || self.clear_session || self.clear_tool_state - || self.session_info) + || self.session_info + || self.rename_session.is_some()) { return Err( - "--session-file requires --active, --freeze, --daemon, --daemon-toggle, --session-info, --clear-session, or --clear-tool-state" + "--session-file requires --active, --freeze, --daemon, --daemon-toggle, --session-info, --clear-session, --clear-tool-state, or --rename-session" .to_string(), ); } @@ -307,6 +358,7 @@ impl Cli { || self.clear_session || self.clear_tool_state || self.session_info + || self.rename_session.is_some() || self.about) { return Err("--daemon-toggle conflicts with the selected command".to_string()); @@ -325,6 +377,7 @@ impl Cli { || self.clear_session || self.clear_tool_state || self.session_info + || self.rename_session.is_some() || self.session_file.is_some() || self.freeze || self.exit_after_capture @@ -346,22 +399,33 @@ impl Cli { return Err("--session-info conflicts with --daemon/--active".to_string()); } if self.freeze - && (self.daemon || self.clear_session || self.clear_tool_state || self.session_info) + && (self.daemon + || self.clear_session + || self.clear_tool_state + || self.session_info + || self.rename_session.is_some()) { return Err("--freeze conflicts with the selected command".to_string()); } - if (self.clear_session || self.clear_tool_state || self.session_info) && self.resume_session + if (self.clear_session + || self.clear_tool_state + || self.session_info + || self.rename_session.is_some()) + && self.resume_session { return Err( - "--resume-session conflicts with --clear-session/--session-info/--clear-tool-state" + "--resume-session conflicts with --clear-session/--session-info/--clear-tool-state/--rename-session" .to_string(), ); } - if (self.clear_session || self.clear_tool_state || self.session_info) + if (self.clear_session + || self.clear_tool_state + || self.session_info + || self.rename_session.is_some()) && self.no_resume_session { return Err( - "--no-resume-session conflicts with --clear-session/--session-info/--clear-tool-state" + "--no-resume-session conflicts with --clear-session/--session-info/--clear-tool-state/--rename-session" .to_string(), ); } @@ -469,6 +533,7 @@ pub(crate) fn print_help() { println!(" wayscriber --active --session-file PATH"); println!(" wayscriber --freeze [--session-file PATH]"); println!(" wayscriber --session-info [--session-file PATH]"); + println!(" wayscriber --rename-session NAME --session-file PATH"); println!(" wayscriber --clear-session [--session-file PATH]"); println!(" wayscriber --clear-tool-state [--session-file PATH]"); println!(" wayscriber --about"); @@ -494,6 +559,7 @@ pub(crate) fn print_help() { println!(" --clear-session Delete persisted session data and backups"); println!(" --clear-tool-state Remove saved tool defaults but keep boards"); println!(" --session-info Show session persistence status"); + println!(" --rename-session NAME Rename a named session catalog label"); println!(" --session-file PATH Use a named session file"); println!(" --about Show the About window"); println!(" --check-update Check wayscriber.com for a newer release"); diff --git a/src/cli/tests.rs b/src/cli/tests.rs index ca0b52d6..353335d1 100644 --- a/src/cli/tests.rs +++ b/src/cli/tests.rs @@ -165,7 +165,7 @@ fn session_file_requires_supported_command() { ]); assert_eq!( result.unwrap_err(), - "--session-file requires --active, --freeze, --daemon, --daemon-toggle, --session-info, --clear-session, or --clear-tool-state" + "--session-file requires --active, --freeze, --daemon, --daemon-toggle, --session-info, --clear-session, --clear-tool-state, or --rename-session" ); } @@ -239,24 +239,100 @@ fn offline_session_commands_reject_resume_overrides() { let info_result = Cli::try_parse_from(["wayscriber", "--session-info", "--resume-session"]); assert_eq!( info_result.unwrap_err(), - "--resume-session conflicts with --clear-session/--session-info/--clear-tool-state" + "--resume-session conflicts with --clear-session/--session-info/--clear-tool-state/--rename-session" ); let clear_result = Cli::try_parse_from(["wayscriber", "--clear-session", "--no-resume-session"]); assert_eq!( clear_result.unwrap_err(), - "--no-resume-session conflicts with --clear-session/--session-info/--clear-tool-state" + "--no-resume-session conflicts with --clear-session/--session-info/--clear-tool-state/--rename-session" ); let tool_state_result = Cli::try_parse_from(["wayscriber", "--clear-tool-state", "--resume-session"]); assert_eq!( tool_state_result.unwrap_err(), - "--resume-session conflicts with --clear-session/--session-info/--clear-tool-state" + "--resume-session conflicts with --clear-session/--session-info/--clear-tool-state/--rename-session" ); } +#[test] +fn rename_session_requires_session_file_and_accepts_equals_form() { + let missing = Cli::try_parse_from(["wayscriber", "--rename-session", "Lecture"]); + assert_eq!( + missing.unwrap_err(), + "--rename-session requires --session-file" + ); + + let cli = parse_cli([ + "wayscriber", + "--rename-session=Lecture 04", + "--session-file", + "/tmp/lecture.wayscriber-session", + ]); + assert_eq!(cli.rename_session.as_deref(), Some("Lecture 04")); + assert_eq!( + cli.session_file, + Some(PathBuf::from("/tmp/lecture.wayscriber-session")) + ); +} + +#[test] +fn rename_session_conflicts_with_other_session_commands() { + let result = Cli::try_parse_from([ + "wayscriber", + "--rename-session", + "Lecture", + "--session-file", + "/tmp/lecture.wayscriber-session", + "--session-info", + ]); + assert_eq!( + result.unwrap_err(), + "--rename-session conflicts with --session-info" + ); +} + +#[test] +fn rename_session_rejects_every_overlay_option() { + let overlay_options = [ + vec!["--daemon"], + vec!["--daemon-toggle"], + vec!["--daemon-action", "toggle_help"], + vec!["--light-toggle"], + vec!["--light-draw-toggle"], + vec!["--light-draw-on"], + vec!["--light-draw-off"], + vec!["--active"], + vec!["--mode", "whiteboard"], + vec!["--no-tray"], + vec!["--freeze-on-show"], + vec!["--freeze"], + vec!["--exit-after-capture"], + vec!["--no-exit-after-capture"], + vec!["--resume-session"], + vec!["--no-resume-session"], + ]; + + for option in overlay_options { + let mut args = vec![ + "wayscriber", + "--rename-session", + "Lecture", + "--session-file", + "/tmp/lecture.wayscriber-session", + ]; + args.extend(option.iter().copied()); + + assert_eq!( + Cli::try_parse_from(args).unwrap_err(), + "--rename-session conflicts with overlay/daemon options", + "expected rename with {option:?} to be rejected" + ); + } +} + #[test] fn offline_session_commands_conflict_with_each_other() { let clear_result = Cli::try_parse_from(["wayscriber", "--clear-tool-state", "--clear-session"]); diff --git a/src/session/catalog.rs b/src/session/catalog.rs index f779d07f..e7f0ee55 100644 --- a/src/session/catalog.rs +++ b/src/session/catalog.rs @@ -150,6 +150,30 @@ pub fn rename_session_display_name_by_id( }) } +/// Rename a catalog entry's display name by session path. Session files are untouched. +#[allow(dead_code)] +pub fn rename_session_display_name_by_path( + path: &Path, + display_name: &str, +) -> Result> { + let display_name = display_name.trim(); + if display_name.is_empty() { + return Err(anyhow!("session display name cannot be empty")); + } + let identity = session_path_identity(path); + with_catalog_write(|catalog| { + let Some(entry) = catalog + .sessions + .iter_mut() + .find(|entry| entry_matches_identity(entry, &identity)) + else { + return Ok(None); + }; + entry.display_name = display_name.to_string(); + Ok(Some(entry.clone())) + }) +} + /// Update a catalog entry's session path after a committed disk move. #[allow(dead_code)] pub fn move_session_path_by_id(id: &str, target_path: &Path) -> Result> { diff --git a/src/session/catalog/tests.rs b/src/session/catalog/tests.rs index 2d79d5b3..b19bd1b0 100644 --- a/src/session/catalog/tests.rs +++ b/src/session/catalog/tests.rs @@ -332,6 +332,35 @@ fn rename_display_name_rejects_empty_names() { assert!(format!("{err:#}").contains("display name cannot be empty")); } +#[test] +fn rename_display_name_by_path_changes_metadata_only() { + let temp = crate::test_temp::tempdir().unwrap(); + let _env = EnvGuard::set_xdg_data_home(temp.path()); + let session = temp.path().join("lecture.wayscriber-session"); + fs::write(&session, b"{}").unwrap(); + upsert_session_event(&session, CatalogEvent::Saved).unwrap(); + + let renamed = rename_session_display_name_by_path(&session, "Lecture 04") + .expect("rename by path should work") + .expect("entry should exist"); + + assert_eq!(renamed.display_name, "Lecture 04"); + assert!(session.exists(), "rename should not touch the session file"); + let recents = recent_sessions().unwrap(); + assert_eq!(recents[0].display_name, "Lecture 04"); +} + +#[test] +fn rename_display_name_by_path_returns_none_for_unknown_path() { + let temp = crate::test_temp::tempdir().unwrap(); + let _env = EnvGuard::set_xdg_data_home(temp.path()); + let missing = temp.path().join("missing.wayscriber-session"); + + let renamed = rename_session_display_name_by_path(&missing, "Lecture") + .expect("unknown path is not a catalog write error"); + assert!(renamed.is_none()); +} + #[test] fn failed_temp_write_leaves_existing_catalog_intact() { let temp = crate::test_temp::tempdir().unwrap(); diff --git a/tests/cli.rs b/tests/cli.rs index 47268182..8002e5a9 100644 --- a/tests/cli.rs +++ b/tests/cli.rs @@ -458,6 +458,55 @@ fn session_clear_command_succeeds_without_files() { .stdout_contains("No session file present"); } +#[test] +fn rename_session_ignores_invalid_configuration() { + let temp = TempDir::new().unwrap(); + let config_dir = temp.path().join("config"); + let data_dir = temp.path().join("data"); + let session = temp.path().join("lecture.wayscriber-session"); + let catalog_dir = data_dir.join("wayscriber"); + fs::create_dir_all(config_dir.join("wayscriber")).unwrap(); + fs::create_dir_all(&catalog_dir).unwrap(); + fs::write( + config_dir.join("wayscriber/config.toml"), + b"not valid toml = [", + ) + .unwrap(); + fs::write(&session, b"{}").unwrap(); + + let catalog_path = catalog_dir.join("sessions.json"); + let catalog = serde_json::json!({ + "version": 1, + "sessions": [{ + "id": "s-test", + "display_name": "Before", + "path": session.to_str().unwrap(), + "canonical_path": session.canonicalize().unwrap().to_str().unwrap(), + "created_at_millis": 1, + "last_opened_at_millis": 1, + "last_saved_at_millis": null + }] + }); + fs::write(&catalog_path, serde_json::to_vec(&catalog).unwrap()).unwrap(); + + run_command( + wayscriber_cmd() + .env(XDG_CONFIG_HOME_ENV, &config_dir) + .env(XDG_DATA_HOME_ENV, &data_dir) + .env_remove(WAYLAND_DISPLAY_ENV) + .arg("--rename-session") + .arg("Lecture 04") + .arg("--session-file") + .arg(&session), + ) + .success() + .stdout_contains("Renamed session to Lecture 04."); + + let updated: serde_json::Value = + serde_json::from_slice(&fs::read(catalog_path).unwrap()).unwrap(); + assert_eq!(updated["sessions"][0]["display_name"], "Lecture 04"); +} + #[test] fn named_session_info_missing_parent_reports_not_found() { let temp = TempDir::new().unwrap();