From 284e70ffe926e677adf0648a60ce88eec7136600 Mon Sep 17 00:00:00 2001 From: Studentweis Date: Thu, 22 Oct 2026 19:55:09 +0800 Subject: [PATCH] fix(app): preserve selection and reconcile automatic startup Refs #300 --- Cargo.lock | 2 + Cargo.toml | 6 +- README.md | 5 +- docs/README/README_ZH.md | 5 +- docs/WAYLAND.md | 6 +- scripts/init.sh | 8 +- scripts/tests/test_init.py | 49 ++++++++++ src/config/autostart.rs | 169 +++++++++++++++++----------------- src/config/autostart/linux.rs | 152 ++++++++++++++++++++++++++++++ src/gui/board/actions.rs | 10 +- src/gui/board/filtering.rs | 36 +++----- src/gui/board/record_ops.rs | 29 +++--- src/gui/board/ui_tests.rs | 93 +++++++++++++++++++ src/gui/tray.rs | 61 +++++------- src/gui/window.rs | 19 ++-- 15 files changed, 463 insertions(+), 187 deletions(-) create mode 100644 scripts/tests/test_init.py create mode 100644 src/config/autostart/linux.rs diff --git a/Cargo.lock b/Cargo.lock index a9270047..01e4b335 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6322,6 +6322,8 @@ dependencies = [ "tray-icon", "wayland-client", "wayland-protocols", + "windows-registry 0.6.1", + "windows-result 0.4.1", "windows-sys 0.61.2", "winres", "x11rb 0.14.0", diff --git a/Cargo.toml b/Cargo.toml index fe298c69..f09972c6 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -24,7 +24,6 @@ thiserror.workspace = true toml.workspace = true image.workspace = true tray-icon = { version = "0.26.1", default-features = false } -auto-launch = "0.6.0" raw-window-handle = "0.6.2" async-channel = "2.5.0" rust-i18n = "4.2.4" @@ -47,11 +46,16 @@ tracing-subscriber = { version = "0.3.23", default-features = false, features = "time", ] } +[target.'cfg(not(target_os = "linux"))'.dependencies] +auto-launch = "0.6.0" + [target.'cfg(target_os = "macos")'.dependencies] objc2 = "0.6.5" [target.'cfg(target_os = "windows")'.dependencies] clipboard-win = "5.4.1" +windows-registry = "0.6.1" +windows-result = "0.4.1" windows-sys = { version = "0.61.2", features = [ "Win32_Graphics_Gdi", "Win32_System_Threading", diff --git a/README.md b/README.md index 777b1db7..1d687dec 100644 --- a/README.md +++ b/README.md @@ -55,10 +55,11 @@ still blocks launch, review and allow Ropy in System Settings → Privacy & Secu After downloading the `.dmg` file and dragging Ropy.app to the Applications folder, you may need to remove the quarantine attribute to run the application without issues. Open Terminal and run the following commands: ```sh -xattr -rc /Applications/Ropy.app -sudo xattr -r -d com.apple.quarantine /Applications/Ropy.app +xattr -r -d com.apple.quarantine /Applications/Ropy.app ``` +If the command reports a permission error, repeat it with `sudo`. + ### Windows You can install Ropy using [Scoop](https://scoop.sh/): diff --git a/docs/README/README_ZH.md b/docs/README/README_ZH.md index 8c82107c..aaa032d1 100644 --- a/docs/README/README_ZH.md +++ b/docs/README/README_ZH.md @@ -56,10 +56,11 @@ Homebrew 管理。当前发布尚未签名，自建 Cask 会在安装后自动 下载 `.dmg` 文件并将 Ropy.app 拖到应用程序文件夹后，您可能需要移除隔离属性才能正常运行应用程序。打开终端并运行以下命令： ```sh -xattr -rc /Applications/Ropy.app -sudo xattr -r -d com.apple.quarantine /Applications/Ropy.app +xattr -r -d com.apple.quarantine /Applications/Ropy.app ``` +如果命令提示权限不足，在同一命令前加上 `sudo` 后重试。 + ### Windows 您可以使用 [Scoop](https://scoop.sh/) 安装 Ropy： diff --git a/docs/WAYLAND.md b/docs/WAYLAND.md index fb15a977..a476a418 100644 --- a/docs/WAYLAND.md +++ b/docs/WAYLAND.md @@ -57,11 +57,7 @@ focus for its own process after the popup maps. No configuration-file edits or extra activation process are needed. Changing or clearing the shortcut updates the runtime binding; configuration reloads restore it while Ropy is running. -The adapter refuses conflicting bindings instead of replacing them. Save failures -retain the previous shortcut. Letters, digits, F1–F24, navigation keys and common -punctuation are supported; unsupported keys fail registration. Physical-keycode -bindings with the same modifiers are conservatively treated as conflicts. Remove -previous manual Ropy bindings before assigning the same chord in Ropy. +The adapter refuses confirmed symbol-key and catch-all conflicts instead of replacing existing bindings. Save failures retain the previous shortcut. Letters, digits, F1–F24, navigation keys and common punctuation are supported; unsupported keys fail registration. Hyprland’s binding list does not include the keymap needed to resolve numeric physical keycodes, so a physical-keycode binding alone is not treated as a confirmed conflict. Check those bindings manually and remove previous manual Ropy bindings before assigning the same chord in Ropy. On shutdown, Ropy disables only its own Lua binding handle. Disabled handles are reused on later registration and removed by Hyprland's next configuration reload. diff --git a/scripts/init.sh b/scripts/init.sh index fc3c0d9b..837f9c2a 100755 --- a/scripts/init.sh +++ b/scripts/init.sh @@ -1,5 +1,9 @@ #!/usr/bin/env bash -set -e +set -euo pipefail + +ROOT="$(cd "$(dirname "${BASH_SOURCE[0]}")/.." && pwd)" +cd "$ROOT" +DIST_VERSION="$(python3 -c 'import tomllib; print(tomllib.load(open("dist-workspace.toml", "rb"))["dist"]["cargo-dist-version"])')" # Parser hooks use uv's isolated Python tool environments. if ! command -v uvx &>/dev/null; then @@ -11,7 +15,7 @@ fi cargo install cargo-binstall # Other dependencies -cargo binstall 'hk@^2.5' cargo-dist cargo-release git-cliff cargo-outdated +cargo binstall 'hk@^2.5' "cargo-dist@$DIST_VERSION" cargo-release git-cliff cargo-outdated # For pre-commit hooks # Replace the previous hook script, including on Git with config-based hooks. diff --git a/scripts/tests/test_init.py b/scripts/tests/test_init.py new file mode 100644 index 00000000..db5a8c09 --- /dev/null +++ b/scripts/tests/test_init.py @@ -0,0 +1,49 @@ +"""Check setup tool selection without installing tools or Git hooks.""" + +import os +from pathlib import Path +import shutil +import subprocess +import tempfile +import unittest + + +ROOT = Path(__file__).resolve().parents[2] + + +class SetupTests(unittest.TestCase): + def test_setup_installs_configured_dist_from_its_repository(self): + with tempfile.TemporaryDirectory() as directory: + work = Path(directory).resolve() + repository = work / "repository" + scripts = repository / "scripts" + scripts.mkdir(parents=True) + shutil.copyfile(ROOT / "scripts/init.sh", scripts / "init.sh") + commands = work / "bin" + commands.mkdir() + log = work / "commands" + for name, body in { + "cargo": 'printf "%s\\n" "$*" >> "$INIT_TEST_LOG"', + "hk": 'printf "hooks in %s\\n" "$PWD" >> "$INIT_TEST_LOG"', + "uvx": "exit 0", + }.items(): + command = commands / name + command.write_text("#!/bin/sh\nset -eu\n" + body + "\n") + command.chmod(0o755) + env = dict(os.environ, PATH=f"{commands}:{os.environ['PATH']}", INIT_TEST_LOG=str(log)) + for version in ("0.32.0", "9.9.9"): + with self.subTest(version=version): + (repository / "dist-workspace.toml").write_text( + f'[dist]\ncargo-dist-version = "{version}"\n' + ) + log.unlink(missing_ok=True) + result = subprocess.run( + ["bash", str(scripts / "init.sh")], cwd=work, env=env, + capture_output=True, text=True, + ) + self.assertEqual(result.returncode, 0, result.stderr) + self.assertEqual(log.read_text().splitlines(), [ + "install cargo-binstall", + f"binstall hk@^2.5 cargo-dist@{version} cargo-release git-cliff cargo-outdated", + f"hooks in {repository}", + ]) diff --git a/src/config/autostart.rs b/src/config/autostart.rs index e01685dd..cfc2e854 100644 --- a/src/config/autostart.rs +++ b/src/config/autostart.rs @@ -5,9 +5,13 @@ use std::{env, path::Path}; use auto_launch::MacOSLaunchMode; #[cfg(target_os = "windows")] use auto_launch::WindowsEnableMode; +#[cfg(not(target_os = "linux"))] use auto_launch::{AutoLaunch, AutoLaunchBuilder}; use thiserror::Error; +#[cfg(any(target_os = "linux", test))] +mod linux; + #[derive(Debug, Error)] pub(crate) enum AutoStartError { #[error("Failed to get executable path: {0}")] @@ -22,10 +26,12 @@ pub(crate) enum AutoStartError { StatusCheck(String), } -/// Owns the platform `AutoLaunch` handle. The app always boots hidden into -/// the tray, so no extra CLI flag is needed to differentiate a -/// system-triggered launch from a user-triggered one. +/// Manages the platform login registration. The app always boots hidden into +/// the tray, so system-triggered launches need no additional CLI flag. pub(crate) struct AutoStartManager { + #[cfg(target_os = "linux")] + desktop_entry: linux::DesktopEntry, + #[cfg(not(target_os = "linux"))] auto_launch: AutoLaunch, #[cfg(target_os = "macos")] legacy_launch_agent: AutoLaunch, @@ -35,20 +41,38 @@ impl AutoStartManager { pub(crate) fn new(app_name: &str) -> Result { let app_path = Self::get_app_path()?; - let mut builder = AutoLaunchBuilder::new(); - builder.set_app_name(app_name).set_app_path(&app_path); + #[cfg(target_os = "linux")] + let desktop_entry = linux::DesktopEntry::new( + &dirs::config_dir().ok_or_else(|| { + AutoStartError::Initialization("config directory not found".into()) + })?, + app_name, + &app_path, + ); - #[cfg(target_os = "macos")] - builder.set_macos_launch_mode(MacOSLaunchMode::AppleScript); + #[cfg(not(target_os = "linux"))] + let auto_launch = { + // Windows Run values are command lines, so quote the executable. + #[cfg(target_os = "windows")] + let app_path = format!("\"{app_path}\""); + let mut builder = AutoLaunchBuilder::new(); + builder.set_app_name(app_name).set_app_path(&app_path); - #[cfg(target_os = "windows")] - builder.set_windows_enable_mode(WindowsEnableMode::CurrentUser); + #[cfg(target_os = "macos")] + builder.set_macos_launch_mode(MacOSLaunchMode::AppleScript); - let auto_launch = builder.build().map_err(|e| { - AutoStartError::Initialization(format!("Failed to build AutoLaunch: {e}")) - })?; + #[cfg(target_os = "windows")] + builder.set_windows_enable_mode(WindowsEnableMode::CurrentUser); + + builder.build().map_err(|e| { + AutoStartError::Initialization(format!("Failed to build AutoLaunch: {e}")) + })? + }; Ok(Self { + #[cfg(target_os = "linux")] + desktop_entry, + #[cfg(not(target_os = "linux"))] auto_launch, #[cfg(target_os = "macos")] legacy_launch_agent: Self::build_macos_legacy_launch_agent(app_name, &app_path)?, @@ -100,40 +124,57 @@ impl AutoStartManager { }) } - pub(crate) fn enable(&self) -> Result<(), AutoStartError> { - self.auto_launch - .enable() - .map_err(|e| AutoStartError::Enable(e.to_string())) - } - - pub(crate) fn disable(&self) -> Result<(), AutoStartError> { - self.auto_launch - .disable() - .map_err(|e| AutoStartError::Disable(e.to_string())) - } - - pub(crate) fn is_enabled(&self) -> Result { - self.auto_launch - .is_enabled() - .map_err(|e| AutoStartError::StatusCheck(e.to_string())) - } - - /// Make the system state match the user's preference, but skip the - /// platform call when it already matches — repeatedly toggling - /// Login Items / registry entries is unnecessary churn and (on - /// some Linux desktops) racy. + /// Reconcile the registered target as well as the enabled state. Matching + /// registrations are left intact to avoid unnecessary native writes. pub(crate) fn sync_state(&self, enabled: bool) -> Result<(), AutoStartError> { - let current_enabled = self.is_enabled()?; - match autostart_sync_action(enabled, current_enabled) { - AutoStartSyncAction::None => Ok(()), - AutoStartSyncAction::Enable => self.enable(), - AutoStartSyncAction::Disable => self.disable(), - }?; + #[cfg(target_os = "linux")] + { + self.desktop_entry.sync_state(enabled) + } + #[cfg(not(target_os = "linux"))] + { + let current_enabled = self + .auto_launch + .is_enabled() + .map_err(|error| AutoStartError::StatusCheck(error.to_string()))?; + #[cfg(target_os = "windows")] + let target_matches = !enabled || !current_enabled || self.windows_target_matches()?; + #[cfg(not(target_os = "windows"))] + let target_matches = true; + + if enabled && (!current_enabled || !target_matches) { + self.auto_launch + .enable() + .map_err(|error| AutoStartError::Enable(error.to_string()))?; + } else if !enabled && current_enabled { + self.auto_launch + .disable() + .map_err(|error| AutoStartError::Disable(error.to_string()))?; + } - #[cfg(target_os = "macos")] - self.disable_macos_legacy_launch_agent()?; + #[cfg(target_os = "macos")] + self.disable_macos_legacy_launch_agent()?; + Ok(()) + } + } - Ok(()) + #[cfg(target_os = "windows")] + fn windows_target_matches(&self) -> Result { + let registered = windows_registry::CURRENT_USER + .open(r"SOFTWARE\Microsoft\Windows\CurrentVersion\Run") + .and_then(|key| key.get_string(self.auto_launch.get_app_name())); + match registered { + Ok(command) => Ok(command.trim_end() == self.auto_launch.get_app_path()), + Err(error) + if error.code() + == windows_result::HRESULT::from_win32( + windows_sys::Win32::Foundation::ERROR_FILE_NOT_FOUND, + ) => + { + Ok(false) + } + Err(error) => Err(AutoStartError::StatusCheck(error.to_string())), + } } #[cfg(target_os = "macos")] @@ -144,27 +185,6 @@ impl AutoStartManager { } } -#[derive(Debug, Eq, PartialEq)] -enum AutoStartSyncAction { - None, - Enable, - Disable, -} - -const fn autostart_sync_action(enabled: bool, current_enabled: bool) -> AutoStartSyncAction { - if enabled { - if current_enabled { - AutoStartSyncAction::None - } else { - AutoStartSyncAction::Enable - } - } else if current_enabled { - AutoStartSyncAction::Disable - } else { - AutoStartSyncAction::None - } -} - #[cfg(any(target_os = "macos", test))] fn macos_app_bundle_path(exe_path: &Path) -> Option { let exe_str = exe_path.to_string_lossy(); @@ -259,27 +279,6 @@ mod tests { ); } - #[test] - fn test_autostart_sync_action_enabled_preference_with_disabled_entry_enables() { - assert_eq!( - autostart_sync_action(true, false), - AutoStartSyncAction::Enable, - ); - } - - #[test] - fn test_autostart_sync_action_current_enabled_entry_is_noop() { - assert_eq!(autostart_sync_action(true, true), AutoStartSyncAction::None); - } - - #[test] - fn test_autostart_sync_action_disabled_preference_disables_enabled_entry() { - assert_eq!( - autostart_sync_action(false, true), - AutoStartSyncAction::Disable, - ); - } - // Pure-string Scoop normalisation tests — no OS state, safe to run // in parallel with each other on any platform. diff --git a/src/config/autostart/linux.rs b/src/config/autostart/linux.rs new file mode 100644 index 00000000..8ae5e60f --- /dev/null +++ b/src/config/autostart/linux.rs @@ -0,0 +1,152 @@ +//! XDG autostart files owned by Ropy, with desktop-entry command encoding. + +use std::{ + fs, + io::Write as _, + path::{Path, PathBuf}, +}; + +use super::AutoStartError; + +pub(super) struct DesktopEntry { + path: PathBuf, + contents: String, +} + +impl DesktopEntry { + pub(super) fn new(config_dir: &Path, app_name: &str, app_path: &str) -> Self { + Self { + path: config_dir + .join("autostart") + .join(format!("{app_name}.desktop")), + contents: format!( + "[Desktop Entry]\nType=Application\nVersion=1.0\nName={app_name}\nExec={}\nStartupNotify=false\nTerminal=false\n", + quote_exec_path(app_path), + ), + } + } + + pub(super) fn sync_state(&self, enabled: bool) -> Result<(), AutoStartError> { + if !enabled { + return match fs::remove_file(&self.path) { + Ok(()) => Ok(()), + Err(error) if error.kind() == std::io::ErrorKind::NotFound => Ok(()), + Err(error) => Err(AutoStartError::Disable(error.to_string())), + }; + } + match fs::read_to_string(&self.path) { + Ok(current) if current == self.contents => return Ok(()), + Ok(_) => {} + Err(error) if error.kind() == std::io::ErrorKind::NotFound => {} + Err(error) => return Err(AutoStartError::StatusCheck(error.to_string())), + } + self.write() + .map_err(|error| AutoStartError::Enable(error.to_string())) + } + + fn write(&self) -> std::io::Result<()> { + // The path always has the autostart directory supplied by new(). + let directory = self + .path + .parent() + .ok_or_else(|| std::io::Error::other("missing autostart directory"))?; + fs::create_dir_all(directory)?; + let mut pending = tempfile::NamedTempFile::new_in(directory)?; + pending.write_all(self.contents.as_bytes())?; + pending.as_file().sync_all()?; + pending.persist(&self.path).map_err(|error| error.error)?; + Ok(()) + } +} + +fn quote_exec_path(path: &str) -> String { + // Desktop Entry string unescaping precedes Exec argument parsing. Encode + // both layers, including literal field-code markers; this is not shell input. + let mut encoded = String::from("\""); + for character in path.chars() { + match character { + '\\' => encoded.push_str(r"\\\\"), + '"' | '`' | '$' => { + encoded.push_str(r"\\"); + encoded.push(character); + } + '\n' => encoded.push_str(r"\n"), + '\t' => encoded.push_str(r"\t"), + '\r' => encoded.push_str(r"\r"), + '%' => encoded.push_str("%%"), + _ => encoded.push(character), + } + } + encoded.push('"'); + encoded +} + +#[cfg(test)] +mod tests { + use super::{DesktopEntry, quote_exec_path}; + + #[test] + fn test_sync_uses_selected_config_directory_and_preserves_other_profile() { + let directory = tempfile::tempdir().unwrap(); + let legacy = directory.path().join("default/autostart/Ropy.desktop"); + std::fs::create_dir_all(legacy.parent().unwrap()).unwrap(); + std::fs::write(&legacy, "another profile").unwrap(); + let config = directory.path().join("custom config"); + let entry = DesktopEntry::new(&config, "Ropy", "/opt/ropy"); + + entry.sync_state(true).unwrap(); + let path = config.join("autostart/Ropy.desktop"); + assert!( + std::fs::read_to_string(&path) + .unwrap() + .contains("Exec=\"/opt/ropy\"\n") + ); + assert_eq!(std::fs::read_to_string(&legacy).unwrap(), "another profile"); + entry.sync_state(false).unwrap(); + entry.sync_state(false).unwrap(); + assert!(!path.exists()); + assert_eq!(std::fs::read_to_string(&legacy).unwrap(), "another profile"); + } + + #[test] + fn test_sync_repairs_enabled_entry_after_executable_moves() { + let directory = tempfile::tempdir().unwrap(); + DesktopEntry::new(directory.path(), "Ropy", "/opt/old/ropy") + .sync_state(true) + .unwrap(); + let entry = DesktopEntry::new(directory.path(), "Ropy", "/opt/My Apps/ropy"); + entry.sync_state(true).unwrap(); + entry.sync_state(true).unwrap(); + let contents = + std::fs::read_to_string(directory.path().join("autostart/Ropy.desktop")).unwrap(); + assert!(contents.contains("Exec=\"/opt/My Apps/ropy\"\n")); + assert!(!contents.contains("/opt/old/ropy")); + } + + #[test] + fn test_sync_reports_unwritable_destination_without_replacing_it() { + let directory = tempfile::tempdir().unwrap(); + let path = directory.path().join("autostart/Ropy.desktop"); + std::fs::create_dir_all(&path).unwrap(); + std::fs::write(path.join("sentinel"), "keep").unwrap(); + let entry = DesktopEntry::new(directory.path(), "Ropy", "/opt/ropy"); + assert!(entry.sync_state(true).is_err()); + assert_eq!( + std::fs::read_to_string(path.join("sentinel")).unwrap(), + "keep" + ); + } + + #[rstest::rstest] + #[case("/opt/My Apps/ropy", "\"/opt/My Apps/ropy\"")] + #[case("/opt/%f/ropy", "\"/opt/%%f/ropy\"")] + #[case("/opt/雪/ropy", "\"/opt/雪/ropy\"")] + #[case( + "/opt/line\nbreak\tand\rreturn/ropy", + "\"/opt/line\\nbreak\\tand\\rreturn/ropy\"" + )] + #[case(r#"/opt/a"b\c$d`e/ropy"#, r#""/opt/a\\"b\\\\c\\$d\\`e/ropy""#)] + fn test_desktop_exec_preserves_literal_path(#[case] path: &str, #[case] expected: &str) { + assert_eq!(quote_exec_path(path), expected); + } +} diff --git a/src/gui/board/actions.rs b/src/gui/board/actions.rs index 88540d02..6782327f 100644 --- a/src/gui/board/actions.rs +++ b/src/gui/board/actions.rs @@ -13,10 +13,8 @@ use crate::{ gui::{ active_window, board::{ActivePanel, RopyBoard, search::next_content_filter}, - constants::default_window_size, hide_window, panel::settings, - reset_window_geometry_for_activation, }, }; @@ -310,7 +308,6 @@ impl RopyBoard { return; } - self.invalidate_copy_confirmation(); self.selected_index = 0; self.ui_state.hide_preview(); if self.active_panel == ActivePanel::Settings { @@ -325,10 +322,15 @@ impl RopyBoard { } self.cancel_shortcut_recording(cx); self.ui_state.clear_confirm = crate::gui::board::ClearConfirmState::Hidden; + self.activate_popup(window, cx); + } + + /// Finish activation after the caller has chosen its panel and reset policy. + pub(crate) fn activate_popup(&mut self, window: &mut Window, cx: &mut Context<'_, Self>) { + self.invalidate_copy_confirmation(); self.activated = true; window.focus(&self.focus_handle, cx); cx.notify(); - reset_window_geometry_for_activation(window, default_window_size()); active_window(window, cx); } diff --git a/src/gui/board/filtering.rs b/src/gui/board/filtering.rs index 75a4d5bd..4b9248b2 100644 --- a/src/gui/board/filtering.rs +++ b/src/gui/board/filtering.rs @@ -6,8 +6,8 @@ use crate::repository::{ClipboardRecord, ClipboardRepository}; #[derive(Debug, Clone, PartialEq, Eq)] pub(super) enum FilteredRecordsUpdate { None, - Reset { new_len: usize }, - Splice { old_len: usize, new_len: usize }, + Reset, + Splice, } #[derive(Debug, Clone, PartialEq, Eq)] @@ -27,21 +27,16 @@ pub(super) fn plan_filtered_records_sync( let list_update = if next_indices == previous_indices { FilteredRecordsUpdate::None } else if deleting_record { - FilteredRecordsUpdate::Splice { - old_len: previous_indices.len(), - new_len: next_indices.len(), - } + FilteredRecordsUpdate::Splice } else { - FilteredRecordsUpdate::Reset { - new_len: next_indices.len(), - } + FilteredRecordsUpdate::Reset }; - let selected_index = if next_indices.is_empty() { - 0 - } else { - selected_index.min(next_indices.len() - 1) - }; + // Backing indices identify records only within the current repository snapshot. + let selected_index = previous_indices + .get(selected_index) + .and_then(|selected| next_indices.iter().position(|index| index == selected)) + .unwrap_or_else(|| selected_index.min(next_indices.len().saturating_sub(1))); FilteredRecordsSyncPlan { indices: next_indices, @@ -99,13 +94,7 @@ mod tests { assert_eq!(plan.indices, vec![0, 2]); assert_eq!(plan.selected_index, 1); - assert_eq!( - plan.list_update, - FilteredRecordsUpdate::Splice { - old_len: 3, - new_len: 2, - } - ); + assert_eq!(plan.list_update, FilteredRecordsUpdate::Splice); assert!(plan.clear_deleting_record); } @@ -115,10 +104,7 @@ mod tests { assert_eq!(plan.indices, vec![1]); assert_eq!(plan.selected_index, 0); - assert_eq!( - plan.list_update, - FilteredRecordsUpdate::Reset { new_len: 1 } - ); + assert_eq!(plan.list_update, FilteredRecordsUpdate::Reset); assert!(!plan.clear_deleting_record); } diff --git a/src/gui/board/record_ops.rs b/src/gui/board/record_ops.rs index eb4323aa..29a6c9fe 100644 --- a/src/gui/board/record_ops.rs +++ b/src/gui/board/record_ops.rs @@ -142,7 +142,7 @@ impl RopyBoard { ); let next_visible_len = self.visible_list_len(plan.indices.len()); - let scroll_position = matches!(plan.list_update, FilteredRecordsUpdate::Splice { .. }) + let scroll_position = matches!(plan.list_update, FilteredRecordsUpdate::Splice) .then(|| self.list_state.logical_scroll_top()); self.filtered_record_indices = Arc::new(plan.indices); @@ -150,10 +150,10 @@ impl RopyBoard { match plan.list_update { FilteredRecordsUpdate::None => {} - FilteredRecordsUpdate::Reset { .. } => { + FilteredRecordsUpdate::Reset => { self.list_state.reset(next_visible_len); } - FilteredRecordsUpdate::Splice { .. } => { + FilteredRecordsUpdate::Splice => { self.list_state .splice(0..previous_visible_len, next_visible_len); if let Some(scroll_position) = scroll_position { @@ -234,21 +234,26 @@ impl RopyBoard { ) { // Selection, scrolling and filters are read at completion, so interaction // during I/O is not undone by the request's earlier presentation state. - let selected_id = self.filtered_record_id_at(self.selected_index); + let previous_selected_index = self.selected_index; + let selected_id = self.filtered_record_id_at(previous_selected_index); let scroll_position = self.list_state.logical_scroll_top(); let (records, favorite_ids) = snapshot.into_parts(); *write_or_recover(&self.records) = records; self.favorite_ids = Arc::new(favorite_ids); self.sync_filtered_records(cx); - if let Some(index) = selected_id.and_then(|id| { - let records = read_or_recover(&self.records); - self.filtered_record_indices - .iter() - .position(|&index| records.get(index).is_some_and(|record| record.id == id)) - }) { - self.selected_index = index; - } + // A new snapshot can reuse every backing index for a different record. + // Resolve by domain ID, and use the old visible position only as fallback. + self.selected_index = selected_id + .and_then(|id| { + let records = read_or_recover(&self.records); + self.filtered_record_indices + .iter() + .position(|&index| records.get(index).is_some_and(|record| record.id == id)) + }) + .unwrap_or_else(|| { + previous_selected_index.min(self.filtered_record_len().saturating_sub(1)) + }); // Record content and order may change while positional indices stay equal. self.list_state .reset(self.visible_list_len(self.filtered_record_len())); diff --git a/src/gui/board/ui_tests.rs b/src/gui/board/ui_tests.rs index 20c7a3a6..b7dba8e8 100644 --- a/src/gui/board/ui_tests.rs +++ b/src/gui/board/ui_tests.rs @@ -374,6 +374,56 @@ fn test_history_config_rebinds_quick_select_target(cx: &mut TestAppContext) { assert!(matches!(rx.try_recv().unwrap(), CopyRequest::Text { text, .. } if text == "record 1")); } +#[gpui_kit::test] +fn test_filter_content_preserves_selected_copy(cx: &mut TestAppContext) { + assert_filter_preserves_selected_copy(cx, "filter-text-btn"); +} + +#[gpui_kit::test] +fn test_filter_favorites_preserves_selected_copy(cx: &mut TestAppContext) { + assert_filter_preserves_selected_copy(cx, "filter-favorites-btn"); +} + +#[gpui_kit::test] +fn test_filter_query_preserves_selected_copy(cx: &mut TestAppContext) { + assert_filter_preserves_selected_copy(cx, "query"); +} + +fn assert_filter_preserves_selected_copy(cx: &mut TestAppContext, filter: &'static str) { + let mut excluded = record(3); + excluded.content = "other".into(); + if filter == "filter-text-btn" { + excluded.content_type = crate::repository::ContentType::Image; + } + let (handle, board, rx) = open_board(cx, vec![record(1), record(2), excluded]); + cx.update_window(handle, |_, window, cx| { + board.update(cx, |board, cx| { + board.favorite_ids = Arc::new([1, 2].into()); + window.focus(&board.focus_handle, cx); + }); + window.render_frame(cx); + window.press("down", cx); + assert_eq!(board.read(cx).filtered_record_id_at(1), Some(2)); + if filter == "query" { + window.press("/", cx); + window.input("record", cx); + } else { + window.click(filter, cx); + } + }) + .unwrap(); + cx.run_until_parked(); + cx.update_window(handle, |_, window, cx| { + assert_eq!(board.read(cx).filtered_record_len(), 2); + let view = board.read(cx); + assert_eq!(view.filtered_record_id_at(view.selected_index), Some(2)); + window.press("enter", cx); + }) + .unwrap(); + cx.run_until_parked(); + assert!(matches!(rx.try_recv().unwrap(), CopyRequest::Text { text, .. } if text == "record 2")); +} + fn record(id: u64) -> ClipboardRecord { // Keep newest-first ordering independent of the platform clock's resolution. let timestamp = 1_700_000_000 + i64::try_from(id).unwrap(); @@ -1463,6 +1513,49 @@ fn test_refresh_new_record_preserves_selected_identity(cx: &mut TestAppContext) }); } +#[gpui_kit::test] +fn test_refresh_removed_selection_does_not_reuse_backing_index(cx: &mut TestAppContext) { + let dir = tempfile::tempdir().unwrap(); + let repo = Arc::new( + ClipboardRepository::open(&dir.path().join("db.redb"), dir.path().join("images")).unwrap(), + ); + let fallback = repo.save_text("keep oldest".into()).unwrap(); + let selected = repo.save_text("keep selected".into()).unwrap(); + repo.save_text("keep newest".into()).unwrap(); + repo.save_text("excluded".into()).unwrap(); + let (handle, board, _) = open_board(cx, repo.get_display_records(100).unwrap()); + cx.update(|cx| cx.set_global(GlobalRepository::new(Some(repo.clone())))); + cx.update_window(handle, |_, window, cx| { + window.render_frame(cx); + window.press("/", cx); + window.input("keep", cx); + }) + .unwrap(); + cx.run_until_parked(); + cx.update_window(handle, |_, window, cx| { + window.press("escape", cx); + window.press("down", cx); + let view = board.read(cx); + assert_eq!( + view.filtered_record_id_at(view.selected_index), + Some(selected.id) + ); + }) + .unwrap(); + repo.delete(selected.id).unwrap(); + repo.save_text("excluded latest".into()).unwrap(); + cx.update(|cx| board.update(cx, |board, cx| board.refresh_records_from_repository(cx))); + cx.run_until_parked(); + cx.update(|cx| { + let view = board.read(cx); + assert_eq!(view.filtered_record_len(), 2); + assert_eq!( + view.filtered_record_id_at(view.selected_index), + Some(fallback.id) + ); + }); +} + #[gpui_kit::test] fn test_open_settings_panel_reveals_opacity_slider_on_next_frame(cx: &mut TestAppContext) { let (handle, board, _) = open_board(cx, vec![]); diff --git a/src/gui/tray.rs b/src/gui/tray.rs index a5b6c91d..3f33d3d5 100644 --- a/src/gui/tray.rs +++ b/src/gui/tray.rs @@ -178,52 +178,35 @@ pub(crate) fn start_tray_handler_inner(tx: async_channel::Sender) -> } pub(crate) fn send_open_settings(cx: &mut App) { + open_panel(super::board::ActivePanel::Settings, cx); +} + +pub(crate) fn send_open_about(cx: &mut App) { + open_panel(super::board::ActivePanel::About, cx); +} + +fn open_panel(panel: super::board::ActivePanel, cx: &mut App) { let Some(window_handle) = super::window::ensure_window(cx) else { return; }; - window_handle - .update(cx, |root, window: &mut gpui_kit::Window, cx| { - if let Ok(board) = root - .view() - .clone() - .downcast::() - { - board.update(cx, |board, cx| { - board.active_panel = crate::gui::board::ActivePanel::Settings; + let _ = window_handle.update(cx, |root, window, cx| { + if let Ok(board) = root.view().clone().downcast::() { + board.update(cx, |board, cx| { + board.cancel_shortcut_recording(cx); + if board.settings_editor.hotkey.recording { + board.cancel_hotkey_recording(window, cx); + } + board.active_panel = panel; + if panel == super::board::ActivePanel::Settings { board .settings_editor .panel_state .window_opacity_slider_visible = true; - board.activated = true; - window.focus(&board.focus_handle, cx); - cx.notify(); - }); - } - super::active_window(window, cx); - }) - .ok(); -} - -pub(crate) fn send_open_about(cx: &mut App) { - let Some(window_handle) = super::window::ensure_window(cx) else { - return; - }; - window_handle - .update(cx, |root, window: &mut gpui_kit::Window, cx| { - if let Ok(board) = root - .view() - .clone() - .downcast::() - { - board.update(cx, |board, cx| { - board.active_panel = crate::gui::board::ActivePanel::About; - board.activated = true; - cx.notify(); - }); - } - super::active_window(window, cx); - }) - .ok(); + } + board.activate_popup(window, cx); + }); + } + }); } fn spawn_tray_menu_event_forwarder( diff --git a/src/gui/window.rs b/src/gui/window.rs index c709f7a8..3eaf95e9 100644 --- a/src/gui/window.rs +++ b/src/gui/window.rs @@ -175,14 +175,6 @@ pub(crate) fn show_with_token(token: Option<&str>, cx: &mut App) { tracing::warn!(%error, "failed to apply Wayland activation token"); } }); - #[cfg(target_os = "linux")] - if cx.has_global::() { - cx.background_executor() - .spawn(async { - crate::linux::hyprland::focus_popup(); - }) - .detach(); - } } } @@ -277,9 +269,8 @@ pub(crate) fn hide_window(window: &mut Window, cx: &Context<'_, T>, pinned: b /// the first time the user triggers the hotkey or the tray "Show" entry — /// on macOS this maps to `[NSWindow makeKeyAndOrderFront]`, which both /// orders the window front and gives it key status. -#[expect(clippy::needless_pass_by_ref_mut)] -#[cfg_attr(target_os = "linux", expect(unused_variables))] pub(crate) fn active_window(window: &mut Window, cx: &Context<'_, T>) { + super::reset_window_geometry_for_activation(window, super::constants::default_window_size()); window.activate_window(); cfg_select! { @@ -308,6 +299,14 @@ pub(crate) fn active_window(window: &mut Window, cx: &Context<'_, T>) { } _ => {} } + #[cfg(target_os = "linux")] + if cx.has_global::() { + cx.background_executor() + .spawn(async { + crate::linux::hyprland::focus_popup(); + }) + .detach(); + } } /// Toggle the always-on-top Z-order on platforms that expose it