From b8376bd631a72f7484c24663699a7d4c813f600e Mon Sep 17 00:00:00 2001 From: srdusr <99972264+srdusr@users.noreply.github.com> Date: Sun, 17 Aug 2025 01:54:00 +0200 Subject: Fix desktop icons v1 regressions: bar overlap, wrong order; add proper menus Live testing found v1 genuinely broken, not just rough: 1. Icons weren't rendering reliably at all - ensure_desktop_icons only ever computed the grid's origin once, on whichever render pass happened to be first. AGS's own top bar registers its exclusive zone after that first pass, so origin got permanently baked in at the pre-bar geometry. Confirmed live via a temporary diagnostic log. Fixed by re-deriving origin from the primary monitor's current geometry on every call instead of just the first. 2. Fixed icons (Home/Computer/Trash) always sorted before real files -- confirmed wrong via direct question. The whole list now sorts alphabetically by label, case-insensitive, fixed icons included. 3. "Set as Wallpaper" was the wrong feature: removed entirely (DesktopMenuAction::SetWallpaper, general.wallpaper_command, is_image_path). The user wants that handled by their real file manager once opened, not reimplemented here. Also adds real menu functionality per "where are all the options": Rename (inline text edit, new CompState::renaming_icon field and keyboard redirect mirroring NativeLock::password's existing precedent), Delete (moves to ~/.local/share/Trash per the freedesktop.org spec, new trash.rs module, same-filesystem case, no confirmation - this is the reversible move-to-trash, not a permanent delete), Empty Trash on the Trash icon, and Open Terminal Here / Open in File Manager on the bare-desktop menu (new general.terminal config key). 133 wayland-crate tests (up from 106), full workspace build and clippy clean. --- crates/wayland/src/desktop_menu.rs | 110 ++++++++++++++++++++++--------------- 1 file changed, 65 insertions(+), 45 deletions(-) (limited to 'crates/wayland/src/desktop_menu.rs') diff --git a/crates/wayland/src/desktop_menu.rs b/crates/wayland/src/desktop_menu.rs index 810e639..bd0bf34 100644 --- a/crates/wayland/src/desktop_menu.rs +++ b/crates/wayland/src/desktop_menu.rs @@ -9,12 +9,24 @@ use crate::desktop_icons::{DesktopIcon, IconKind}; pub(crate) enum DesktopMenuAction { /// Open the icon with this id - same action a double-click runs. OpenIcon(String), - /// Shell out to `general.wallpaper_command` with this icon's path -- - /// only ever offered for an image-file icon, and only when that - /// config key is actually set (see `WindowManager::wallpaper_command`'s - /// own doc comment). - SetWallpaper(String), + /// Enter inline rename mode for this real file/folder icon - see + /// `CompState::renaming_icon`'s own doc comment. + Rename(String), + /// Move this real file/folder into `~/.local/share/Trash` - no + /// confirmation, same as every mainstream file manager: this is the + /// reversible move-to-trash, not a permanent delete. + Delete(String), + /// Empties `~/.local/share/Trash` entirely - same no-confirmation + /// convention as `Delete`. + EmptyTrash, NewFolder, + /// Spawns a terminal with `~/Desktop` as its working directory -- + /// `general.terminal`, or a common-binary fallback list if unset. + OpenTerminalHere, + /// Opens `~/Desktop` itself in `general.file_manager`/`xdg-open` - the + /// concrete path to a real file manager's own richer menu (cut/copy/ + /// paste, properties, ...), deliberately not reimplemented here. + OpenInFileManager, Refresh, } @@ -28,33 +40,36 @@ pub(crate) struct DesktopMenu { const MENU_WIDTH: u32 = 170; const ROW_HEIGHT: u32 = 28; -/// Extensions `render_desktop_icon`'s `IconKind::File` icons treat as an -/// image for "Set as Wallpaper" purposes - not a real mimetype sniff (no -/// such capability exists anywhere in this workspace, see `desktop_icons. -/// rs`'s own doc comment on why icon art itself is hand-drawn, not -/// decoded), just the common raster formats a wallpaper tool actually -/// accepts. -const IMAGE_EXTENSIONS: &[&str] = &["png", "jpg", "jpeg", "webp", "bmp", "gif"]; - -fn is_image_path(path: &std::path::Path) -> bool { - path.extension().and_then(|e| e.to_str()).map(|e| IMAGE_EXTENSIONS.iter().any(|ext| e.eq_ignore_ascii_case(ext))).unwrap_or(false) -} - impl DesktopMenu { - /// Right-click on `icon` itself: "Open" always, plus "Set as Wallpaper" - /// when `icon` is an image file and `wallpaper_command` is non-empty. - pub(crate) fn open_for_icon(icon: &DesktopIcon, pos: (i32, i32), wallpaper_command: &str) -> Self { - let mut items = vec![("Open", DesktopMenuAction::OpenIcon(icon.id.clone()))]; - if icon.kind == IconKind::File && !wallpaper_command.is_empty() && is_image_path(&icon.target) { - items.push(("Set as Wallpaper", DesktopMenuAction::SetWallpaper(icon.id.clone()))); - } + /// Right-click on `icon` itself - the action set depends on what kind + /// of icon it is, not one fixed list: a real file/folder gets Open/ + /// Rename/Delete (real filesystem operations); Home/Computer (fixed + /// shortcuts to somewhere, not real files of their own) get Open only, + /// since renaming or deleting the shortcut itself isn't a meaningful + /// action; Trash gets Open/Empty Trash instead of Rename/Delete, since + /// "delete the trash" and "rename the trash" aren't real trash + /// operations the way "empty it" is. + pub(crate) fn open_for_icon(icon: &DesktopIcon, pos: (i32, i32)) -> Self { + let items = match icon.kind { + IconKind::Trash => vec![("Open", DesktopMenuAction::OpenIcon(icon.id.clone())), ("Empty Trash", DesktopMenuAction::EmptyTrash)], + IconKind::Home | IconKind::Computer => vec![("Open", DesktopMenuAction::OpenIcon(icon.id.clone()))], + IconKind::Folder | IconKind::File => vec![ + ("Open", DesktopMenuAction::OpenIcon(icon.id.clone())), + ("Rename", DesktopMenuAction::Rename(icon.id.clone())), + ("Delete", DesktopMenuAction::Delete(icon.id.clone())), + ], + }; Self { pos, width: MENU_WIDTH, row_height: ROW_HEIGHT, items } } - /// Right-click on bare desktop (no icon under the pointer): "New - /// Folder" and "Refresh". + /// Right-click on bare desktop (no icon under the pointer). pub(crate) fn open_for_desktop(pos: (i32, i32)) -> Self { - let items = vec![("New Folder", DesktopMenuAction::NewFolder), ("Refresh", DesktopMenuAction::Refresh)]; + let items = vec![ + ("New Folder", DesktopMenuAction::NewFolder), + ("Open Terminal Here", DesktopMenuAction::OpenTerminalHere), + ("Open in File Manager", DesktopMenuAction::OpenInFileManager), + ("Refresh", DesktopMenuAction::Refresh), + ]; Self { pos, width: MENU_WIDTH, row_height: ROW_HEIGHT, items } } @@ -80,40 +95,45 @@ mod tests { use super::*; use std::path::PathBuf; - fn icon(kind: IconKind, target: &str) -> DesktopIcon { - DesktopIcon { id: "x".into(), label: "x".into(), kind, target: PathBuf::from(target), cell: (0, 0), selected: false } + fn icon(kind: IconKind) -> DesktopIcon { + DesktopIcon { id: "x".into(), label: "x".into(), kind, target: PathBuf::new(), cell: (0, 0), selected: false } } #[test] - fn image_file_with_a_configured_command_gets_the_wallpaper_row() { - let menu = DesktopMenu::open_for_icon(&icon(IconKind::File, "pic.png"), (0, 0), "swww img"); - assert_eq!(menu.items.len(), 2); - assert_eq!(menu.items[1].0, "Set as Wallpaper"); + fn a_real_file_gets_open_rename_and_delete() { + let menu = DesktopMenu::open_for_icon(&icon(IconKind::File), (0, 0)); + let labels: Vec<&str> = menu.items.iter().map(|(l, _)| *l).collect(); + assert_eq!(labels, vec!["Open", "Rename", "Delete"]); } #[test] - fn image_file_with_no_configured_command_has_no_wallpaper_row() { - let menu = DesktopMenu::open_for_icon(&icon(IconKind::File, "pic.png"), (0, 0), ""); - assert_eq!(menu.items.len(), 1, "Open only"); + fn a_real_folder_gets_open_rename_and_delete_too() { + let menu = DesktopMenu::open_for_icon(&icon(IconKind::Folder), (0, 0)); + let labels: Vec<&str> = menu.items.iter().map(|(l, _)| *l).collect(); + assert_eq!(labels, vec!["Open", "Rename", "Delete"]); } #[test] - fn non_image_file_has_no_wallpaper_row_even_with_a_command_configured() { - let menu = DesktopMenu::open_for_icon(&icon(IconKind::File, "notes.txt"), (0, 0), "swww img"); - assert_eq!(menu.items.len(), 1, "Open only"); + fn home_and_computer_get_open_only() { + for kind in [IconKind::Home, IconKind::Computer] { + let menu = DesktopMenu::open_for_icon(&icon(kind), (0, 0)); + let labels: Vec<&str> = menu.items.iter().map(|(l, _)| *l).collect(); + assert_eq!(labels, vec!["Open"], "shortcuts aren't real files - no rename/delete"); + } } #[test] - fn a_folder_never_gets_the_wallpaper_row() { - let menu = DesktopMenu::open_for_icon(&icon(IconKind::Folder, "pic.png"), (0, 0), "swww img"); - assert_eq!(menu.items.len(), 1, "a directory named like an image is still not a file"); + fn trash_gets_open_and_empty_trash_not_rename_or_delete() { + let menu = DesktopMenu::open_for_icon(&icon(IconKind::Trash), (0, 0)); + let labels: Vec<&str> = menu.items.iter().map(|(l, _)| *l).collect(); + assert_eq!(labels, vec!["Open", "Empty Trash"]); } #[test] - fn desktop_menu_offers_new_folder_and_refresh() { + fn desktop_menu_offers_the_full_set() { let menu = DesktopMenu::open_for_desktop((10, 10)); - assert_eq!(menu.items[0].0, "New Folder"); - assert_eq!(menu.items[1].0, "Refresh"); + let labels: Vec<&str> = menu.items.iter().map(|(l, _)| *l).collect(); + assert_eq!(labels, vec!["New Folder", "Open Terminal Here", "Open in File Manager", "Refresh"]); } #[test] -- cgit v1.2.3