From 2ad8e30c8d0e3a4b5187aec1e1a2ec27061ae4ce Mon Sep 17 00:00:00 2001 From: Victor Garcia Date: Thu, 7 May 2026 01:22:42 +0200 Subject: [PATCH] fix(FIX-011, FIX-014): drop --temp mode and command palette; rebind Ctrl+Shift+{N,P} MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit After P-018 (AD-016) the unpinned-default lifecycle (active/ → trash/ on clean close, recoverable for 30s via Undo or --restore-session) already covers what --temp was for. The remaining gap (--temp wrote nothing on disk, unpinned writes a trashable file) was too small to justify a separate code path, CLI flag, menu item, action handler, accelerator, and ~200 LoC of legacy local-only fallback code. FIX-014 bundled mid-QA per dogfood directive: the Ctrl+Shift+P command palette duplicated F1's adw::ShortcutsWindow with rendering issues, so the 540-LoC custom popover plus its registry were deleted and Ctrl+Shift+P rebound onto the existing win.show-shortcuts action. Ctrl+Shift+N rebound to win.new-window so muscle memory is preserved and the unbound-accelerator → kitty-keyboard escape leak (a fix-cycle 1 regression Vick caught at T2) is fixed. ADs: AD-011 (daemon mode is the only mode) preserved on every code path now that --temp is gone. AD-016 (pinned-aware close + daemon does not survive) preserved. AD-001 (one daemon per window) preserved. No AD changes. Perf: pty_to_subscriber_idle 17.4 µs (28× under SPEC's 500 µs ceiling), daemon_cold_start 18.26 ms (11× under 200 ms ceiling). Criterion change% flagged regressions vs P-018's 2.87 ms cold-start measurement which was a host-quiet outlier; absolute numbers are at-noise vs the FIX-005A baseline (15.86 ms) and well within SPEC. Hot path untouched. Net: 4 files, +88 / −747 / −659 LoC. CHORE-FIX-011-cleanup follow-up deletes the now-unreachable duplicate_workspace_temp_fallback family and refactors daemon_client: Option> → Arc<>. FEAT-001 filed for a real fuzzy command palette to revisit when the action surface outgrows F1. --- crates/forgetty-gtk/src/app.rs | 812 +++-------------------- crates/forgetty-gtk/src/settings_view.rs | 16 +- src/cli.rs | 5 - src/main.rs | 2 - 4 files changed, 88 insertions(+), 747 deletions(-) diff --git a/crates/forgetty-gtk/src/app.rs b/crates/forgetty-gtk/src/app.rs index e440ceb..9aa6c7b 100644 --- a/crates/forgetty-gtk/src/app.rs +++ b/crates/forgetty-gtk/src/app.rs @@ -217,10 +217,6 @@ pub struct LaunchOptions { /// Restore all saved sessions (open one window per session file). pub restore_all: bool, - - /// Open an ephemeral session that is never persisted. No daemon is spawned - /// and no session file is written on close. - pub temp: bool, } /// Derive the daemon socket path for a given session UUID. @@ -232,16 +228,11 @@ fn socket_path_for(session_id: uuid::Uuid) -> PathBuf { } } -/// Try to connect to the running daemon; if not running, spawn it and retry. -/// -/// Returns `Some(DaemonClient)` on success, `None` if daemon is unavailable -/// (in which case GTK falls back to self-contained PTY mode). /// Connect to (or spawn) the daemon for this session. /// /// Per AD-011 the daemon is a hard dependency — there is no local-PTY fallback. /// If the daemon cannot be started or connected to, the process exits with -/// status 1 after logging the cause. The caller is expected to handle the -/// `--temp` bypass *before* calling this function. +/// status 1 after logging the cause. fn ensure_daemon(session_id: uuid::Uuid) -> Arc { let socket_path = socket_path_for(session_id); @@ -323,8 +314,7 @@ pub fn run(config: Config, launch: LaunchOptions) -> Result<(), Box Result<(), Box tracing::info!( - "p018 orphan recovery: promoted pinned {uuid} → sessions/" - ), - Err(e) => { - tracing::warn!("p018 orphan recovery: failed to promote {uuid}: {e}") - } + if let Err(e) = forgetty_workspace::run_migration_p018() { + tracing::warn!("p018 migration: {e} (continuing — not fatal)"); + } + let orphans = forgetty_workspace::recover_orphans_in_active(); + if !orphans.is_empty() { + tracing::info!("p018 orphan recovery: {} file(s) found in active/", orphans.len()); + for (uuid, is_pinned) in orphans { + if is_pinned { + match forgetty_workspace::move_active_to_sessions(uuid) { + Ok(()) => { + tracing::info!("p018 orphan recovery: promoted pinned {uuid} → sessions/") } - } else { - match forgetty_workspace::delete_active_for(uuid) { - Ok(()) => { - tracing::info!("p018 orphan recovery: deleted unpinned orphan {uuid}") - } - Err(e) => { - tracing::warn!("p018 orphan recovery: failed to delete {uuid}: {e}") - } + Err(e) => { + tracing::warn!("p018 orphan recovery: failed to promote {uuid}: {e}") + } + } + } else { + match forgetty_workspace::delete_active_for(uuid) { + Ok(()) => { + tracing::info!("p018 orphan recovery: deleted unpinned orphan {uuid}") + } + Err(e) => { + tracing::warn!("p018 orphan recovery: failed to delete {uuid}: {e}") } } } @@ -370,17 +358,12 @@ pub fn run(config: Config, launch: LaunchOptions) -> Result<(), Box> = if launch.temp { - info!("GTK running in ephemeral mode: no daemon, no session file will be written"); - None - } else { - let dc = ensure_daemon(session_id); - info!("GTK running in daemon-client mode: sessions survive window close"); - Some(dc) - }; + // Per AD-011 the daemon is a hard dependency; `ensure_daemon` exits the + // process on failure. The `Option` wrapper is retained for now to keep the + // existing `if let Some(dc) = daemon_client` patterns compiling — see + // CHORE-FIX-011-cleanup for the follow-up refactor to `Arc`. + let daemon_client: Option> = Some(ensure_daemon(session_id)); + info!("GTK running in daemon-client mode: sessions survive window close"); let app = adw::Application::builder() .application_id(app_id) @@ -745,12 +728,8 @@ fn build_ui( // CLI overrides skip both session restore AND session save so a one-off // launch (e.g. `forgetty --working-directory /tmp`) never overwrites the // user's real saved session. - // `--temp` is also treated as a CLI override: ephemeral sessions neither - // restore from nor write to a session file. - let has_cli_override = launch.working_directory.is_some() - || launch.command.is_some() - || launch.no_restore - || launch.temp; + let has_cli_override = + launch.working_directory.is_some() || launch.command.is_some() || launch.no_restore; let skip_session_save = Rc::new(Cell::new(has_cli_override)); // --- Widget hierarchy --- @@ -784,11 +763,9 @@ fn build_ui( // Section 2 -- Window & Tab management let window_tab_section = gio::Menu::new(); - window_tab_section.append(Some("New Window"), Some("win.new-window")); - let new_temp_window_item = - gio::MenuItem::new(Some("New Temporary Window"), Some("win.new-temp-window")); - new_temp_window_item.set_attribute_value("accel", Some(&"n".to_variant())); - window_tab_section.append_item(&new_temp_window_item); + let new_window_item = gio::MenuItem::new(Some("New Window"), Some("win.new-window")); + new_window_item.set_attribute_value("accel", Some(&"n".to_variant())); + window_tab_section.append_item(&new_window_item); window_tab_section.append(Some("Close Window"), Some("win.close-window")); window_tab_section .append(Some("Close Window Permanently"), Some("win.close-window-permanently")); @@ -1015,10 +992,6 @@ fn build_ui( preferences::build_appearance_sidebar(&shared_config, &tab_states, &window); main_area.append(&appearance_revealer); - // --- Command palette overlay (built after workspace_manager is ready) --- - let command_palette = build_command_palette(&window, &workspace_manager); - main_overlay.add_overlay(&command_palette); - // --- Tab bar right-click (Capture phase, claimed) --- // // libadwaita 1.5 claims button-3 events on AdwTabButton WITHOUT emitting @@ -1117,43 +1090,6 @@ fn build_ui( ); terminal_row.prepend(&workspace_sidebar_revealer); - // Click-outside-to-close: a GestureClick on the overlay detects clicks - // that land outside the palette card and closes it. - { - let palette_ref = command_palette.clone(); - let wm_click = Rc::clone(&workspace_manager); - let click_gesture = gtk4::GestureClick::new(); - click_gesture.set_propagation_phase(gtk4::PropagationPhase::Capture); - click_gesture.connect_pressed(move |gesture, _n_press, x, y| { - if !palette_ref.is_visible() { - gesture.set_state(gtk4::EventSequenceState::None); - return; - } - // Check if the click is inside the palette widget bounds - let Some(parent) = palette_ref.parent() else { - gesture.set_state(gtk4::EventSequenceState::None); - return; - }; - let Some(bounds) = palette_ref.compute_bounds(&parent) else { - gesture.set_state(gtk4::EventSequenceState::None); - return; - }; - let inside = x >= bounds.x() as f64 - && x <= (bounds.x() + bounds.width()) as f64 - && y >= bounds.y() as f64 - && y <= (bounds.y() + bounds.height()) as f64; - - if !inside { - let ft = active_focus_tracker(&wm_click); - close_command_palette(&palette_ref, &ft); - gesture.set_state(gtk4::EventSequenceState::Claimed); - } else { - gesture.set_state(gtk4::EventSequenceState::None); - } - }); - main_overlay.add_controller(click_gesture); - } - // --- Tab close handling --- // When a tab's close button is clicked: // - In local mode: kill all PTYs in the tab's widget tree. @@ -1172,7 +1108,8 @@ fn build_ui( // Daemon mode: send close_tab RPC for each pane in the subtree. daemon_close_panes_in_subtree(&container, &states_close, dc); } else { - // --temp mode: no daemon, no PTY — just drop the registry entries. + // Legacy fallback (unreachable post-FIX-011 since daemon_client + // is always Some). Retained until CHORE-FIX-011-cleanup. remove_panes_in_subtree(&container, &states_close); } @@ -1673,7 +1610,7 @@ fn build_ui( app.set_accels_for_action( "win.show-shortcuts", - &["F1", "question", "slash"], + &["F1", "question", "slash", "p"], ); // --- Show about dialog action --- @@ -1697,7 +1634,7 @@ fn build_ui( // --- New Window action (menu only, no accelerator) --- // Spawns with a fresh UUID so the restore-all logic is bypassed (no session // file exists for that UUID), while the new window still saves its session - // on close (persistent, not ephemeral). + // on close. { let action = gio::SimpleAction::new("new-window", None); action.connect_activate(move |_action, _param| { @@ -1725,26 +1662,11 @@ fn build_ui( window.add_action(&action); } - // --- New Temporary Window action (Ctrl+Shift+N) --- - // Spawns `forgetty --temp` so the new window is ephemeral: no session file - // is written on close and no daemon is started. - { - let action = gio::SimpleAction::new("new-temp-window", None); - action.connect_activate(move |_action, _param| { - if let Ok(exe) = std::env::current_exe() { - if let Err(e) = std::process::Command::new(exe).arg("--temp").spawn() { - tracing::warn!("Failed to spawn temporary window: {e}"); - } - } - }); - window.add_action(&action); - } - // --- Close Window Permanently action (menu only, no accelerator) --- // Kills the daemon first (so it can't auto-save), then deletes the // session file from BOTH the legacy `sessions/{uuid}.json` location // (pinned sessions) and the P-018 `active/{uuid}.json` location (the - // live bucket). In --temp mode: nothing to delete. + // live bucket). // // Order matters: daemon shutdown FIRST so the debounce/periodic save // tasks cannot race with the file delete and re-create the file. @@ -1752,7 +1674,6 @@ fn build_ui( let win_perm_close = window.clone(); let dc_perm_close = daemon_client.clone(); let skip_save_perm = Rc::clone(&skip_session_save); - let is_temp = launch.temp; let action = gio::SimpleAction::new("close-window-permanently", None); action.connect_activate(move |_action, _param| { // Prevent the window close handler from re-writing the session file. @@ -1761,20 +1682,18 @@ fn build_ui( if let Some(ref dc) = dc_perm_close { dc.shutdown(); } - if !is_temp { - // Pinned session file (top-level). - let path = forgetty_workspace::session_path_for(session_id); - if path.exists() { - if let Err(e) = std::fs::remove_file(&path) { - tracing::warn!("Failed to delete session file on permanent close: {e}"); - } - } - // P-018: also remove the live `active/` file so it doesn't - // appear as a crash orphan on next launch. - if let Err(e) = forgetty_workspace::delete_active_for(session_id) { - tracing::warn!("Failed to delete active session file on permanent close: {e}"); + // Pinned session file (top-level). + let path = forgetty_workspace::session_path_for(session_id); + if path.exists() { + if let Err(e) = std::fs::remove_file(&path) { + tracing::warn!("Failed to delete session file on permanent close: {e}"); } } + // P-018: also remove the live `active/` file so it doesn't + // appear as a crash orphan on next launch. + if let Err(e) = forgetty_workspace::delete_active_for(session_id) { + tracing::warn!("Failed to delete active session file on permanent close: {e}"); + } win_perm_close.close(); }); window.add_action(&action); @@ -2000,7 +1919,6 @@ fn build_ui( // P-018 / AD-016: Ctrl+Shift+Q uses the same pinned-aware close as the // X-button. Pre-P-018 (AD-012) the daemon survived this path; the AD-016 // amendment binds daemon survival to the single explicit pin action. - // --temp mode (dc is None): nothing to save; the session is ephemeral. { let app_quit = app.clone(); let wm_quit = Rc::clone(&workspace_manager); @@ -2255,21 +2173,7 @@ fn build_ui( app.set_accels_for_action("win.toggle-workspace-sidebar", &["b"]); - // --- Command Palette action (Ctrl+Shift+P) --- - { - let palette_ref = command_palette.clone(); - let wm_palette = Rc::clone(&workspace_manager); - let win_ref = window.clone(); - let action = gio::SimpleAction::new("command-palette", None); - action.connect_activate(move |_action, _param| { - let ft = active_focus_tracker(&wm_palette); - toggle_command_palette(&palette_ref, &ft, &win_ref); - }); - window.add_action(&action); - } - - app.set_accels_for_action("win.new-temp-window", &["n"]); - app.set_accels_for_action("win.command-palette", &["p"]); + app.set_accels_for_action("win.new-window", &["n"]); // --- Apply user keybinding overrides (AC-20) --- // Must be called AFTER all default set_accels_for_action calls so user @@ -2397,8 +2301,6 @@ fn build_ui( // If the closing window has any sibling forgetty windows still open and // the session is unpinned, an "Undo Close" desktop notification is // emitted so the user can recover within 30 s. - // - // --temp mode (dc is None): nothing to persist — just proceed with the close. { let wm_close = Rc::clone(&workspace_manager); let dc_window_close = daemon_client.clone(); @@ -2447,7 +2349,6 @@ fn build_ui( // Pre-P-018 (V2-005 / AD-012) signals dropped the connection but kept // the daemon alive; AD-016 amends that — daemon survival is bound to // the explicit pin, not to client lifecycle. - // --temp mode (dc is None): nothing to persist — just quit. { let signals: &[(i32, &str)] = &[(SIGTERM, "SIGTERM"), (SIGHUP, "SIGHUP"), (SIGINT, "SIGINT")]; @@ -2721,8 +2622,7 @@ fn send_undo_close_notification(_session_id: uuid::Uuid) { /// Read the CWD of a terminal pane. /// /// Returns the `daemon_cwd` captured at connect time from the daemon's -/// `PaneInfo`. Returns `None` for `--temp` panes or if the daemon did not -/// provide a CWD. +/// `PaneInfo`. Returns `None` if the daemon did not provide a CWD. fn read_pane_cwd(state_rc: &Rc>) -> Option { let s = state_rc.try_borrow().ok()?; s.daemon_cwd.clone() @@ -2979,10 +2879,9 @@ fn handle_layout_event( // // The widget-build is gated on a daemon_client reference because // we need `subscribe_output` to obtain the byte stream channel. - // In `--temp` mode there is no daemon path that fires this event - // (auto-spawn runs in the per-temp-window daemon), so the gate is - // tight in practice. If the gate fails we degrade to the original - // log-and-continue. + // Post-FIX-011 daemon_client is always Some on the live path; the + // gate is retained for defensiveness until CHORE-FIX-011-cleanup. + // If the gate fails we degrade to the original log-and-continue. if let Some(dc) = daemon_client { build_auto_spawned_tab_widget( workspace_manager, @@ -3430,7 +3329,8 @@ fn build_auto_spawned_tab_widget( } /// Close every pane in the workspace: send daemon close RPCs (or clear the registry -/// in `--temp` mode). +/// when no daemon client is available — legacy fallback retained until +/// CHORE-FIX-011-cleanup). fn close_workspace_panes(tab_states: &TabStateMap, daemon_client: Option<&DaemonClient>) { let pane_names: Vec = tab_states.try_borrow().map(|states| states.keys().cloned().collect()).unwrap_or_default(); @@ -5524,8 +5424,9 @@ fn daemon_close_pane( /// Drop every leaf pane's `TerminalState` from the registry without any daemon RPC. /// -/// Used exclusively by `--temp` mode, where there is no daemon to notify and no -/// local PTY to kill (the daemon panel owns the PTY in every non-`--temp` path). +/// Legacy fallback for the no-daemon-client path. Post-FIX-011 daemon_client is +/// always Some, making this function unreachable on the live path; deletion is +/// deferred to CHORE-FIX-011-cleanup. fn remove_panes_in_subtree(widget: >k4::Widget, tab_states: &TabStateMap) { let leaves = collect_leaf_drawing_areas(widget); let mut states = tab_states.borrow_mut(); @@ -6707,551 +6608,6 @@ fn open_config_file() { } } -// --------------------------------------------------------------------------- -// Command Palette -// --------------------------------------------------------------------------- - -/// A single entry in the command palette registry. -struct CommandEntry { - display_name: &'static str, - action_name: &'static str, - shortcut_label: &'static str, -} - -/// The static list of commands shown in the command palette. -/// -/// Order matches the hamburger menu grouping for discoverability. -/// Commands with parameters (e.g., open-url) and disabled placeholders -/// (terminal-inspector) are excluded. -fn command_registry() -> &'static [CommandEntry] { - static COMMANDS: &[CommandEntry] = &[ - CommandEntry { display_name: "Copy", action_name: "win.copy", shortcut_label: "Ctrl+C" }, - CommandEntry { display_name: "Paste", action_name: "win.paste", shortcut_label: "Ctrl+V" }, - CommandEntry { - display_name: "New Window", - action_name: "win.new-window", - shortcut_label: "", - }, - CommandEntry { - display_name: "New Temporary Window", - action_name: "win.new-temp-window", - shortcut_label: "Ctrl+Shift+N", - }, - CommandEntry { - display_name: "Close Window", - action_name: "win.close-window", - shortcut_label: "", - }, - CommandEntry { - display_name: "Close Window Permanently", - action_name: "win.close-window-permanently", - shortcut_label: "", - }, - CommandEntry { - display_name: "New Tab", - action_name: "win.new-tab", - shortcut_label: "Ctrl+Shift+T", - }, - CommandEntry { - display_name: "Close Tab", - action_name: "win.close-tab", - shortcut_label: "", - }, - CommandEntry { - display_name: "Close Pane", - action_name: "win.close-pane", - shortcut_label: "Ctrl+Shift+W", - }, - CommandEntry { - display_name: "Change Tab Title", - action_name: "win.change-tab-title", - shortcut_label: "", - }, - CommandEntry { - display_name: "New Workspace", - action_name: "win.new-workspace", - shortcut_label: "Ctrl+Alt+N", - }, - CommandEntry { - display_name: "Rename Workspace", - action_name: "win.rename-workspace", - shortcut_label: "", - }, - CommandEntry { - display_name: "Delete Workspace", - action_name: "win.delete-workspace", - shortcut_label: "", - }, - CommandEntry { - display_name: "Toggle Workspace Sidebar", - action_name: "win.toggle-workspace-sidebar", - shortcut_label: "Ctrl+Alt+B", - }, - CommandEntry { - display_name: "Previous Workspace", - action_name: "win.prev-workspace", - shortcut_label: "Ctrl+Alt+PgUp", - }, - CommandEntry { - display_name: "Next Workspace", - action_name: "win.next-workspace", - shortcut_label: "Ctrl+Alt+PgDn", - }, - CommandEntry { display_name: "Split Up", action_name: "win.split-up", shortcut_label: "" }, - CommandEntry { - display_name: "Split Down", - action_name: "win.split-down", - shortcut_label: "Alt+Shift+\u{2212}", - }, - CommandEntry { - display_name: "Split Left", - action_name: "win.split-left", - shortcut_label: "", - }, - CommandEntry { - display_name: "Split Right", - action_name: "win.split-right", - shortcut_label: "Alt+Shift+=", - }, - CommandEntry { - display_name: "Focus Pane Left", - action_name: "win.focus-pane-left", - shortcut_label: "Alt+Left", - }, - CommandEntry { - display_name: "Focus Pane Right", - action_name: "win.focus-pane-right", - shortcut_label: "Alt+Right", - }, - CommandEntry { - display_name: "Focus Pane Up", - action_name: "win.focus-pane-up", - shortcut_label: "Alt+Up", - }, - CommandEntry { - display_name: "Focus Pane Down", - action_name: "win.focus-pane-down", - shortcut_label: "Alt+Down", - }, - CommandEntry { - display_name: "Resize Pane Left", - action_name: "win.resize-pane-left", - shortcut_label: "Alt+Shift+Left", - }, - CommandEntry { - display_name: "Resize Pane Right", - action_name: "win.resize-pane-right", - shortcut_label: "Alt+Shift+Right", - }, - CommandEntry { - display_name: "Resize Pane Up", - action_name: "win.resize-pane-up", - shortcut_label: "Alt+Shift+Up", - }, - CommandEntry { - display_name: "Resize Pane Down", - action_name: "win.resize-pane-down", - shortcut_label: "Alt+Shift+Down", - }, - CommandEntry { - display_name: "Find in Terminal", - action_name: "win.search", - shortcut_label: "Ctrl+Shift+F", - }, - CommandEntry { - display_name: "Zoom In", - action_name: "win.zoom-in", - shortcut_label: "Ctrl+=", - }, - CommandEntry { - display_name: "Zoom Out", - action_name: "win.zoom-out", - shortcut_label: "Ctrl+\u{2212}", - }, - CommandEntry { - display_name: "Reset Zoom", - action_name: "win.zoom-reset", - shortcut_label: "Ctrl+0", - }, - CommandEntry { display_name: "Clear", action_name: "win.clear", shortcut_label: "" }, - CommandEntry { display_name: "Reset", action_name: "win.reset", shortcut_label: "" }, - CommandEntry { - display_name: "Open Configuration", - action_name: "win.open-config", - shortcut_label: "", - }, - CommandEntry { - display_name: "Reload Configuration", - action_name: "win.reload-config", - shortcut_label: "", - }, - CommandEntry { - display_name: "Appearance", - action_name: "win.appearance", - shortcut_label: "Ctrl+,", - }, - CommandEntry { - display_name: "Keyboard Shortcuts", - action_name: "win.show-shortcuts", - shortcut_label: "F1", - }, - CommandEntry { - display_name: "About Forgetty", - action_name: "win.show-about", - shortcut_label: "", - }, - CommandEntry { - display_name: "Quit", - action_name: "app.quit", - shortcut_label: "Ctrl+Shift+Q", - }, - ]; - COMMANDS -} - -/// Build the command palette overlay widget. -/// -/// Returns the outer container (a `gtk4::Box` used as the overlay child) -/// and internal widgets needed for wiring up actions. -/// -/// The palette is a centered card with a SearchEntry at the top and a -/// scrollable ListBox below it. It starts hidden; the caller adds it -/// as an overlay child on the `main_overlay` and toggles visibility. -fn build_command_palette( - window: &adw::ApplicationWindow, - workspace_manager: &WorkspaceManager, -) -> gtk4::Box { - let registry = command_registry(); - - // --- Outer alignment container --- - // This Box fills the entire overlay area. We use alignment to center the - // palette card horizontally at the top third of the window. - let outer = gtk4::Box::new(gtk4::Orientation::Vertical, 0); - outer.set_halign(gtk4::Align::Center); - outer.set_valign(gtk4::Align::Start); - outer.set_margin_top(60); - outer.set_hexpand(true); - outer.set_vexpand(true); - // Request ~55% of default window width; actual width adapts via CSS/hexpand. - outer.set_width_request((DEFAULT_WIDTH as f64 * 0.55) as i32); - outer.set_visible(false); - outer.set_can_focus(false); - // Give it a card-like look - outer.add_css_class("card"); - outer.add_css_class("command-palette"); - - // --- Search entry --- - let search_entry = gtk4::SearchEntry::new(); - search_entry.set_placeholder_text(Some("Type a command\u{2026}")); - search_entry.set_hexpand(true); - search_entry.set_focusable(true); - search_entry.set_can_focus(true); - search_entry.set_margin_start(8); - search_entry.set_margin_end(8); - search_entry.set_margin_top(8); - search_entry.set_margin_bottom(4); - outer.append(&search_entry); - - let separator = gtk4::Separator::new(gtk4::Orientation::Horizontal); - outer.append(&separator); - - // --- Scrollable command list --- - let list_box = gtk4::ListBox::new(); - list_box.set_selection_mode(gtk4::SelectionMode::Single); - list_box.add_css_class("navigation-sidebar"); - - // Populate with all commands - for entry in registry { - let row = build_palette_row(entry); - list_box.append(&row); - } - - let scrolled = gtk4::ScrolledWindow::new(); - scrolled.set_child(Some(&list_box)); - scrolled.set_vexpand(true); - scrolled.set_propagate_natural_height(true); - scrolled.set_max_content_height(400); - scrolled.set_policy(gtk4::PolicyType::Never, gtk4::PolicyType::Automatic); - outer.append(&scrolled); - - // Select the first row by default - if let Some(first_row) = list_box.row_at_index(0) { - list_box.select_row(Some(&first_row)); - } - - // --- Filtering logic --- - // On search-changed, show/hide rows based on substring match and - // auto-select the first visible row. - { - let lb = list_box.clone(); - search_entry.connect_search_changed(move |entry| { - let query = entry.text().to_string().to_lowercase(); - let registry = command_registry(); - let mut first_visible: Option = None; - - for (i, cmd) in registry.iter().enumerate() { - let Some(row) = lb.row_at_index(i as i32) else { - continue; - }; - let visible = query.is_empty() || cmd.display_name.to_lowercase().contains(&query); - row.set_visible(visible); - if visible && first_visible.is_none() { - first_visible = Some(row); - } - } - - // Auto-select first visible row - if let Some(row) = first_visible { - lb.select_row(Some(&row)); - // Scroll to make the selected row visible - row.grab_focus(); - // Return focus to search entry after scroll adjustment - entry.grab_focus(); - } else { - lb.select_row(gtk4::ListBoxRow::NONE); - } - }); - } - - // --- Keyboard navigation --- - // Up/Down arrows move selection; Enter executes; Escape closes. - { - let lb = list_box.clone(); - let outer_ref = outer.clone(); - let win = window.clone(); - let wm = Rc::clone(workspace_manager); - let key_controller = gtk4::EventControllerKey::new(); - key_controller.connect_key_pressed(move |_ctrl, key, _code, _mods| { - match key { - gtk4::gdk::Key::Escape => { - let ft = active_focus_tracker(&wm); - close_command_palette(&outer_ref, &ft); - glib::Propagation::Stop - } - gtk4::gdk::Key::Return | gtk4::gdk::Key::KP_Enter => { - if let Some(row) = lb.selected_row() { - let index = row.index(); - let registry = command_registry(); - if let Some(cmd) = registry.get(index as usize) { - let action_name = cmd.action_name.to_string(); - let ft = active_focus_tracker(&wm); - close_command_palette(&outer_ref, &ft); - // Defer action dispatch so the palette is fully hidden - // before any dialog opens (e.g. "Change Tab Title"). - let win_deferred = win.clone(); - glib::idle_add_local_once(move || { - let _ = gtk4::prelude::WidgetExt::activate_action( - &win_deferred, - &action_name, - None, - ); - }); - } - } - glib::Propagation::Stop - } - gtk4::gdk::Key::Down => { - move_palette_selection(&lb, true); - glib::Propagation::Stop - } - gtk4::gdk::Key::Up => { - move_palette_selection(&lb, false); - glib::Propagation::Stop - } - _ => glib::Propagation::Proceed, - } - }); - search_entry.add_controller(key_controller); - } - - // --- Row activation (Enter or click on a row) --- - { - let outer_ref = outer.clone(); - let win = window.clone(); - let wm = Rc::clone(workspace_manager); - list_box.connect_row_activated(move |_lb, row| { - let index = row.index(); - let registry = command_registry(); - if let Some(cmd) = registry.get(index as usize) { - let action_name = cmd.action_name.to_string(); - let ft = active_focus_tracker(&wm); - close_command_palette(&outer_ref, &ft); - // Defer action dispatch so the palette is fully hidden - // before any dialog opens (e.g. "Change Tab Title"). - let win_deferred = win.clone(); - glib::idle_add_local_once(move || { - let _ = gtk4::prelude::WidgetExt::activate_action( - &win_deferred, - &action_name, - None, - ); - }); - } - }); - } - - outer -} - -/// Build a single row for the command palette list. -/// -/// Each row is a horizontal Box with the command name on the left and -/// the shortcut label (if any) on the right in a muted style. -fn build_palette_row(entry: &CommandEntry) -> gtk4::ListBoxRow { - let hbox = gtk4::Box::new(gtk4::Orientation::Horizontal, 12); - hbox.set_margin_start(12); - hbox.set_margin_end(12); - hbox.set_margin_top(6); - hbox.set_margin_bottom(6); - - let name_label = gtk4::Label::new(Some(entry.display_name)); - name_label.set_halign(gtk4::Align::Start); - name_label.set_hexpand(true); - hbox.append(&name_label); - - if !entry.shortcut_label.is_empty() { - let shortcut_label = gtk4::Label::new(Some(entry.shortcut_label)); - shortcut_label.set_halign(gtk4::Align::End); - shortcut_label.add_css_class("dim-label"); - hbox.append(&shortcut_label); - } - - let row = gtk4::ListBoxRow::new(); - row.set_child(Some(&hbox)); - row -} - -/// Move the palette selection up or down with wrapping. -/// -/// Skips hidden (filtered-out) rows to navigate only visible entries. -fn move_palette_selection(list_box: >k4::ListBox, forward: bool) { - let current_index = list_box.selected_row().map(|r| r.index()).unwrap_or(-1); - - // Collect indices of visible rows - let mut visible_indices: Vec = Vec::new(); - let mut i = 0; - while let Some(row) = list_box.row_at_index(i) { - if row.is_visible() { - visible_indices.push(i); - } - i += 1; - } - - if visible_indices.is_empty() { - return; - } - - // Find current position in the visible list - let current_pos = visible_indices.iter().position(|&idx| idx == current_index); - let next_pos = match current_pos { - Some(pos) => { - if forward { - (pos + 1) % visible_indices.len() - } else if pos == 0 { - visible_indices.len() - 1 - } else { - pos - 1 - } - } - // No current selection: pick the first or last visible row - None => { - if forward { - 0 - } else { - visible_indices.len() - 1 - } - } - }; - - let target_index = visible_indices[next_pos]; - if let Some(row) = list_box.row_at_index(target_index) { - list_box.select_row(Some(&row)); - } -} - -/// Close the command palette and restore focus to the previously focused pane. -fn close_command_palette(palette: >k4::Box, focus_tracker: &FocusTracker) { - palette.set_visible(false); - - // Restore focus to the DrawingArea that was focused before the palette opened - let focused_name = { - let Ok(name) = focus_tracker.try_borrow() else { - return; - }; - name.clone() - }; - - if focused_name.is_empty() { - return; - } - - let app = - gtk4::gio::Application::default().and_then(|a| a.downcast::().ok()); - let Some(app) = app else { - return; - }; - let Some(window) = app.active_window() else { - return; - }; - - if let Some(da) = find_drawing_area_by_name(&window, &focused_name) { - da.grab_focus(); - } -} - -/// Open the command palette: show it, clear previous query, focus the search entry. -/// -/// Resets all row visibility and selects the first row so the palette always -/// opens in a clean state. -fn open_command_palette(palette: >k4::Box, window: &adw::ApplicationWindow) { - palette.set_visible(true); - - // Find the SearchEntry (first child) and clear + focus it - let search_entry = palette.first_child().and_then(|w| w.downcast::().ok()); - if let Some(ref entry) = search_entry { - entry.set_text(""); - } - - // Find the ListBox (inside ScrolledWindow, third child: entry, separator, scrolled) - // and ensure all rows are visible + first is selected. - let mut child = palette.first_child(); - while let Some(c) = child { - if let Some(scrolled) = c.downcast_ref::() { - if let Some(lb) = scrolled.child().and_then(|w| w.downcast::().ok()) { - let mut i = 0; - while let Some(row) = lb.row_at_index(i) { - row.set_visible(true); - i += 1; - } - if let Some(first) = lb.row_at_index(0) { - lb.select_row(Some(&first)); - } - } - break; - } - child = c.next_sibling(); - } - - // Use the window's set_focus() to directly assign keyboard focus to the - // search entry. This works reliably even before the widget is fully mapped, - // unlike grab_focus() which silently fails on unrealized widgets. - if let Some(entry) = search_entry { - gtk4::prelude::GtkWindowExt::set_focus(window, Some(&entry)); - } -} - -/// Toggle the command palette open/closed. -fn toggle_command_palette( - palette: >k4::Box, - focus_tracker: &FocusTracker, - window: &adw::ApplicationWindow, -) { - if palette.is_visible() { - close_command_palette(palette, focus_tracker); - } else { - open_command_palette(palette, window); - } -} // --------------------------------------------------------------------------- // Tab right-click context menu (T-M1-extra-009) @@ -8917,7 +8273,8 @@ fn delete_current_workspace( let delete_idx = mgr.active_index; let ws = &mgr.workspaces[delete_idx]; - // Ask the daemon to close every pane in the workspace (or just drop in --temp). + // Ask the daemon to close every pane in the workspace (or just drop in + // the legacy no-daemon fallback — see CHORE-FIX-011-cleanup). close_workspace_panes(&ws.tab_states, daemon_client.map(|a| a.as_ref())); // Remove the TabView from main_area if it is currently visible. @@ -10009,9 +9366,9 @@ fn show_rename_workspace_dialog_for( /// Duplicate a workspace (FIX-007). The daemon is authoritative — GTK sends /// a `duplicate_workspace` RPC, then builds widgets from the response. /// -/// In `--temp` mode (no daemon) the pre-FIX-007 local-only behaviour is -/// preserved. In daemon mode the old local-only flow is replaced by an -/// RPC-first flow: +/// Post-FIX-011 the daemon path is the only live path; the legacy local-only +/// fallback (`duplicate_workspace_temp_fallback`) is retained as unreachable +/// code pending CHORE-FIX-011-cleanup. In daemon mode the flow is RPC-first: /// /// 1. TOCTOU-safe bounds re-check against GTK state (context menu already /// captured `workspace_idx` at construction time per the existing pattern @@ -10049,9 +9406,9 @@ fn duplicate_workspace( } } - // --temp mode (no daemon client): fall back to the legacy local-only - // duplicate to keep in-memory workspaces working. Daily-driver runs - // the daemon-mode path below (AC-10). + // Legacy fallback (no daemon client): unreachable post-FIX-011 since + // daemon_client is always Some on the live path. Retained until + // CHORE-FIX-011-cleanup deletes the fallback function. let Some(dc) = daemon_client.as_ref().cloned() else { duplicate_workspace_temp_fallback( workspace_idx, @@ -10298,9 +9655,11 @@ fn duplicate_workspace( ); } -/// Legacy local-only duplicate for `--temp` mode (no daemon client). +/// Legacy local-only duplicate for the no-daemon-client path. /// -/// Pre-FIX-007 behaviour, kept unchanged for compatibility. Daemon mode +/// Pre-FIX-007 behaviour, kept unchanged for compatibility. Post-FIX-011 the +/// daemon path is the only live path, making this function unreachable on the +/// live launcher. Deletion is deferred to CHORE-FIX-011-cleanup; daemon mode /// (the daily-driver path) uses the RPC-authoritative `duplicate_workspace` /// above. fn duplicate_workspace_temp_fallback( @@ -10371,11 +9730,11 @@ fn duplicate_workspace_temp_fallback( new_tv.connect_create_window(move |_| Some(open_detached_tab_window(&app_c))); } - // Add one tab per source CWD — in --temp mode `add_new_tab` no-ops - // without a daemon client, which means the duplicate is effectively - // an empty workspace. This matches pre-FIX-007 behaviour exactly - // (the old code also called `add_new_tab` with `daemon_client.clone()` - // which would be `None` in --temp mode). + // Add one tab per source CWD — without a daemon client `add_new_tab` + // no-ops, which means the duplicate is effectively an empty workspace. + // This matches pre-FIX-007 behaviour exactly (the old code also called + // `add_new_tab` with `daemon_client.clone()` which was `None` in this + // legacy path). Unreachable post-FIX-011 — see CHORE-FIX-011-cleanup. for cwd_opt in &cwds { add_new_tab( workspace_idx, @@ -10680,8 +10039,10 @@ fn delete_workspace_at_index( /// event (or next refresh) will bring GTK back in sync with daemon truth /// (AC-12). /// -/// In `--temp` mode (no daemon_client) we skip the RPC and fall back to -/// closing panes locally via `close_workspace_panes`. +/// In the legacy no-daemon-client fallback (unreachable post-FIX-011) we +/// skip the RPC and fall back to closing panes locally via +/// `close_workspace_panes`. The fallback is retained until +/// CHORE-FIX-011-cleanup. #[allow(clippy::too_many_arguments)] fn do_delete_workspace_at_index( target_idx: usize, @@ -10704,8 +10065,8 @@ fn do_delete_workspace_at_index( } // FIX-003: daemon is authoritative. Call RPC before any local widget - // mutation. In --temp mode (no daemon_client) fall back to the per-pane - // close loop below. + // mutation. In the legacy no-daemon-client fallback (unreachable + // post-FIX-011) fall back to the per-pane close loop below. if let Some(dc) = daemon_client.as_ref() { if let Err(e) = dc.delete_workspace(target_idx) { tracing::warn!( @@ -10723,9 +10084,10 @@ fn do_delete_workspace_at_index( let ws = &mgr.workspaces[target_idx]; - // --temp mode only: daemon RPC didn't fire, so we still need to tear down - // the in-process pane registry locally. In daemon mode the daemon already - // killed the panes + unlinked byte logs as part of the RPC. + // Legacy fallback only: daemon RPC didn't fire, so we still need to tear + // down the in-process pane registry locally. In daemon mode the daemon + // already killed the panes + unlinked byte logs as part of the RPC. + // Unreachable post-FIX-011 — see CHORE-FIX-011-cleanup. if daemon_client.is_none() { close_workspace_panes(&ws.tab_states, None); } diff --git a/crates/forgetty-gtk/src/settings_view.rs b/crates/forgetty-gtk/src/settings_view.rs index 58fa7c4..c5b57a7 100644 --- a/crates/forgetty-gtk/src/settings_view.rs +++ b/crates/forgetty-gtk/src/settings_view.rs @@ -228,18 +228,11 @@ pub static ACTION_DEFS: &[ActionDef] = &[ default_accels: &[], category: "Configuration", }, - ActionDef { - display_name: "Command Palette", - config_key: "command-palette", - action_name: "win.command-palette", - default_accels: &["p"], - category: "Configuration", - }, ActionDef { display_name: "Keyboard Shortcuts (reference)", config_key: "show-shortcuts", action_name: "win.show-shortcuts", - default_accels: &["F1"], + default_accels: &["F1", "p"], category: "Configuration", }, // --- Workspaces --- @@ -353,13 +346,6 @@ pub static ACTION_DEFS: &[ActionDef] = &[ display_name: "New Window", config_key: "new-window", action_name: "win.new-window", - default_accels: &[], - category: "Application", - }, - ActionDef { - display_name: "New Temporary Window", - config_key: "new-temp-window", - action_name: "win.new-temp-window", default_accels: &["n"], category: "Application", }, diff --git a/src/cli.rs b/src/cli.rs index b08cc0d..2dfc4d6 100644 --- a/src/cli.rs +++ b/src/cli.rs @@ -36,11 +36,6 @@ pub struct Args { #[arg(long)] pub restore_all: bool, - /// Open an ephemeral session that is never persisted. The terminal works - /// normally but no session file is written on close. - #[arg(long)] - pub temp: bool, - /// Restore a specific trashed session by UUID. /// /// Moves the session file from `sessions/trash/` back to `sessions/`, diff --git a/src/main.rs b/src/main.rs index c8f1b4e..5526301 100644 --- a/src/main.rs +++ b/src/main.rs @@ -195,7 +195,6 @@ fn main() { // In that case, if config says Restore and sessions exist, spawn one window // per saved session and exit — identical to --restore-all. let is_bare_launch = !args.no_restore - && !args.temp && args.session_id.is_none() && working_directory.is_none() && args.execute.is_empty(); @@ -269,7 +268,6 @@ fn main() { no_restore: args.no_restore, session_id: args.session_id, restore_all: args.restore_all, - temp: args.temp, }; if let Err(e) = forgetty_gtk::app::run(config, launch_opts) {