From 29ed12f28ebf3c3092b9c5e1c09a189570eeeade Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Fri, 23 Oct 2026 07:58:33 +0700 Subject: [PATCH 01/11] feat(linux-editor): Vim keybindings in the SQL editor --- linux/README.md | 23 + linux/ROADMAP.md | 4 +- .../app/src/application/accelerators.rs | 86 +++ linux/crates/app/src/application/mod.rs | 4 +- .../app/src/ui/editor/editor_actions.rs | 2 +- linux/crates/app/src/ui/editor/editor_file.rs | 60 +- linux/crates/app/src/ui/editor/imp.rs | 11 + linux/crates/app/src/ui/editor/mod.rs | 17 +- linux/crates/app/src/ui/editor/tests.rs | 513 +++++++++++++++++- linux/crates/app/src/ui/editor/vim_mode.rs | 391 +++++++++++++ .../src/ui/preferences_dialog/editor_page.rs | 21 +- linux/crates/app/src/ui/workspace/mod.rs | 3 + .../app/src/ui/workspace/tab_factory.rs | 2 + .../app/src/ui/workspace/tests/close_tab.rs | 50 ++ .../app/src/ui/workspace/workspace_tabs.rs | 9 + linux/crates/storage/src/settings/keys.rs | 1 + linux/crates/storage/src/settings/mod.rs | 9 + linux/crates/storage/tests/settings.rs | 12 + .../data/app.tablepro.TablePro.gschema.xml.in | 6 + .../resources/ui/preferences-editor-page.ui | 11 + linux/data/resources/ui/sql-editor.ui | 75 ++- linux/docs/storage.md | 1 + linux/po/POTFILES.in | 1 + 23 files changed, 1262 insertions(+), 50 deletions(-) create mode 100644 linux/crates/app/src/ui/editor/vim_mode.rs diff --git a/linux/README.md b/linux/README.md index 055dd260fa..79c47503ac 100644 --- a/linux/README.md +++ b/linux/README.md @@ -236,6 +236,29 @@ exits. A file already open brings its window forward. A database on a network or FUSE filesystem asks whether to open read-only first, because SQLite's locking cannot be trusted there. +## Vim keybindings + +Turn on **Preferences → Editor → Vim Keybindings**. The SQL editor then +runs GtkSourceView's Vim emulation: normal, insert, replace and visual +modes, motions, text objects, registers, marks, search with `/` and `?`, +and `.` to repeat. The line under the text shows the mode and the command +you are typing. + +| Command | Does | +|---|---| +| `:w` | Saves the tab's file, as Ctrl+S does. A tab with no file asks for one | +| `:w name` | Saves to `name`, next to the tab's file or in your home folder, and the tab follows it. An existing file is left alone; `:w! name` replaces it | +| `:e` | Reads the tab's file again, asking first when that loses changes. `:e!` does not ask | +| `:e name` | Opens `name` in the tab, as Open SQL File does. `:e! name` does not ask | +| `:q`, `:q!` | Closes the tab. A tab with unsaved file changes or an open transaction asks first, as closing does | +| `:wq`, `:x` | Saves, then closes the tab. `:x` skips the save when the file has no changes | + +The editor's own Ctrl shortcuts work in every mode: Ctrl+Return runs, +Ctrl+Shift+Return runs the statement at the cursor, and find, find and +replace, format, toggle comment, open and save keep their keys. Every +other key goes to Vim, Escape too, so while Vim is on the **Stop** button +stops a running query. + ## SQL Server with Windows integrated auth Pick **Method → Windows (Kerberos)** in the connect dialog. There is no diff --git a/linux/ROADMAP.md b/linux/ROADMAP.md index 4252135c22..026659b23d 100644 --- a/linux/ROADMAP.md +++ b/linux/ROADMAP.md @@ -74,6 +74,7 @@ The app runs on GNOME 50 ([ADR 0015](docs/decisions/0015-gnome-50-platform-floor | Files | Open a .sql file into the editor (Ctrl+O) or drop one on it, and save to it (Ctrl+S, or Save As with Ctrl+Shift+S) | | Results | Several result sets per run, a limit of 10,000 rows per result and 100,000 per run with a banner and Export All, and Go to Error | | Drafts | Each editor tab's text is kept, and a connection's tabs (table tabs with page and sort, structure and editor tabs) reopen when it opens | +| Vim mode | GtkSourceView's Vim emulation, off by default, with the mode and pending command under the text; `:w`, `:e`, `:q`, `:wq` and `:x` act on the tab, and the editor's Ctrl shortcuts work in every mode ([README](README.md#vim-keybindings)) | ### History and saved queries @@ -102,7 +103,7 @@ The app runs on GNOME 50 ([ADR 0015](docs/decisions/0015-gnome-50-platform-floor | Session restore | The windows of saved connections and SQLite files open at quit open again at launch (Restore Session, on by default), also after New Window or a quit during the restore | | Closing safely | Unsaved edits, a save in flight or of unknown outcome, an open editor transaction, or a running export ask before a window closes or the app quits; logout waits while any of those holds, and suspend while a save runs or an export writes | | Notifications | Work that finishes in a tab you are not looking at sends a notification | -| Preferences | Restore session, page size, the bulk delete confirmation, query history, editor font and scheme, query timeout | +| Preferences | Restore session, page size, the bulk delete confirmation, query history, editor font and scheme, Vim keybindings, query timeout | | Shortcuts and About | A shortcuts dialog, and About from the metainfo with third-party licenses and debug info | ### Packaging and platform @@ -141,7 +142,6 @@ Not built yet, in no order. ### SQL editor - Multi-cursor editing. -- Vim mode. - Snippets. ### Release and project diff --git a/linux/crates/app/src/application/accelerators.rs b/linux/crates/app/src/application/accelerators.rs index 619ac096de..199896ab50 100644 --- a/linux/crates/app/src/application/accelerators.rs +++ b/linux/crates/app/src/application/accelerators.rs @@ -385,6 +385,28 @@ pub fn component_matches(action: &str, keyval: gdk::Key, state: gdk::ModifierTyp .any(|(key, mods)| mods == modifiers && (key == keyval || keypad_twins(key).contains(&keyval))) } +/// The SQL editor's own Ctrl keys, which Vim mode leaves to the editor in +/// every mode. Escape and the other plain keys are Vim's. +pub fn editor_keeps(keyval: gdk::Key, state: gdk::ModifierType) -> bool { + let modifiers = state & gtk::accelerator_get_default_mod_mask(); + let keyval = keyval.to_lower(); + ACCELERATORS + .iter() + .filter(|entry| { + matches!( + entry.scope, + Local { + owner: SqlEditor | SqlEditorTab, + .. + } + ) + }) + .flat_map(|entry| entry.triggers.iter()) + .filter_map(|accel| parse(accel)) + .filter(|(_, mods)| mods.contains(gdk::ModifierType::CONTROL_MASK)) + .any(|(key, mods)| mods == modifiers && (key == keyval || keypad_twins(key).contains(&keyval))) +} + /// GtkPopoverMenu shows an application accelerator by itself; a Local or /// Component one has to be put on the item. pub fn menu_item(label: &str, detailed_action: &str) -> gio::MenuItem { @@ -662,6 +684,70 @@ mod tests { assert!(!component_matches("cell-editor.pick-date", gdk::Key::Return, none)); } + #[gtk4::test] + fn the_editor_keeps_its_ctrl_keys_from_vim() { + let ctrl = gdk::ModifierType::CONTROL_MASK; + let ctrl_shift = ctrl | gdk::ModifierType::SHIFT_MASK; + let none = gdk::ModifierType::empty(); + + for (key, state) in [ + (gdk::Key::Return, ctrl), + (gdk::Key::KP_Enter, ctrl), + (gdk::Key::Return, ctrl_shift), + (gdk::Key::F, ctrl_shift), + (gdk::Key::slash, ctrl), + (gdk::Key::f, ctrl), + (gdk::Key::f, ctrl | gdk::ModifierType::LOCK_MASK), + (gdk::Key::h, ctrl), + (gdk::Key::g, ctrl), + (gdk::Key::G, ctrl_shift), + (gdk::Key::o, ctrl), + (gdk::Key::s, ctrl), + (gdk::Key::S, ctrl_shift), + ] { + assert!(editor_keeps(key, state), "{key:?} with {state:?}"); + } + for (key, state) in [ + (gdk::Key::Escape, none), + (gdk::Key::f, none), + (gdk::Key::Return, none), + (gdk::Key::r, ctrl), + (gdk::Key::d, ctrl), + (gdk::Key::v, ctrl), + (gdk::Key::f, gdk::ModifierType::ALT_MASK), + (gdk::Key::F6, none), + ] { + assert!(!editor_keeps(key, state), "{key:?} with {state:?}"); + } + } + + #[gtk4::test] + fn every_ctrl_key_of_the_editor_is_kept_from_vim() { + let mut checked = 0; + for entry in ACCELERATORS { + if !matches!( + entry.scope, + Local { + owner: SqlEditor | SqlEditorTab, + .. + } + ) { + continue; + } + for accel in entry.triggers { + let (key, mods) = gtk::accelerator_parse(*accel).expect("the table parses"); + assert_eq!( + editor_keeps(key, mods), + mods.contains(gdk::ModifierType::CONTROL_MASK), + "{accel} of {}", + entry.action + ); + checked += 1; + } + } + assert_eq!(checked, 12); + } + #[gtk4::test] fn shortcuts_dialog_matches_accelerator_table() { adw::init().expect("libadwaita initialises under the test backend"); diff --git a/linux/crates/app/src/application/mod.rs b/linux/crates/app/src/application/mod.rs index 85a4ffe6e4..29e68ef1ac 100644 --- a/linux/crates/app/src/application/mod.rs +++ b/linux/crates/app/src/application/mod.rs @@ -40,8 +40,8 @@ pub use accelerator_scope::AcceleratorScope; #[cfg(test)] pub(crate) use accelerators::keypad_twins; pub use accelerators::{ - ACCELERATORS, component_matches, component_trigger, controller_name, install_application_accelerators, - local_shortcut_controller, menu_item, triggers_for, + ACCELERATORS, component_matches, component_trigger, controller_name, editor_keeps, + install_application_accelerators, local_shortcut_controller, menu_item, triggers_for, }; pub use app_actions::install_app_actions; pub use app_services::{AppServices, AppServicesParts}; diff --git a/linux/crates/app/src/ui/editor/editor_actions.rs b/linux/crates/app/src/ui/editor/editor_actions.rs index c89b717d05..a06c24d4e0 100644 --- a/linux/crates/app/src/ui/editor/editor_actions.rs +++ b/linux/crates/app/src/ui/editor/editor_actions.rs @@ -111,7 +111,7 @@ impl SqlEditor { /// GtkSourceView parents its completion list and other assistants to /// the view as popovers, and its Escape hides every visible one. -fn close_view_popups(view: &impl IsA) -> bool { +pub(super) fn close_view_popups(view: &impl IsA) -> bool { let mut closed = false; let mut child = view.as_ref().first_child(); while let Some(widget) = child { diff --git a/linux/crates/app/src/ui/editor/editor_file.rs b/linux/crates/app/src/ui/editor/editor_file.rs index 28d60b19cb..d138e43833 100644 --- a/linux/crates/app/src/ui/editor/editor_file.rs +++ b/linux/crates/app/src/ui/editor/editor_file.rs @@ -67,14 +67,42 @@ impl SqlEditor { /// Ctrl+S: write to the tab's file, or ask for one. pub(super) fn save(&self) { - match self.file() { - Some(file) => self.write_reporting(file), - None => self.save_as(), - } + self.save_then(|_| {}); + } + + /// A cancelled choice or a failed write skips `then`. + pub(super) fn save_then(&self, then: impl FnOnce(&SqlEditor) + 'static) { + let weak = self.downgrade(); + let file = self.file(); + let chosen = file.is_none().then(|| self.choose_save_file()); + glib::spawn_future_local(async move { + let file = match (file, chosen) { + (Some(file), _) => Some(file), + (None, Some(chosen)) => chosen.await, + (None, None) => None, + }; + let (Some(file), Some(editor)) = (file, weak.upgrade()) else { + return; + }; + if editor.write_reporting(&file).await { + then(&editor); + } + }); } /// Ctrl+Shift+S: ask where to write, starting from the tab's file. pub(super) fn save_as(&self) { + let weak = self.downgrade(); + let chosen = self.choose_save_file(); + glib::spawn_future_local(async move { + if let (Some(file), Some(editor)) = (chosen.await, weak.upgrade()) { + editor.write_reporting(&file).await; + } + }); + } + + /// Holds only the dialog's window while it is open. + fn choose_save_file(&self) -> impl Future> + use<> { let filter = sql_filter(); let filters = gio::ListStore::new::(); filters.append(&filter); @@ -89,15 +117,7 @@ impl SqlEditor { None => dialog.set_initial_name(Some(&suggested_file_name(&buffer_text(&self.buffer())))), } let parent = self.root().and_downcast::(); - let weak = self.downgrade(); - glib::spawn_future_local(async move { - let Ok(file) = dialog.save_future(parent.as_ref()).await else { - return; - }; - if let Some(editor) = weak.upgrade() { - editor.write_reporting(file); - } - }); + async move { dialog.save_future(parent.as_ref()).await.ok() } } /// Write the text to the tab's file, for a close that chose Save. @@ -112,13 +132,15 @@ impl SqlEditor { } } - fn write_reporting(&self, file: gio::File) { - let editor = self.clone(); - glib::spawn_future_local(async move { - if let Err(message) = editor.write_to(&file).await { - editor.alert_save_failed(&message); + /// Whether the text landed in `file`; an alert says why it did not. + pub(super) async fn write_reporting(&self, file: &gio::File) -> bool { + match self.write_to(file).await { + Ok(()) => true, + Err(message) => { + self.alert_save_failed(&message); + false } - }); + } } /// Text typed while the write runs is not in the file, so the tab diff --git a/linux/crates/app/src/ui/editor/imp.rs b/linux/crates/app/src/ui/editor/imp.rs index 11861592fa..60e00fb65a 100644 --- a/linux/crates/app/src/ui/editor/imp.rs +++ b/linux/crates/app/src/ui/editor/imp.rs @@ -15,6 +15,7 @@ use super::find_bar::FindBar; use super::results_pane::ResultsPane; use super::run_log::RunLog; use super::transaction_dirty::OpenTransaction; +use super::vim_mode::VimMode; use crate::ui::grid::GridSink; use crate::ui::handler_guard::HandlerGuard; @@ -41,6 +42,12 @@ pub struct SqlEditor { #[template_child] pub source_view: TemplateChild, #[template_child] + pub text_pane: TemplateChild, + #[template_child] + pub vim_command_bar: TemplateChild, + #[template_child] + pub vim_command: TemplateChild, + #[template_child] pub results: TemplateChild, pub(super) deps: OnceCell, @@ -69,6 +76,7 @@ pub struct SqlEditor { /// settings, which would otherwise outlive the tab. pub(super) app_handlers: RefCell>, pub(super) completion_providers: RefCell>, + pub(super) vim: RefCell>, } #[glib::object_subclass] @@ -114,6 +122,8 @@ impl ObjectImpl for SqlEditor { .param_types([bool::static_type()]) .build(), Signal::builder("file-changed").build(), + // Vim's :q, which closes the tab through its close prompts. + Signal::builder("close-requested").build(), ] }) } @@ -121,6 +131,7 @@ impl ObjectImpl for SqlEditor { fn dispose(&self) { self.obj().shutdown(); self.app_handlers.take(); + self.obj().detach_vim(); self.obj().release_view(); } } diff --git a/linux/crates/app/src/ui/editor/mod.rs b/linux/crates/app/src/ui/editor/mod.rs index 4e892998e4..29f6319838 100644 --- a/linux/crates/app/src/ui/editor/mod.rs +++ b/linux/crates/app/src/ui/editor/mod.rs @@ -27,6 +27,7 @@ mod statement_cursor; #[cfg(test)] mod tests; mod transaction_dirty; +mod vim_mode; use std::rc::Rc; use std::sync::Arc; @@ -131,6 +132,9 @@ impl SqlEditor { editor.connect_buffer(&buffer); editor.follow_file_state(&buffer); editor.install_actions(); + // After the editor's own key controllers, so Vim's runs first. + let vim = editor.follow_vim_preference(&init.settings); + editor.imp().app_handlers.borrow_mut().push(vim); editor.install_drop_target(); editor.connect_buttons(); editor.connect_grid(); @@ -855,12 +859,17 @@ impl SqlEditor { } fn open_file(&self) { + self.replace_after_asking(SqlEditor::choose_file); + } + + /// Loading replaces what is in the tab, which is where the user's + /// unrun work lives, so `replace` runs once the user agrees, or + /// straight away when the tab holds nothing. + fn replace_after_asking(&self, replace: impl Fn(&SqlEditor) + 'static) { if buffer_text(&self.buffer()).trim().is_empty() { - self.choose_file(); + replace(self); return; } - // Loading replaces what is in the tab, which is where the user's - // unrun work lives, so it asks first. let dialog = adw::AlertDialog::new( Some(&crate::i18n::gettext("Replace the Editor Contents?")), Some(&crate::i18n::gettext( @@ -874,7 +883,7 @@ impl SqlEditor { let weak = self.downgrade(); dialog.connect_response(Some("replace"), move |_, _| { if let Some(editor) = weak.upgrade() { - editor.choose_file(); + replace(&editor); } }); dialog.present(Some(self)); diff --git a/linux/crates/app/src/ui/editor/tests.rs b/linux/crates/app/src/ui/editor/tests.rs index 75cfda97a1..33ee65b5d8 100644 --- a/linux/crates/app/src/ui/editor/tests.rs +++ b/linux/crates/app/src/ui/editor/tests.rs @@ -19,7 +19,7 @@ struct Harness { fake: Arc, history: Option, active: ActiveSession, - _settings: MemorySettings, + settings: MemorySettings, runtime: crate::runtime::AppRuntime, } @@ -49,9 +49,17 @@ fn editor_with(runs: Replies, text: &str) -> Harness { } fn build_editor(runs: Replies, text: &str, history: impl FnOnce(&Tasks) -> Option) -> Harness { + build_editor_on(MemorySettings::new(), runs, text, history) +} + +fn build_editor_on( + settings: MemorySettings, + runs: Replies, + text: &str, + history: impl FnOnce(&Tasks) -> Option, +) -> Harness { crate::register_test_resources(); let runtime = crate::test_support::test_runtime(); - let settings = MemorySettings::new(); let fake = Arc::new(FakeEditorSession::new(runs)); let active = ActiveSession::new(); active.replace(session_with(fake.clone(), runtime.tasks())); @@ -70,7 +78,7 @@ fn build_editor(runs: Replies, text: &str, history: impl FnOnce(&Tasks) fake, history, active, - _settings: settings, + settings, runtime, } } @@ -1009,6 +1017,27 @@ fn a_closed_editor_lets_go_of_its_script_and_completion() { ); } +#[gtk4::test] +fn a_closed_editor_in_vim_lets_go_of_its_script_and_vim() { + let harness = editor_with(answering(), "SELECT 1"); + let vim = vim(&harness).downgrade(); + let view = harness.editor.imp().source_view.get(); + let script = harness.editor.buffer().downgrade(); + { + let shown = InWindow::new(&harness.editor); + shown.0.present(); + wait_until(WAIT, || view.is_mapped()).expect("the editor on screen"); + crate::test_support::after_next_frame(&view); + } + + drop(view); + drop(harness); + crate::test_support::drain_main_context(); + + assert!(script.upgrade().is_none(), "the script outlived its editor"); + assert!(vim.upgrade().is_none(), "Vim outlived its editor"); +} + fn editor_keys(harness: &Harness) -> gtk4::ShortcutController { crate::test_support::shortcut_keys::owner_controller( &harness.editor.imp().source_view.get(), @@ -1219,6 +1248,309 @@ fn a_tab_with_no_file_has_nothing_unsaved_to_ask_about() { assert!(!harness.editor.has_unsaved_file_changes()); } +/// Turn the preference on and hand over Vim's input method. +fn vim(harness: &Harness) -> sourceview5::VimIMContext { + harness + .settings + .get() + .set_vim_keybindings(true) + .expect("turn Vim on"); + harness.editor.vim_context().expect("Vim on the view") +} + +/// A command line as GtkSourceView sends it once Return is pressed. +fn vim_command(context: &sourceview5::VimIMContext, command: &str) -> bool { + context.emit_by_name::("execute-command", &[&command]) +} + +#[gtk4::test] +fn vim_follows_the_preference_and_leaves_the_view_as_it_was() { + let harness = editor_with(answering(), "select 1"); + let imp = harness.editor.imp(); + let view = imp.source_view.get(); + assert!(!harness.editor.has_vim(), "Vim is on by default"); + assert!(!imp.text_pane.reveals_bottom_bars()); + + vim(&harness); + + assert!(imp.text_pane.reveals_bottom_bars(), "the mode line is hidden"); + assert!(view.overwrites(), "normal mode draws no block cursor"); + + harness + .settings + .get() + .set_vim_keybindings(false) + .expect("turn Vim off"); + + assert!(!harness.editor.has_vim()); + assert!(!imp.text_pane.reveals_bottom_bars()); + assert!(!view.overwrites(), "the block cursor stayed"); + assert!(view.is_cursor_visible()); + assert_eq!(imp.vim_command_bar.label(), ""); +} + +#[gtk4::test] +fn an_editor_opened_with_the_preference_on_starts_in_vim() { + let settings = MemorySettings::new(); + settings.get().set_vim_keybindings(true).expect("turn Vim on"); + + let harness = build_editor_on(settings, answering(), "select 1", |_| None); + + assert!(harness.editor.has_vim()); + assert!(harness.editor.imp().text_pane.reveals_bottom_bars()); +} + +#[gtk4::test] +fn vim_quit_asks_the_workspace_to_close_the_tab() { + let harness = editor_with(answering(), "select 1"); + let context = vim(&harness); + let closes = SignalLog::connect(&harness.editor, "close-requested", |_| ()); + + assert!(vim_command(&context, ":q")); + assert!(vim_command(&context, ":quit!")); + assert!(!vim_command(&context, ":nohl"), "a command of GtkSourceView's own was taken"); + + assert_eq!(closes.count(), 2); +} + +#[gtk4::test] +fn vim_write_saves_the_tabs_file_as_ctrl_s_does() { + let root = tempfile::tempdir().expect("tempdir"); + let path = root.path().join("daily.sql"); + std::fs::write(&path, "SELECT 1;").expect("seed"); + let harness = editor_with(answering(), ""); + open_path(&harness, &path); + let buffer = harness.editor.buffer(); + buffer.insert(&mut buffer.end_iter(), "\nSELECT 2;"); + let context = vim(&harness); + + assert!(vim_command(&context, ":w")); + wait_until(WAIT, || !harness.editor.has_unsaved_file_changes()).expect("the save to land"); + + assert_eq!( + std::fs::read_to_string(&path).expect("read back"), + "SELECT 1;\nSELECT 2;" + ); +} + +#[gtk4::test] +fn vim_write_quit_closes_once_the_file_is_written() { + let root = tempfile::tempdir().expect("tempdir"); + let path = root.path().join("daily.sql"); + std::fs::write(&path, "SELECT 1;").expect("seed"); + let harness = editor_with(answering(), ""); + open_path(&harness, &path); + let buffer = harness.editor.buffer(); + buffer.insert(&mut buffer.end_iter(), "\nSELECT 2;"); + let context = vim(&harness); + let closes = SignalLog::connect(&harness.editor, "close-requested", |_| ()); + + assert!(vim_command(&context, ":wq")); + wait_until(WAIT, || closes.count() == 1).expect("the close after the write"); + + assert_eq!( + std::fs::read_to_string(&path).expect("read back"), + "SELECT 1;\nSELECT 2;" + ); + assert!(!harness.editor.has_unsaved_file_changes()); +} + +#[gtk4::test] +fn vim_write_quit_keeps_the_tab_when_the_write_fails() { + let root = tempfile::tempdir().expect("tempdir"); + let harness = editor_with(answering(), "SELECT 1;"); + harness + .editor + .restore_file(gio::File::for_path(root.path().join("gone").join("daily.sql")), true); + let window = shown(&harness); + let context = vim(&harness); + let closes = SignalLog::connect(&harness.editor, "close-requested", |_| ()); + + assert!(vim_command(&context, ":wq")); + wait_until(WAIT, || crate::workspace::visible_alert(&window).is_some()).expect("the failed write to say so"); + crate::test_support::drain_main_context(); + + assert_eq!(closes.count(), 0, "the tab closed without its text in the file"); +} + +/// The file is not there to write to, so a close means no write was tried. +#[gtk4::test] +fn vim_exit_closes_an_unchanged_file_without_writing() { + let root = tempfile::tempdir().expect("tempdir"); + let harness = editor_with(answering(), "SELECT 1;"); + harness + .editor + .restore_file(gio::File::for_path(root.path().join("gone").join("daily.sql")), false); + let context = vim(&harness); + let closes = SignalLog::connect(&harness.editor, "close-requested", |_| ()); + + assert!(vim_command(&context, ":x")); + + assert_eq!(closes.count(), 1); +} + +#[gtk4::test] +fn vim_write_to_a_name_saves_next_to_the_tabs_file() { + let root = tempfile::tempdir().expect("tempdir"); + let path = root.path().join("daily.sql"); + std::fs::write(&path, "SELECT 1;").expect("seed"); + let harness = editor_with(answering(), ""); + open_path(&harness, &path); + let context = vim(&harness); + + assert!(vim_command(&context, ":w weekly.sql")); + let copy = root.path().join("weekly.sql"); + wait_until(WAIT, || copy.exists()).expect("the new file"); + + assert_eq!(std::fs::read_to_string(©).expect("read back"), "SELECT 1;"); + assert_eq!(harness.editor.file().and_then(|file| file.path()), Some(copy)); +} + +#[gtk4::test] +fn vim_write_to_a_name_leaves_another_file_alone() { + let root = tempfile::tempdir().expect("tempdir"); + let path = root.path().join("daily.sql"); + let other = root.path().join("weekly.sql"); + std::fs::write(&path, "SELECT 1;").expect("seed"); + std::fs::write(&other, "SELECT 7;").expect("seed"); + let harness = editor_with(answering(), ""); + open_path(&harness, &path); + let context = vim(&harness); + let toasts = SignalLog::connect(&harness.editor, "toast", |values| { + values[1].get::().expect("a message") + }); + + assert!(vim_command(&context, ":w weekly.sql")); + wait_until(WAIT, || toasts.count() == 1).expect("the refusal"); + + assert!(toasts.take()[0].starts_with("“weekly.sql” already exists.")); + assert_eq!(std::fs::read_to_string(&other).expect("read back"), "SELECT 7;"); + assert_eq!(harness.editor.file().and_then(|file| file.path()), Some(path)); +} + +#[gtk4::test] +fn vim_force_write_to_a_name_replaces_another_file() { + let root = tempfile::tempdir().expect("tempdir"); + let path = root.path().join("daily.sql"); + let other = root.path().join("weekly.sql"); + std::fs::write(&path, "SELECT 1;").expect("seed"); + std::fs::write(&other, "SELECT 7;").expect("seed"); + let harness = editor_with(answering(), ""); + open_path(&harness, &path); + let context = vim(&harness); + + assert!(vim_command(&context, ":w! weekly.sql")); + wait_until(WAIT, || std::fs::read_to_string(&other).is_ok_and(|text| text == "SELECT 1;")) + .expect("the other file replaced"); + + assert_eq!(harness.editor.file().and_then(|file| file.path()), Some(other)); +} + +#[gtk4::test] +fn vim_force_edit_reads_the_tabs_file_without_asking() { + let root = tempfile::tempdir().expect("tempdir"); + let path = root.path().join("daily.sql"); + std::fs::write(&path, "SELECT 1;").expect("seed"); + let harness = editor_with(answering(), ""); + open_path(&harness, &path); + let window = shown(&harness); + let buffer = harness.editor.buffer(); + buffer.insert(&mut buffer.end_iter(), "\nSELECT 2;"); + let context = vim(&harness); + + assert!(vim_command(&context, ":e!")); + wait_until(WAIT, || buffer_text(&buffer) == "SELECT 1;").expect("the file again"); + + assert!(crate::workspace::visible_alert(&window).is_none(), "it asked"); + assert!(!harness.editor.has_unsaved_file_changes()); +} + +#[gtk4::test] +fn vim_edit_asks_before_it_drops_changes_to_the_file() { + let root = tempfile::tempdir().expect("tempdir"); + let path = root.path().join("daily.sql"); + std::fs::write(&path, "SELECT 1;").expect("seed"); + let harness = editor_with(answering(), ""); + open_path(&harness, &path); + let window = shown(&harness); + let buffer = harness.editor.buffer(); + buffer.insert(&mut buffer.end_iter(), "\nSELECT 2;"); + let context = vim(&harness); + + assert!(vim_command(&context, ":e")); + wait_until(WAIT, || crate::workspace::visible_alert(&window).is_some()).expect("the question"); + assert_eq!(buffer_text(&buffer), "SELECT 1;\nSELECT 2;"); + assert!(crate::workspace::answer(&window, "replace")); + + wait_until(WAIT, || buffer_text(&buffer) == "SELECT 1;").expect("the file again"); +} + +#[gtk4::test] +fn vim_edit_reads_the_tabs_file_again() { + let root = tempfile::tempdir().expect("tempdir"); + let path = root.path().join("daily.sql"); + std::fs::write(&path, "SELECT 1;").expect("seed"); + let harness = editor_with(answering(), ""); + open_path(&harness, &path); + std::fs::write(&path, "SELECT 2;").expect("change it on disk"); + let context = vim(&harness); + + assert!(vim_command(&context, ":e")); + let buffer = harness.editor.buffer(); + wait_until(WAIT, || buffer_text(&buffer) == "SELECT 2;").expect("the file again"); + + assert!(!harness.editor.has_unsaved_file_changes()); +} + +#[gtk4::test] +fn vim_edit_without_a_file_says_so() { + let harness = editor_with(answering(), "SELECT 1;"); + let context = vim(&harness); + let toasts = SignalLog::connect(&harness.editor, "toast", |values| { + values[1].get::().expect("a message") + }); + + assert!(vim_command(&context, ":e")); + + assert_eq!(toasts.take(), ["This tab is not saved to a file."]); + assert_eq!(buffer_text(&harness.editor.buffer()), "SELECT 1;"); +} + +#[gtk4::test] +fn vim_edit_a_name_opens_that_file_in_an_empty_tab() { + let root = tempfile::tempdir().expect("tempdir"); + let path = root.path().join("daily.sql"); + std::fs::write(&path, "SELECT 3;").expect("seed"); + let harness = editor_with(answering(), ""); + let context = vim(&harness); + + assert!(vim_command(&context, &format!(":e {}", path.display()))); + let buffer = harness.editor.buffer(); + wait_until(WAIT, || buffer_text(&buffer) == "SELECT 3;").expect("the file"); + + assert_eq!(harness.editor.file().and_then(|file| file.path()), Some(path)); +} + +#[gtk4::test] +fn vim_edit_a_name_asks_before_it_replaces_the_tabs_text() { + let root = tempfile::tempdir().expect("tempdir"); + let path = root.path().join("daily.sql"); + std::fs::write(&path, "SELECT 3;").expect("seed"); + let harness = editor_with(answering(), "SELECT 1;"); + let window = shown(&harness); + let context = vim(&harness); + let buffer = harness.editor.buffer(); + + assert!(vim_command(&context, &format!(":e {}", path.display()))); + wait_until(WAIT, || crate::workspace::visible_alert(&window).is_some()).expect("the question"); + assert_eq!(buffer_text(&buffer), "SELECT 1;", "the text went before the answer"); + assert!(crate::workspace::answer(&window, "replace")); + + wait_until(WAIT, || buffer_text(&buffer) == "SELECT 3;").expect("the file"); + harness.editor.undo(); + assert_eq!(buffer_text(&buffer), "SELECT 1;"); +} + fn find_bar(harness: &Harness) -> find_bar::FindBar { harness.editor.imp().find_bar.get() } @@ -1558,4 +1890,179 @@ mod real_keys { wait_until(Duration::from_secs(2), || !bar.is_open() && view.has_focus()) .expect("Escape to close the bar and return to the view"); } + + const KEY_WAIT: Duration = Duration::from_secs(2); + + /// An editor in Vim's normal mode with the cursor at the start, + /// shown and holding the keyboard focus. + fn in_vim(harness: &Harness) -> (InWindow, crate::test_support::KeyInjector) { + vim(harness); + let window = InWindow::presented(&harness.editor); + let buffer = harness.editor.buffer(); + buffer.place_cursor(&buffer.start_iter()); + harness.editor.imp().source_view.grab_focus(); + (window, crate::test_support::KeyInjector::connect().expect("an X11 display")) + } + + fn press(keys: &crate::test_support::KeyInjector, key: gtk4::gdk::Key) { + keys.press(key, gtk4::gdk::ModifierType::empty()).expect("the key press"); + } + + /// A key typed with Shift, such as `:` or `G` on a US layout. + fn shifted(keys: &crate::test_support::KeyInjector, key: gtk4::gdk::Key) { + keys.press(key, gtk4::gdk::ModifierType::SHIFT_MASK) + .expect("the key press"); + } + + fn ctrl(keys: &crate::test_support::KeyInjector, key: gtk4::gdk::Key) { + keys.press(key, gtk4::gdk::ModifierType::CONTROL_MASK) + .expect("the key press"); + } + + #[gtk4::test] + fn vim_takes_normal_insert_and_visual_mode_keys() { + use gtk4::gdk::Key; + let harness = editor_with(answering(), "select 1"); + let (_window, keys) = in_vim(&harness); + let buffer = harness.editor.buffer(); + let mode = harness.editor.imp().vim_command_bar.get(); + + press(&keys, Key::x); + wait_until(KEY_WAIT, || buffer_text(&buffer) == "elect 1").expect("x to delete a character"); + + press(&keys, Key::i); + wait_until(KEY_WAIT, || !mode.label().is_empty()).expect("i to show insert mode"); + press(&keys, Key::s); + wait_until(KEY_WAIT, || buffer_text(&buffer) == "select 1").expect("insert mode to type"); + press(&keys, Key::Escape); + wait_until(KEY_WAIT, || mode.label().is_empty()).expect("Escape to leave insert mode"); + + press(&keys, Key::v); + press(&keys, Key::e); + wait_until(KEY_WAIT, || buffer.has_selection() && !mode.label().is_empty()) + .expect("v and e to select a word in visual mode"); + press(&keys, Key::Escape); + wait_until(KEY_WAIT, || mode.label().is_empty()).expect("Escape to leave visual mode"); + } + + #[gtk4::test] + fn the_run_keys_run_from_vims_normal_and_insert_modes() { + use gtk4::gdk::Key; + let harness = editor_with(answering(), "select 1"); + let (_window, keys) = in_vim(&harness); + + ctrl(&keys, Key::Return); + wait_until(WAIT, || sent(&harness.fake) == ["select 1"]).expect("Ctrl+Return to run in normal mode"); + wait_until(WAIT, || !harness.editor.is_running()).expect("the run to end"); + + press(&keys, Key::i); + keys.press( + Key::Return, + gtk4::gdk::ModifierType::CONTROL_MASK | gtk4::gdk::ModifierType::SHIFT_MASK, + ) + .expect("the key press"); + wait_until(WAIT, || sent(&harness.fake).len() == 2).expect("Ctrl+Shift+Return to run in insert mode"); + assert_eq!(buffer_text(&harness.editor.buffer()), "select 1", "Return went into the text"); + } + + /// Normal mode binds Ctrl+F to a page down, and the command line + /// takes every key. + #[gtk4::test] + fn find_and_comment_keep_their_keys_in_vim() { + use gtk4::gdk::Key; + if let Some(settings) = gtk4::Settings::default() { + settings.set_gtk_enable_animations(false); + } + let harness = editor_with(answering(), "select 1"); + let (_window, keys) = in_vim(&harness); + let view = harness.editor.imp().source_view.get(); + let buffer = harness.editor.buffer(); + + ctrl(&keys, Key::slash); + wait_until(KEY_WAIT, || buffer_text(&buffer) == "-- select 1").expect("Ctrl+/ to comment the line"); + + shifted(&keys, Key::colon); + ctrl(&keys, Key::f); + let bar = find_bar(&harness); + wait_until(KEY_WAIT, || bar.is_open()).expect("Ctrl+F to open the find bar from the command line"); + assert!(!view.has_focus(), "the view kept the focus"); + } + + #[gtk4::test] + fn escape_is_vims_while_a_query_runs() { + use gtk4::gdk::Key; + let mut runs = Replies::default(); + runs.always(FakeRun::until_stopped()); + let harness = editor_with(runs, "select 1"); + let (_window, keys) = in_vim(&harness); + let mode = harness.editor.imp().vim_command_bar.get(); + harness.editor.run(); + assert!(harness.editor.is_running()); + + press(&keys, Key::i); + wait_until(KEY_WAIT, || !mode.label().is_empty()).expect("insert mode"); + press(&keys, Key::Escape); + wait_until(KEY_WAIT, || mode.label().is_empty()).expect("Escape to leave insert mode"); + + assert!(harness.editor.is_running(), "Escape stopped the query"); + harness.editor.stop(); + wait_until(WAIT, || !harness.editor.is_running()).expect("the Stop button's stop"); + } + + #[gtk4::test] + fn typing_colon_w_saves_and_colon_q_asks_to_close() { + use gtk4::gdk::Key; + let root = tempfile::tempdir().expect("tempdir"); + let path = root.path().join("daily.sql"); + std::fs::write(&path, "SELECT 1;").expect("seed"); + let harness = editor_with(answering(), ""); + open_path(&harness, &path); + let buffer = harness.editor.buffer(); + buffer.insert(&mut buffer.end_iter(), "\nSELECT 2;"); + let (_window, keys) = in_vim(&harness); + let closes = SignalLog::connect(&harness.editor, "close-requested", |_| ()); + + shifted(&keys, Key::colon); + press(&keys, Key::w); + press(&keys, Key::Return); + wait_until(WAIT, || !harness.editor.has_unsaved_file_changes()).expect(":w to save"); + assert_eq!( + std::fs::read_to_string(&path).expect("read back"), + "SELECT 1;\nSELECT 2;" + ); + + shifted(&keys, Key::colon); + press(&keys, Key::q); + press(&keys, Key::Return); + wait_until(KEY_WAIT, || closes.count() == 1).expect(":q to ask for the close"); + } + + #[gtk4::test] + fn escape_from_insert_mode_closes_the_completion_list() { + use gtk4::gdk::Key; + let harness = editor_with(answering(), "SELECT 1;\n"); + let (_window, keys) = in_vim(&harness); + let view = harness.editor.imp().source_view.get(); + let shown_popup = || { + let mut child = view.first_child(); + while let Some(widget) = child { + if widget.is::() && widget.is_visible() { + return true; + } + child = widget.next_sibling(); + } + false + }; + + shifted(&keys, Key::G); + press(&keys, Key::o); + shifted(&keys, Key::S); + shifted(&keys, Key::E); + view.completion().show(); + wait_until(KEY_WAIT, shown_popup).expect("the completion list"); + + press(&keys, Key::Escape); + + wait_until(KEY_WAIT, || !shown_popup()).expect("Escape to close the list"); + } } diff --git a/linux/crates/app/src/ui/editor/vim_mode.rs b/linux/crates/app/src/ui/editor/vim_mode.rs new file mode 100644 index 0000000000..bd0428b27a --- /dev/null +++ b/linux/crates/app/src/ui/editor/vim_mode.rs @@ -0,0 +1,391 @@ +use std::path::PathBuf; +use std::rc::Rc; + +use gtk4::prelude::*; +use gtk4::subclass::prelude::*; +use gtk4::{gdk, gio, glib}; +use tablepro_storage::AppSettings; +use tablepro_storage::settings::keys; + +use super::SqlEditor; +use crate::ui::handler_guard::HandlerGuard; + +pub(crate) struct VimMode { + context: sourceview5::VimIMContext, + keys: gtk4::EventControllerKey, + bindings: [glib::Binding; 2], +} + +/// GtkSourceView runs `:w` and `:e` itself and leaves these to the +/// application. +#[derive(Debug, Clone, PartialEq, Eq)] +enum TabCommand { + /// `:q!` too: the close prompts also guard an open transaction, which + /// `!` says nothing about. + Close, + WriteAndClose, + /// `:x` writes only when the file has changes. + WriteChangesAndClose, + /// `:w!` may replace another file. + ForceWrite(Option), + /// `:e!` drops the tab's text without asking. + ForceEdit(Option), +} + +impl TabCommand { + fn parse(command: &str) -> Option { + let line = command.strip_prefix(':')?.trim(); + let (name, argument) = match line.split_once(char::is_whitespace) { + Some((name, rest)) => (name, Some(rest.trim().to_owned())), + None => (line, None), + }; + let (name, bang) = match name.strip_suffix('!') { + Some(name) => (name, true), + None => (name, false), + }; + match (name, bang, argument) { + ("q" | "qu" | "qui" | "quit", _, None) => Some(Self::Close), + ("wq", _, None) => Some(Self::WriteAndClose), + ("x" | "xi" | "xit" | "exi" | "exit", _, None) => Some(Self::WriteChangesAndClose), + ("w" | "write", true, argument) => Some(Self::ForceWrite(argument)), + ("e" | "edit", true, argument) => Some(Self::ForceEdit(argument)), + _ => None, + } + } +} + +impl SqlEditor { + pub(super) fn follow_vim_preference(&self, settings: &Rc) -> HandlerGuard { + self.set_vim(settings.vim_keybindings()); + let editor = self.downgrade(); + let handler = settings + .gio() + .connect_changed(Some(keys::VIM_KEYBINDINGS), move |settings, key| { + if let Some(editor) = editor.upgrade() { + editor.set_vim(settings.boolean(key)); + } + }); + HandlerGuard::new(settings.gio(), handler) + } + + pub(super) fn has_vim(&self) -> bool { + self.imp().vim.borrow().is_some() + } + + #[cfg(test)] + pub(super) fn vim_context(&self) -> Option { + self.imp().vim.borrow().as_ref().map(|vim| vim.context.clone()) + } + + fn set_vim(&self, on: bool) { + match (on, self.has_vim()) { + (true, false) => self.attach_vim(), + (false, true) => self.detach_vim(), + _ => {} + } + } + + fn attach_vim(&self) { + let imp = self.imp(); + let view = imp.source_view.get(); + let context = sourceview5::VimIMContext::new(); + context.set_client_widget(Some(&view)); + let bindings = [ + context + .bind_property("command-bar-text", &*imp.vim_command_bar, "label") + .sync_create() + .build(), + context + .bind_property("command-text", &*imp.vim_command, "label") + .sync_create() + .build(), + ]; + self.connect_vim_commands(&context); + + let keys = gtk4::EventControllerKey::new(); + keys.set_name(Some("tablepro-vim")); + keys.set_propagation_phase(gtk4::PropagationPhase::Capture); + let filter = context.clone(); + let popups = view.downgrade(); + keys.connect_key_pressed(move |keys, keyval, _, state| { + if crate::application::editor_keeps(keyval, state) { + return glib::Propagation::Proceed; + } + let Some(event) = keys.current_event() else { + return glib::Propagation::Proceed; + }; + if !filter.filter_keypress(&event) { + return glib::Propagation::Proceed; + } + // Vim takes Escape before the view, whose own Escape hides + // the completion list. + if leaves_insert(keyval, state) + && let Some(view) = popups.upgrade() + { + super::editor_actions::close_view_popups(&view); + } + glib::Propagation::Stop + }); + // GTK runs a widget's last controller first, so this one runs + // before the editor's own keys, which it lets through. + view.add_controller(keys.clone()); + + imp.text_pane.set_reveal_bottom_bars(true); + // Escape is Vim's now; the Stop button still stops a run. + imp.stop_button.reset_property(gtk4::AccessibleProperty::KeyShortcuts); + imp.vim.replace(Some(VimMode { + context, + keys, + bindings, + })); + } + + pub(super) fn detach_vim(&self) { + let imp = self.imp(); + let Some(vim) = imp.vim.take() else { + return; + }; + let view = imp.source_view.get(); + view.remove_controller(&vim.keys); + for binding in vim.bindings { + binding.unbind(); + } + vim.context.reset(); + vim.context.set_client_widget(None::<>k4::Widget>); + // Normal mode draws a block cursor through overwrite, and the + // command line hides the cursor. + view.set_overwrite(false); + view.set_cursor_visible(true); + imp.vim_command_bar.set_label(""); + imp.vim_command.set_label(""); + imp.text_pane.set_reveal_bottom_bars(false); + crate::ui::a11y::announce_key_shortcuts(&imp.stop_button.get(), "editor.cancel"); + } + + fn connect_vim_commands(&self, context: &sourceview5::VimIMContext) { + let editor = self.downgrade(); + context.connect_execute_command(move |_, command| { + let (Some(editor), Some(command)) = (editor.upgrade(), TabCommand::parse(command)) else { + return false; + }; + editor.run_tab_command(command); + true + }); + let editor = self.downgrade(); + context.connect_write(move |_, _, path| { + if let Some(editor) = editor.upgrade() { + match path { + Some(path) => editor.write_named(path, false), + None => editor.save(), + } + } + }); + let editor = self.downgrade(); + context.connect_edit(move |_, _, path| { + if let Some(editor) = editor.upgrade() { + match path { + Some(path) => editor.edit_named(path, false), + None => editor.reload_file(false), + } + } + }); + } + + fn run_tab_command(&self, command: TabCommand) { + match command { + TabCommand::Close => self.request_close(), + TabCommand::WriteChangesAndClose if self.file().is_some() && !self.has_unsaved_file_changes() => { + self.request_close(); + } + TabCommand::WriteAndClose | TabCommand::WriteChangesAndClose => { + self.save_then(SqlEditor::request_close); + } + TabCommand::ForceWrite(None) => self.save(), + TabCommand::ForceWrite(Some(path)) => self.write_named(&path, true), + TabCommand::ForceEdit(None) => self.reload_file(true), + TabCommand::ForceEdit(Some(path)) => self.edit_named(&path, true), + } + } + + fn request_close(&self) { + self.emit_by_name::<()>("close-requested", &[]); + } + + pub(crate) fn connect_close_requested(&self, f: F) -> glib::SignalHandlerId { + self.connect_closure( + "close-requested", + false, + glib::closure_local!(move |editor: &Self| f(editor)), + ) + } + + /// Save As to the named file. Unlike Save As it cannot ask, so + /// without `!` it leaves another file of that name alone, as Vim does. + fn write_named(&self, typed: &str, replace: bool) { + let file = typed_file(typed, self.file().as_ref()); + let editor = self.clone(); + glib::spawn_future_local(async move { + let own = editor.file().is_some_and(|tab| tab.equal(&file)); + if !replace && !own && exists(&file).await { + editor.emit_by_name::<()>( + "toast", + &[&crate::i18n::gettext_f( + "“{name}” already exists. Use :w! to replace it.", + &[("name", &super::file_name(&file))], + )], + ); + return; + } + editor.write_reporting(&file).await; + }); + } + + fn edit_named(&self, typed: &str, force: bool) { + let file = typed_file(typed, self.file().as_ref()); + if force { + self.load_file(file); + } else { + self.replace_after_asking(move |editor| editor.load_file(file.clone())); + } + } + + fn reload_file(&self, force: bool) { + let Some(file) = self.file() else { + self.emit_by_name::<()>( + "toast", + &[&crate::i18n::gettext("This tab is not saved to a file.")], + ); + return; + }; + if force || !self.has_unsaved_file_changes() { + self.load_file(file); + } else { + self.replace_after_asking(move |editor| editor.load_file(file.clone())); + } + } + + fn load_file(&self, file: gio::File) { + let editor = self.clone(); + glib::spawn_future_local(async move { editor.insert_file(file, true).await }); + } +} + +fn leaves_insert(keyval: gdk::Key, state: gdk::ModifierType) -> bool { + let control = state.contains(gdk::ModifierType::CONTROL_MASK); + keyval == gdk::Key::Escape || (control && matches!(keyval, gdk::Key::bracketleft | gdk::Key::c)) +} + +/// A relative name is next to the tab's file, or in the home folder +/// when the tab has none, never in the app's working folder. +fn typed_file(typed: &str, tab_file: Option<&gio::File>) -> gio::File { + let home = glib::home_dir(); + let expanded = match typed.strip_prefix('~') { + Some("") => home.clone(), + Some(rest) if rest.starts_with('/') => home.join(rest.trim_start_matches('/')), + _ => PathBuf::from(typed), + }; + let folder = tab_file + .and_then(|file| file.parent()) + .and_then(|parent| parent.path()) + .unwrap_or(home); + gio::File::for_commandline_arg_and_cwd(expanded, folder) +} + +async fn exists(file: &gio::File) -> bool { + file.query_info_future( + gio::FILE_ATTRIBUTE_STANDARD_TYPE, + gio::FileQueryInfoFlags::NONE, + glib::Priority::DEFAULT, + ) + .await + .is_ok() +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn the_quit_and_write_commands_act_on_the_tab() { + for (typed, command) in [ + (":q", TabCommand::Close), + (":quit", TabCommand::Close), + (":qu", TabCommand::Close), + (":q!", TabCommand::Close), + (":quit!", TabCommand::Close), + (":q ", TabCommand::Close), + (":wq", TabCommand::WriteAndClose), + (":wq!", TabCommand::WriteAndClose), + (":x", TabCommand::WriteChangesAndClose), + (":xit", TabCommand::WriteChangesAndClose), + (":exit", TabCommand::WriteChangesAndClose), + (":x!", TabCommand::WriteChangesAndClose), + ] { + assert_eq!(TabCommand::parse(typed), Some(command), "{typed}"); + } + } + + #[test] + fn the_bang_forms_of_write_and_edit_act_on_the_tab() { + for (typed, command) in [ + (":w!", TabCommand::ForceWrite(None)), + (":write!", TabCommand::ForceWrite(None)), + (":w! a.sql", TabCommand::ForceWrite(Some("a.sql".to_owned()))), + (":w! my query.sql ", TabCommand::ForceWrite(Some("my query.sql".to_owned()))), + (":e!", TabCommand::ForceEdit(None)), + (":edit!", TabCommand::ForceEdit(None)), + (":e! a.sql", TabCommand::ForceEdit(Some("a.sql".to_owned()))), + ] { + assert_eq!(TabCommand::parse(typed), Some(command), "{typed}"); + } + } + + #[test] + fn other_commands_stay_with_gtksourceview() { + for typed in [ + ":w", ":write", ":w a.sql", ":e", ":e a.sql", ":set nu", ":s/a/b/", "q", ":", ":qa", ":ex", ":q a.sql", + ] { + assert_eq!(TabCommand::parse(typed), None, "{typed}"); + } + } + + #[test] + fn a_typed_name_is_next_to_the_tabs_file() { + let tab = gio::File::for_path("/tmp/reports/daily.sql"); + + assert_eq!( + typed_file("weekly.sql", Some(&tab)).path(), + Some(PathBuf::from("/tmp/reports/weekly.sql")) + ); + assert_eq!( + typed_file("../all.sql", Some(&tab)).path(), + Some(PathBuf::from("/tmp/all.sql")) + ); + assert_eq!( + typed_file("/srv/q.sql", Some(&tab)).path(), + Some(PathBuf::from("/srv/q.sql")) + ); + } + + #[test] + fn a_typed_name_without_a_tab_file_is_in_the_home_folder() { + let home = glib::home_dir(); + + assert_eq!(typed_file("q.sql", None).path(), Some(home.join("q.sql"))); + assert_eq!(typed_file("~/sql/q.sql", None).path(), Some(home.join("sql/q.sql"))); + assert_eq!( + typed_file("~/q.sql", Some(&gio::File::for_path("/tmp/a.sql"))).path(), + Some(home.join("q.sql")) + ); + } + + #[test] + fn escape_and_its_control_twins_leave_insert() { + let ctrl = gdk::ModifierType::CONTROL_MASK; + assert!(leaves_insert(gdk::Key::Escape, gdk::ModifierType::empty())); + assert!(leaves_insert(gdk::Key::bracketleft, ctrl)); + assert!(leaves_insert(gdk::Key::c, ctrl)); + assert!(!leaves_insert(gdk::Key::c, gdk::ModifierType::empty())); + assert!(!leaves_insert(gdk::Key::bracketleft, gdk::ModifierType::empty())); + } +} diff --git a/linux/crates/app/src/ui/preferences_dialog/editor_page.rs b/linux/crates/app/src/ui/preferences_dialog/editor_page.rs index 4898847d59..0e0d944317 100644 --- a/linux/crates/app/src/ui/preferences_dialog/editor_page.rs +++ b/linux/crates/app/src/ui/preferences_dialog/editor_page.rs @@ -27,6 +27,8 @@ mod imp { #[template_child] pub style_scheme_box: TemplateChild, #[template_child] + pub vim_row: TemplateChild, + #[template_child] pub timeout_row: TemplateChild, } @@ -51,7 +53,8 @@ mod imp { } glib::wrapper! { - /// The Editor page: the font, the style scheme and the query timeout. + /// The Editor page: the font, the style scheme, Vim keybindings and + /// the query timeout. pub struct EditorPage(ObjectSubclass) @extends adw::PreferencesPage, gtk4::Widget, @implements gtk4::Accessible, gtk4::Buildable, gtk4::ConstraintTarget; @@ -74,6 +77,9 @@ impl EditorPage { gio_settings .bind(keys::USE_SYSTEM_FONT, &imp.system_font_row.get(), "active") .build(); + gio_settings + .bind(keys::VIM_KEYBINDINGS, &imp.vim_row.get(), "active") + .build(); // The font row is only reachable when the system font is off, so // the switch drives its sensitivity with the value inverted. @@ -132,6 +138,7 @@ mod tests { [ "Use the System Monospace Font", "Editor font", + "Vim Keybindings", "Query timeout (seconds)" ] ); @@ -161,6 +168,18 @@ mod tests { ); } + #[gtk4::test] + fn the_vim_row_is_bound_and_off_by_default() { + let settings = MemorySettings::new(); + let page = bound_page(&settings); + let row = page.imp().vim_row.get(); + + assert!(!row.is_active()); + row.set_active(true); + + assert!(settings.get().vim_keybindings()); + } + #[gtk4::test] fn the_timeout_row_is_bound() { let settings = MemorySettings::new(); diff --git a/linux/crates/app/src/ui/workspace/mod.rs b/linux/crates/app/src/ui/workspace/mod.rs index 51ba750807..52f8c25362 100644 --- a/linux/crates/app/src/ui/workspace/mod.rs +++ b/linux/crates/app/src/ui/workspace/mod.rs @@ -478,6 +478,8 @@ pub(crate) enum WorkspaceInput { outcome: crate::workspace::WorkOutcome, }, WorkspaceTabClosed(Uuid), + /// Close the tab as its close button does, prompts and all. + CloseWorkspaceTab(Uuid), /// Each tab still asks about its own unsaved changes. CloseOtherWorkspaceTabs(Uuid), CloseWorkspaceTabsToRight(Uuid), @@ -1005,6 +1007,7 @@ impl WorkspaceModel { } WorkspaceInput::ShowTransactionTab => self.show_transaction_tab(), WorkspaceInput::WorkspaceTabClosed(id) => self.close_workspace_tab_by_id(id), + WorkspaceInput::CloseWorkspaceTab(id) => self.close_workspace_tab(id), WorkspaceInput::CloseOtherWorkspaceTabs(id) => self.close_other_workspace_tabs(id), WorkspaceInput::CloseWorkspaceTabsToRight(id) => self.close_workspace_tabs_to_right(id), WorkspaceInput::CloseActiveWorkspaceTab => self.close_active_workspace_tab(), diff --git a/linux/crates/app/src/ui/workspace/tab_factory.rs b/linux/crates/app/src/ui/workspace/tab_factory.rs index ede0a9f16a..95be7738e4 100644 --- a/linux/crates/app/src/ui/workspace/tab_factory.rs +++ b/linux/crates/app/src/ui/workspace/tab_factory.rs @@ -345,6 +345,8 @@ pub(super) fn connect_editor( editor.connect_transaction_changed(move |_, _| transaction.deliver(WorkspaceInput::EditorTransactionChanged)); let toast = input.clone(); editor.connect_toast(move |_, text| toast.deliver(WorkspaceInput::ShowToast(text))); + let close = input.clone(); + editor.connect_close_requested(move |_| close.deliver(WorkspaceInput::CloseWorkspaceTab(tab_id))); let export = input.clone(); editor.connect_export_results(move |_, offer| { export.deliver(WorkspaceInput::ExportResults { diff --git a/linux/crates/app/src/ui/workspace/tests/close_tab.rs b/linux/crates/app/src/ui/workspace/tests/close_tab.rs index 82e8320a19..c81fcf8bc7 100644 --- a/linux/crates/app/src/ui/workspace/tests/close_tab.rs +++ b/linux/crates/app/src/ui/workspace/tests/close_tab.rs @@ -252,3 +252,53 @@ fn closing_the_window_keeps_unsaved_file_changes_without_asking() { wait_until(WAIT, || test.app.windows().is_empty()).expect("the window closes"); } + +/// Vim's `:q` closes its own tab through the prompts Ctrl+W meets. +#[gtk4::test] +fn an_editor_asking_to_close_goes_through_the_close_prompts() { + let test = TestWindow::connected("VimQuitUnsavedFile"); + let path = test.dir().join("daily.sql"); + crate::test_support::write_text_file(&path, "SELECT 1;"); + let (id, editor) = editor_on_file(&test, &path); + + editor.emit_by_name::<()>("close-requested", &[]); + + wait_until(WAIT, || { + visible_alert(&test.window).is_some_and(|alert| alert.heading().as_deref() == Some("Save Changes?")) + }) + .expect("the close asks about the file"); + assert!(answer(&test.window, "discard")); + wait_until(WAIT, || !tab_is_open(&test, id)).expect("the tab closes"); + assert_eq!(crate::test_support::read_text_file(&path), "SELECT 1;"); +} + +/// A `:wq` whose write lands after the user moved to another tab still +/// closes the tab it was typed in. +#[gtk4::test] +fn an_editor_asking_to_close_closes_its_own_tab_not_the_one_in_front() { + let test = TestWindow::connected("VimQuitBackgroundTab"); + let workspace = test.workspace(); + let open_editor = |query: &str| { + workspace.with_model(|model| { + model.open_tab(super::super::tab_spec::TabSpec::Editor { + initial_query: Some(query.to_owned()), + draft: None, + file: None, + }) + }); + let page = workspace.imp().tab_view.selected_page().expect("the editor in front"); + let editor = page + .child() + .downcast::() + .expect("an editor tab"); + (workspace.selected_tab().expect("its id"), editor) + }; + let (behind, editor) = open_editor("SELECT 1;"); + let (front, _) = open_editor("SELECT 2;"); + + editor.emit_by_name::<()>("close-requested", &[]); + + wait_until(WAIT, || !tab_is_open(&test, behind)).expect("the tab that asked closes"); + assert!(tab_is_open(&test, front)); + assert_eq!(workspace.selected_tab(), Some(front)); +} diff --git a/linux/crates/app/src/ui/workspace/workspace_tabs.rs b/linux/crates/app/src/ui/workspace/workspace_tabs.rs index d0333dec9e..d9e6cc38da 100644 --- a/linux/crates/app/src/ui/workspace/workspace_tabs.rs +++ b/linux/crates/app/src/ui/workspace/workspace_tabs.rs @@ -225,6 +225,15 @@ impl WorkspaceModel { self.report_tabs_changed(); } + /// The view's close-page handler takes it from there, as for the + /// tab's close button. A tab already gone has nothing to close. + pub(super) fn close_workspace_tab(&mut self, id: Uuid) { + let page = self.workspace_tabs.borrow().get(&id).map(|tab| tab.page().clone()); + if let Some(page) = page { + self.tab_view.close_page(&page); + } + } + /// With no tab open, Ctrl+W closes the window, as in GNOME Text /// Editor. pub(super) fn close_active_workspace_tab(&mut self) { diff --git a/linux/crates/storage/src/settings/keys.rs b/linux/crates/storage/src/settings/keys.rs index a61977484f..6f98ca6fec 100644 --- a/linux/crates/storage/src/settings/keys.rs +++ b/linux/crates/storage/src/settings/keys.rs @@ -6,6 +6,7 @@ pub const QUERY_TIMEOUT_SECS: &str = "query-timeout-secs"; pub const USE_SYSTEM_FONT: &str = "use-system-font"; pub const CUSTOM_FONT: &str = "custom-font"; pub const STYLE_SCHEME: &str = "style-scheme"; +pub const VIM_KEYBINDINGS: &str = "vim-keybindings"; pub const CONNECT_DIALOG_DRIVER: &str = "connect-dialog-driver"; pub const RESTORE_SESSION: &str = "restore-session"; diff --git a/linux/crates/storage/src/settings/mod.rs b/linux/crates/storage/src/settings/mod.rs index b0265ae492..9bbf4efd9a 100644 --- a/linux/crates/storage/src/settings/mod.rs +++ b/linux/crates/storage/src/settings/mod.rs @@ -133,6 +133,15 @@ impl AppSettings { self.set_string(keys::STYLE_SCHEME, scheme) } + /// Whether the SQL editor runs Vim's modes and commands. + pub fn vim_keybindings(&self) -> bool { + self.settings.boolean(keys::VIM_KEYBINDINGS) + } + + pub fn set_vim_keybindings(&self, on: bool) -> Result<(), SettingsError> { + self.set_boolean(keys::VIM_KEYBINDINGS, on) + } + /// The driver the connect dialog opens on. Remembered so the /// common case is one click, not a combo hunt every time. pub fn connect_dialog_driver(&self) -> String { diff --git a/linux/crates/storage/tests/settings.rs b/linux/crates/storage/tests/settings.rs index 71c613e9df..df61199d8c 100644 --- a/linux/crates/storage/tests/settings.rs +++ b/linux/crates/storage/tests/settings.rs @@ -50,6 +50,18 @@ fn defaults_match_schema() -> TestResult { assert_eq!(fixture.settings.csv_options(), CsvOptions::default()); assert!(fixture.settings.restore_session()); assert!(fixture.settings.save_query_history()); + assert!(!fixture.settings.vim_keybindings()); + Ok(()) +} + +#[test] +fn vim_keybindings_round_trip() -> TestResult { + let fixture = Fixture::new()?; + + fixture.settings.set_vim_keybindings(true)?; + assert!(fixture.settings.vim_keybindings()); + fixture.settings.set_vim_keybindings(false)?; + assert!(!fixture.settings.vim_keybindings()); Ok(()) } diff --git a/linux/data/app.tablepro.TablePro.gschema.xml.in b/linux/data/app.tablepro.TablePro.gschema.xml.in index fecf7c4d5c..ca11e2c910 100644 --- a/linux/data/app.tablepro.TablePro.gschema.xml.in +++ b/linux/data/app.tablepro.TablePro.gschema.xml.in @@ -96,6 +96,12 @@ GtkSourceView style scheme id for the SQL editor. + + false + Vim keybindings in the SQL editor + Whether the SQL editor takes Vim's modes and commands. The editor's own Ctrl shortcuts keep working in every mode. + + true Restore session diff --git a/linux/data/resources/ui/preferences-editor-page.ui b/linux/data/resources/ui/preferences-editor-page.ui index bc7db020e2..83b249ab73 100644 --- a/linux/data/resources/ui/preferences-editor-page.ui +++ b/linux/data/resources/ui/preferences-editor-page.ui @@ -43,6 +43,17 @@ + + + Keyboard + + + Vim Keybindings + Normal, insert and visual modes, and commands such as :w and :q + + + + Running Queries diff --git a/linux/data/resources/ui/sql-editor.ui b/linux/data/resources/ui/sql-editor.ui index b9bdca096c..7ccff62bf7 100644 --- a/linux/data/resources/ui/sql-editor.ui +++ b/linux/data/resources/ui/sql-editor.ui @@ -110,27 +110,66 @@ true false - - 200 - - - true - true - true - true - 4 - 8 - 8 - 8 - 8 - + + false + + + 200 + + + true + true + true + true + 4 + 8 + 8 + 8 + 8 + + + SQL editor + + + + + + + + status + 12 + 4 + 4 + 12 + 12 - SQL editor + Vim mode + + + true + 0 + end + + + + + + 1 + 8 + start + + + - + diff --git a/linux/docs/storage.md b/linux/docs/storage.md index 9feeed2998..5459e61e9d 100644 --- a/linux/docs/storage.md +++ b/linux/docs/storage.md @@ -267,6 +267,7 @@ The schema id is the application id and the path follows it: `app.tablepro.Table | `use-system-font` | `b` | `true` | | | `custom-font` | `s` | `Monospace 12` | | | `style-scheme` | `s` | `Adwaita` | | +| `vim-keybindings` | `b` | `false` | | | `csv-null-to-empty` | `b` | `true` | | | `csv-line-break-to-space` | `b` | `false` | | | `csv-header-row` | `b` | `true` | | diff --git a/linux/po/POTFILES.in b/linux/po/POTFILES.in index 8d0eff6045..6894dd3db4 100644 --- a/linux/po/POTFILES.in +++ b/linux/po/POTFILES.in @@ -229,6 +229,7 @@ crates/app/src/ui/editor/sql_file.rs crates/app/src/ui/editor/statement_cursor.rs crates/app/src/ui/editor/tests.rs crates/app/src/ui/editor/transaction_dirty.rs +crates/app/src/ui/editor/vim_mode.rs crates/app/src/ui/editor_font_provider.rs crates/app/src/ui/error_text/ddl.rs crates/app/src/ui/error_text/driver.rs From 275997ffc5c3d142996a7c78291f55d1142891ad Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Fri, 23 Oct 2026 08:19:43 +0700 Subject: [PATCH 02/11] fix(linux-editor): ask before :e drops an emptied file and keep typed names next to a remote file --- linux/crates/app/src/ui/editor/mod.rs | 6 +++- linux/crates/app/src/ui/editor/tests.rs | 18 +++++++++++ linux/crates/app/src/ui/editor/vim_mode.rs | 37 +++++++++++++++++----- 3 files changed, 52 insertions(+), 9 deletions(-) diff --git a/linux/crates/app/src/ui/editor/mod.rs b/linux/crates/app/src/ui/editor/mod.rs index 29f6319838..056da64a7a 100644 --- a/linux/crates/app/src/ui/editor/mod.rs +++ b/linux/crates/app/src/ui/editor/mod.rs @@ -868,8 +868,12 @@ impl SqlEditor { fn replace_after_asking(&self, replace: impl Fn(&SqlEditor) + 'static) { if buffer_text(&self.buffer()).trim().is_empty() { replace(self); - return; + } else { + self.ask_to_replace(replace); } + } + + fn ask_to_replace(&self, replace: impl Fn(&SqlEditor) + 'static) { let dialog = adw::AlertDialog::new( Some(&crate::i18n::gettext("Replace the Editor Contents?")), Some(&crate::i18n::gettext( diff --git a/linux/crates/app/src/ui/editor/tests.rs b/linux/crates/app/src/ui/editor/tests.rs index 33ee65b5d8..2a42aef872 100644 --- a/linux/crates/app/src/ui/editor/tests.rs +++ b/linux/crates/app/src/ui/editor/tests.rs @@ -1485,6 +1485,24 @@ fn vim_edit_asks_before_it_drops_changes_to_the_file() { wait_until(WAIT, || buffer_text(&buffer) == "SELECT 1;").expect("the file again"); } +#[gtk4::test] +fn vim_edit_asks_before_it_undoes_emptying_the_file() { + let root = tempfile::tempdir().expect("tempdir"); + let path = root.path().join("daily.sql"); + std::fs::write(&path, "SELECT 1;").expect("seed"); + let harness = editor_with(answering(), ""); + open_path(&harness, &path); + let window = shown(&harness); + let buffer = harness.editor.buffer(); + buffer.delete(&mut buffer.start_iter(), &mut buffer.end_iter()); + let context = vim(&harness); + + assert!(vim_command(&context, ":e")); + + wait_until(WAIT, || crate::workspace::visible_alert(&window).is_some()).expect("the question"); + assert_eq!(buffer_text(&buffer), ""); +} + #[gtk4::test] fn vim_edit_reads_the_tabs_file_again() { let root = tempfile::tempdir().expect("tempdir"); diff --git a/linux/crates/app/src/ui/editor/vim_mode.rs b/linux/crates/app/src/ui/editor/vim_mode.rs index bd0428b27a..6b4cc8b5a4 100644 --- a/linux/crates/app/src/ui/editor/vim_mode.rs +++ b/linux/crates/app/src/ui/editor/vim_mode.rs @@ -260,7 +260,8 @@ impl SqlEditor { if force || !self.has_unsaved_file_changes() { self.load_file(file); } else { - self.replace_after_asking(move |editor| editor.load_file(file.clone())); + // Even when the text is now empty: emptying it was the change. + self.ask_to_replace(move |editor| editor.load_file(file.clone())); } } @@ -275,20 +276,26 @@ fn leaves_insert(keyval: gdk::Key, state: gdk::ModifierType) -> bool { keyval == gdk::Key::Escape || (control && matches!(keyval, gdk::Key::bracketleft | gdk::Key::c)) } -/// A relative name is next to the tab's file, or in the home folder -/// when the tab has none, never in the app's working folder. +/// A relative name is next to the tab's file, which may be a remote +/// location, or in the home folder when the tab has none, never in the +/// app's working folder. fn typed_file(typed: &str, tab_file: Option<&gio::File>) -> gio::File { + if glib::Uri::peek_scheme(typed).is_some() { + return gio::File::for_uri(typed); + } let home = glib::home_dir(); - let expanded = match typed.strip_prefix('~') { + let path = match typed.strip_prefix('~') { Some("") => home.clone(), Some(rest) if rest.starts_with('/') => home.join(rest.trim_start_matches('/')), _ => PathBuf::from(typed), }; - let folder = tab_file + if path.is_absolute() { + return gio::File::for_path(path); + } + tab_file .and_then(|file| file.parent()) - .and_then(|parent| parent.path()) - .unwrap_or(home); - gio::File::for_commandline_arg_and_cwd(expanded, folder) + .unwrap_or_else(|| gio::File::for_path(home)) + .resolve_relative_path(path) } async fn exists(file: &gio::File) -> bool { @@ -367,6 +374,20 @@ mod tests { ); } + #[test] + fn a_typed_name_next_to_a_remote_file_stays_remote() { + let tab = gio::File::for_uri("sftp://db.example/srv/sql/daily.sql"); + + assert_eq!( + typed_file("weekly.sql", Some(&tab)).uri().as_str(), + "sftp://db.example/srv/sql/weekly.sql" + ); + assert_eq!( + typed_file("sftp://db.example/all.sql", None).uri().as_str(), + "sftp://db.example/all.sql" + ); + } + #[test] fn a_typed_name_without_a_tab_file_is_in_the_home_folder() { let home = glib::home_dir(); From f2965e6cf99f4e7aa02fb9efd3bac4c2f6553b92 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Fri, 23 Oct 2026 08:48:50 +0700 Subject: [PATCH 03/11] fix(linux-editor): claim a new :w name before writing to it --- linux/crates/app/src/ui/editor/tests.rs | 19 +++++++++ linux/crates/app/src/ui/editor/vim_mode.rs | 49 ++++++++++++++-------- 2 files changed, 51 insertions(+), 17 deletions(-) diff --git a/linux/crates/app/src/ui/editor/tests.rs b/linux/crates/app/src/ui/editor/tests.rs index 2a42aef872..0fbf68f645 100644 --- a/linux/crates/app/src/ui/editor/tests.rs +++ b/linux/crates/app/src/ui/editor/tests.rs @@ -1428,6 +1428,25 @@ fn vim_write_to_a_name_leaves_another_file_alone() { assert_eq!(harness.editor.file().and_then(|file| file.path()), Some(path)); } +#[gtk4::test] +fn vim_write_to_a_new_name_that_fails_leaves_no_file_behind() { + let root = tempfile::tempdir().expect("tempdir"); + let path = root.path().join("daily.sql"); + std::fs::write(&path, "SELECT 1;").expect("seed"); + let harness = editor_with(answering(), ""); + open_path(&harness, &path); + let window = shown(&harness); + let context = vim(&harness); + harness.editor.imp().saving.set(true); + + assert!(vim_command(&context, ":w weekly.sql")); + wait_until(WAIT, || crate::workspace::visible_alert(&window).is_some()).expect("the failed write to say so"); + let claimed = root.path().join("weekly.sql"); + + wait_until(WAIT, || !claimed.exists()).expect("the claimed name removed again"); + assert_eq!(harness.editor.file().and_then(|file| file.path()), Some(path)); +} + #[gtk4::test] fn vim_force_write_to_a_name_replaces_another_file() { let root = tempfile::tempdir().expect("tempdir"); diff --git a/linux/crates/app/src/ui/editor/vim_mode.rs b/linux/crates/app/src/ui/editor/vim_mode.rs index 6b4cc8b5a4..5acb5432be 100644 --- a/linux/crates/app/src/ui/editor/vim_mode.rs +++ b/linux/crates/app/src/ui/editor/vim_mode.rs @@ -226,17 +226,33 @@ impl SqlEditor { let editor = self.clone(); glib::spawn_future_local(async move { let own = editor.file().is_some_and(|tab| tab.equal(&file)); - if !replace && !own && exists(&file).await { - editor.emit_by_name::<()>( - "toast", - &[&crate::i18n::gettext_f( - "“{name}” already exists. Use :w! to replace it.", - &[("name", &super::file_name(&file))], - )], - ); + if replace || own { + editor.write_reporting(&file).await; return; } - editor.write_reporting(&file).await; + // Claimed in one step, so a second tab's write to the same + // new name cannot land between the check and the write. + let claimed = match claim_new_file(&file).await { + Ok(()) => true, + Err(error) if error.matches(gio::IOErrorEnum::Exists) => { + editor.emit_by_name::<()>( + "toast", + &[&crate::i18n::gettext_f( + "“{name}” already exists. Use :w! to replace it.", + &[("name", &super::file_name(&file))], + )], + ); + return; + } + // The write meets the same error and says it. + Err(_) => false, + }; + if !editor.write_reporting(&file).await + && claimed + && let Err(error) = file.delete_future(glib::Priority::DEFAULT).await + { + tracing::warn!(%error, "could not remove the empty file a failed :w left"); + } }); } @@ -298,14 +314,13 @@ fn typed_file(typed: &str, tab_file: Option<&gio::File>) -> gio::File { .resolve_relative_path(path) } -async fn exists(file: &gio::File) -> bool { - file.query_info_future( - gio::FILE_ATTRIBUTE_STANDARD_TYPE, - gio::FileQueryInfoFlags::NONE, - glib::Priority::DEFAULT, - ) - .await - .is_ok() +/// An empty file, created only when no file has the name yet. PRIVATE +/// as `file_write::replace_file` makes a new file, which keeps the mode. +async fn claim_new_file(file: &gio::File) -> Result<(), glib::Error> { + let stream = file + .create_future(gio::FileCreateFlags::PRIVATE, glib::Priority::DEFAULT) + .await?; + stream.close_future(glib::Priority::DEFAULT).await } #[cfg(test)] From 7aa815da7282d4b3c0ed75d68c1c48dbfc5327b8 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Fri, 23 Oct 2026 08:56:43 +0700 Subject: [PATCH 04/11] test(linux-editor): wait for the tab to follow a named :w --- linux/crates/app/src/ui/editor/tests.rs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/linux/crates/app/src/ui/editor/tests.rs b/linux/crates/app/src/ui/editor/tests.rs index 0fbf68f645..9a98e296c5 100644 --- a/linux/crates/app/src/ui/editor/tests.rs +++ b/linux/crates/app/src/ui/editor/tests.rs @@ -1400,10 +1400,11 @@ fn vim_write_to_a_name_saves_next_to_the_tabs_file() { assert!(vim_command(&context, ":w weekly.sql")); let copy = root.path().join("weekly.sql"); - wait_until(WAIT, || copy.exists()).expect("the new file"); + // The name is claimed empty first; the tab follows once the text is in. + wait_until(WAIT, || harness.editor.file().and_then(|file| file.path()).as_ref() == Some(©)) + .expect("the tab on the new file"); assert_eq!(std::fs::read_to_string(©).expect("read back"), "SELECT 1;"); - assert_eq!(harness.editor.file().and_then(|file| file.path()), Some(copy)); } #[gtk4::test] @@ -1459,10 +1460,10 @@ fn vim_force_write_to_a_name_replaces_another_file() { let context = vim(&harness); assert!(vim_command(&context, ":w! weekly.sql")); - wait_until(WAIT, || std::fs::read_to_string(&other).is_ok_and(|text| text == "SELECT 1;")) - .expect("the other file replaced"); + wait_until(WAIT, || harness.editor.file().and_then(|file| file.path()).as_ref() == Some(&other)) + .expect("the tab on the other file"); - assert_eq!(harness.editor.file().and_then(|file| file.path()), Some(other)); + assert_eq!(std::fs::read_to_string(&other).expect("read back"), "SELECT 1;"); } #[gtk4::test] From 03f03a2caafae276c799ceee76d09b14839d03d9 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Fri, 23 Oct 2026 09:05:10 +0700 Subject: [PATCH 05/11] fix(linux-editor): match the keys Vim leaves to the editor as GTK matches shortcuts --- .../app/src/application/accelerators.rs | 121 ++++++++---------- linux/crates/app/src/application/mod.rs | 2 +- linux/crates/app/src/ui/editor/vim_mode.rs | 10 +- 3 files changed, 59 insertions(+), 74 deletions(-) diff --git a/linux/crates/app/src/application/accelerators.rs b/linux/crates/app/src/application/accelerators.rs index 7bf3434ba9..89884e46b0 100644 --- a/linux/crates/app/src/application/accelerators.rs +++ b/linux/crates/app/src/application/accelerators.rs @@ -387,25 +387,27 @@ pub fn component_matches(action: &str, keyval: gdk::Key, state: gdk::ModifierTyp } /// The SQL editor's own Ctrl keys, which Vim mode leaves to the editor in -/// every mode. Escape and the other plain keys are Vim's. -pub fn editor_keeps(keyval: gdk::Key, state: gdk::ModifierType) -> bool { - let modifiers = state & gtk::accelerator_get_default_mod_mask(); - let keyval = keyval.to_lower(); - ACCELERATORS +/// every mode. A trigger, so an event matches it as the editor's shortcut +/// controllers match it, layout and consumed Shift included. Escape and +/// the other plain keys are Vim's. +pub fn editor_kept_keys() -> Option { + let accels: Vec<&str> = ACCELERATORS .iter() - .filter(|entry| { - matches!( - entry.scope, - Local { - owner: SqlEditor | SqlEditorTab, - .. - } - ) - }) - .flat_map(|entry| entry.triggers.iter()) - .filter_map(|accel| parse(accel)) - .filter(|(_, mods)| mods.contains(gdk::ModifierType::CONTROL_MASK)) - .any(|(key, mods)| mods == modifiers && (key == keyval || keypad_twins(key).contains(&keyval))) + .filter(|entry| is_editors(entry)) + .flat_map(|entry| entry.triggers.iter().copied()) + .filter(|accel| parse(accel).is_some_and(|(_, mods)| mods.contains(gdk::ModifierType::CONTROL_MASK))) + .collect(); + shortcut_trigger(&accels) +} + +fn is_editors(entry: &Accelerator) -> bool { + matches!( + entry.scope, + Local { + owner: SqlEditor | SqlEditorTab, + .. + } + ) } /// GtkPopoverMenu shows an application accelerator by itself; a Local or @@ -687,66 +689,45 @@ mod tests { #[gtk4::test] fn the_editor_keeps_its_ctrl_keys_from_vim() { - let ctrl = gdk::ModifierType::CONTROL_MASK; - let ctrl_shift = ctrl | gdk::ModifierType::SHIFT_MASK; - let none = gdk::ModifierType::empty(); + let kept: HashSet = trigger_keys(&editor_kept_keys().expect("the editor has Ctrl keys")) + .into_iter() + .collect(); - for (key, state) in [ - (gdk::Key::Return, ctrl), - (gdk::Key::KP_Enter, ctrl), - (gdk::Key::Return, ctrl_shift), - (gdk::Key::F, ctrl_shift), - (gdk::Key::slash, ctrl), - (gdk::Key::f, ctrl), - (gdk::Key::f, ctrl | gdk::ModifierType::LOCK_MASK), - (gdk::Key::h, ctrl), - (gdk::Key::g, ctrl), - (gdk::Key::G, ctrl_shift), - (gdk::Key::o, ctrl), - (gdk::Key::s, ctrl), - (gdk::Key::S, ctrl_shift), + for accel in [ + "Return", + "KP_Enter", + "Return", + "f", + "slash", + "f", + "h", + "g", + "g", + "o", + "s", + "s", ] { - assert!(editor_keeps(key, state), "{key:?} with {state:?}"); + assert!(kept.contains(&keystroke(accel)), "{accel}"); } - for (key, state) in [ - (gdk::Key::Escape, none), - (gdk::Key::f, none), - (gdk::Key::Return, none), - (gdk::Key::r, ctrl), - (gdk::Key::d, ctrl), - (gdk::Key::v, ctrl), - (gdk::Key::f, gdk::ModifierType::ALT_MASK), - (gdk::Key::F6, none), - ] { - assert!(!editor_keeps(key, state), "{key:?} with {state:?}"); + for accel in ["Escape", "r", "d", "v", "w", "f"] { + assert!(!kept.contains(&keystroke(accel)), "{accel}"); } } #[gtk4::test] fn every_ctrl_key_of_the_editor_is_kept_from_vim() { - let mut checked = 0; - for entry in ACCELERATORS { - if !matches!( - entry.scope, - Local { - owner: SqlEditor | SqlEditorTab, - .. - } - ) { - continue; - } - for accel in entry.triggers { - let (key, mods) = gtk::accelerator_parse(*accel).expect("the table parses"); - assert_eq!( - editor_keeps(key, mods), - mods.contains(gdk::ModifierType::CONTROL_MASK), - "{accel} of {}", - entry.action - ); - checked += 1; - } - } - assert_eq!(checked, 12); + let kept: HashSet = trigger_keys(&editor_kept_keys().expect("the editor has Ctrl keys")) + .into_iter() + .collect(); + let owned: HashSet = ACCELERATORS + .iter() + .filter(|entry| is_editors(entry)) + .flat_map(|entry| entry.triggers.iter()) + .flat_map(|accel| keystrokes_with_twins(accel)) + .filter(|(_, mods)| mods.contains(gdk::ModifierType::CONTROL_MASK)) + .collect(); + + assert_eq!(kept, owned); } #[gtk4::test] diff --git a/linux/crates/app/src/application/mod.rs b/linux/crates/app/src/application/mod.rs index 29e68ef1ac..be58117052 100644 --- a/linux/crates/app/src/application/mod.rs +++ b/linux/crates/app/src/application/mod.rs @@ -40,7 +40,7 @@ pub use accelerator_scope::AcceleratorScope; #[cfg(test)] pub(crate) use accelerators::keypad_twins; pub use accelerators::{ - ACCELERATORS, component_matches, component_trigger, controller_name, editor_keeps, + ACCELERATORS, component_matches, component_trigger, controller_name, editor_kept_keys, install_application_accelerators, local_shortcut_controller, menu_item, triggers_for, }; pub use app_actions::install_app_actions; diff --git a/linux/crates/app/src/ui/editor/vim_mode.rs b/linux/crates/app/src/ui/editor/vim_mode.rs index 5acb5432be..70ede7bac0 100644 --- a/linux/crates/app/src/ui/editor/vim_mode.rs +++ b/linux/crates/app/src/ui/editor/vim_mode.rs @@ -107,13 +107,17 @@ impl SqlEditor { keys.set_propagation_phase(gtk4::PropagationPhase::Capture); let filter = context.clone(); let popups = view.downgrade(); + let kept = crate::application::editor_kept_keys(); keys.connect_key_pressed(move |keys, keyval, _, state| { - if crate::application::editor_keeps(keyval, state) { - return glib::Propagation::Proceed; - } let Some(event) = keys.current_event() else { return glib::Propagation::Proceed; }; + if kept + .as_ref() + .is_some_and(|kept| kept.trigger(&event, false) != gdk::KeyMatch::None) + { + return glib::Propagation::Proceed; + } if !filter.filter_keypress(&event) { return glib::Propagation::Proceed; } From 7c7057f512259b12b72a1d6cbaaa8288f01ad1e8 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Fri, 23 Oct 2026 09:14:22 +0700 Subject: [PATCH 06/11] fix(linux-editor): ask before a load replaces a file emptied and not saved --- linux/crates/app/src/ui/editor/mod.rs | 11 ++++------- linux/crates/app/src/ui/editor/tests.rs | 21 +++++++++++++++++++++ linux/crates/app/src/ui/editor/vim_mode.rs | 3 +-- 3 files changed, 26 insertions(+), 9 deletions(-) diff --git a/linux/crates/app/src/ui/editor/mod.rs b/linux/crates/app/src/ui/editor/mod.rs index 056da64a7a..ae5e0bbc05 100644 --- a/linux/crates/app/src/ui/editor/mod.rs +++ b/linux/crates/app/src/ui/editor/mod.rs @@ -864,16 +864,13 @@ impl SqlEditor { /// Loading replaces what is in the tab, which is where the user's /// unrun work lives, so `replace` runs once the user agrees, or - /// straight away when the tab holds nothing. + /// straight away when the tab holds nothing. A file emptied and not + /// saved still holds a change. fn replace_after_asking(&self, replace: impl Fn(&SqlEditor) + 'static) { - if buffer_text(&self.buffer()).trim().is_empty() { + if buffer_text(&self.buffer()).trim().is_empty() && !self.has_unsaved_file_changes() { replace(self); - } else { - self.ask_to_replace(replace); + return; } - } - - fn ask_to_replace(&self, replace: impl Fn(&SqlEditor) + 'static) { let dialog = adw::AlertDialog::new( Some(&crate::i18n::gettext("Replace the Editor Contents?")), Some(&crate::i18n::gettext( diff --git a/linux/crates/app/src/ui/editor/tests.rs b/linux/crates/app/src/ui/editor/tests.rs index 9a98e296c5..e37bbaa44b 100644 --- a/linux/crates/app/src/ui/editor/tests.rs +++ b/linux/crates/app/src/ui/editor/tests.rs @@ -1505,6 +1505,27 @@ fn vim_edit_asks_before_it_drops_changes_to_the_file() { wait_until(WAIT, || buffer_text(&buffer) == "SELECT 1;").expect("the file again"); } +#[gtk4::test] +fn vim_edit_a_name_asks_before_it_drops_an_emptied_file() { + let root = tempfile::tempdir().expect("tempdir"); + let path = root.path().join("daily.sql"); + let other = root.path().join("weekly.sql"); + std::fs::write(&path, "SELECT 1;").expect("seed"); + std::fs::write(&other, "SELECT 7;").expect("seed"); + let harness = editor_with(answering(), ""); + open_path(&harness, &path); + let window = shown(&harness); + let buffer = harness.editor.buffer(); + buffer.delete(&mut buffer.start_iter(), &mut buffer.end_iter()); + let context = vim(&harness); + + assert!(vim_command(&context, ":e weekly.sql")); + + wait_until(WAIT, || crate::workspace::visible_alert(&window).is_some()).expect("the question"); + assert_eq!(buffer_text(&buffer), ""); + assert_eq!(harness.editor.file().and_then(|file| file.path()), Some(path)); +} + #[gtk4::test] fn vim_edit_asks_before_it_undoes_emptying_the_file() { let root = tempfile::tempdir().expect("tempdir"); diff --git a/linux/crates/app/src/ui/editor/vim_mode.rs b/linux/crates/app/src/ui/editor/vim_mode.rs index 70ede7bac0..a944dc1c24 100644 --- a/linux/crates/app/src/ui/editor/vim_mode.rs +++ b/linux/crates/app/src/ui/editor/vim_mode.rs @@ -280,8 +280,7 @@ impl SqlEditor { if force || !self.has_unsaved_file_changes() { self.load_file(file); } else { - // Even when the text is now empty: emptying it was the change. - self.ask_to_replace(move |editor| editor.load_file(file.clone())); + self.replace_after_asking(move |editor| editor.load_file(file.clone())); } } From cc59b5948ac8caf98f0485d9903f34a200cedd7d Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Fri, 23 Oct 2026 09:35:40 +0700 Subject: [PATCH 07/11] style(linux-editor): rustfmt the Vim mode tests --- linux/crates/app/src/ui/editor/tests.rs | 43 +++++++++++++--------- linux/crates/app/src/ui/editor/vim_mode.rs | 10 ++--- 2 files changed, 30 insertions(+), 23 deletions(-) diff --git a/linux/crates/app/src/ui/editor/tests.rs b/linux/crates/app/src/ui/editor/tests.rs index e37bbaa44b..6f88ee0784 100644 --- a/linux/crates/app/src/ui/editor/tests.rs +++ b/linux/crates/app/src/ui/editor/tests.rs @@ -1250,11 +1250,7 @@ fn a_tab_with_no_file_has_nothing_unsaved_to_ask_about() { /// Turn the preference on and hand over Vim's input method. fn vim(harness: &Harness) -> sourceview5::VimIMContext { - harness - .settings - .get() - .set_vim_keybindings(true) - .expect("turn Vim on"); + harness.settings.get().set_vim_keybindings(true).expect("turn Vim on"); harness.editor.vim_context().expect("Vim on the view") } @@ -1276,11 +1272,7 @@ fn vim_follows_the_preference_and_leaves_the_view_as_it_was() { assert!(imp.text_pane.reveals_bottom_bars(), "the mode line is hidden"); assert!(view.overwrites(), "normal mode draws no block cursor"); - harness - .settings - .get() - .set_vim_keybindings(false) - .expect("turn Vim off"); + harness.settings.get().set_vim_keybindings(false).expect("turn Vim off"); assert!(!harness.editor.has_vim()); assert!(!imp.text_pane.reveals_bottom_bars()); @@ -1308,7 +1300,10 @@ fn vim_quit_asks_the_workspace_to_close_the_tab() { assert!(vim_command(&context, ":q")); assert!(vim_command(&context, ":quit!")); - assert!(!vim_command(&context, ":nohl"), "a command of GtkSourceView's own was taken"); + assert!( + !vim_command(&context, ":nohl"), + "a command of GtkSourceView's own was taken" + ); assert_eq!(closes.count(), 2); } @@ -1401,8 +1396,10 @@ fn vim_write_to_a_name_saves_next_to_the_tabs_file() { assert!(vim_command(&context, ":w weekly.sql")); let copy = root.path().join("weekly.sql"); // The name is claimed empty first; the tab follows once the text is in. - wait_until(WAIT, || harness.editor.file().and_then(|file| file.path()).as_ref() == Some(©)) - .expect("the tab on the new file"); + wait_until(WAIT, || { + harness.editor.file().and_then(|file| file.path()).as_ref() == Some(©) + }) + .expect("the tab on the new file"); assert_eq!(std::fs::read_to_string(©).expect("read back"), "SELECT 1;"); } @@ -1460,8 +1457,10 @@ fn vim_force_write_to_a_name_replaces_another_file() { let context = vim(&harness); assert!(vim_command(&context, ":w! weekly.sql")); - wait_until(WAIT, || harness.editor.file().and_then(|file| file.path()).as_ref() == Some(&other)) - .expect("the tab on the other file"); + wait_until(WAIT, || { + harness.editor.file().and_then(|file| file.path()).as_ref() == Some(&other) + }) + .expect("the tab on the other file"); assert_eq!(std::fs::read_to_string(&other).expect("read back"), "SELECT 1;"); } @@ -1960,11 +1959,15 @@ mod real_keys { let buffer = harness.editor.buffer(); buffer.place_cursor(&buffer.start_iter()); harness.editor.imp().source_view.grab_focus(); - (window, crate::test_support::KeyInjector::connect().expect("an X11 display")) + ( + window, + crate::test_support::KeyInjector::connect().expect("an X11 display"), + ) } fn press(keys: &crate::test_support::KeyInjector, key: gtk4::gdk::Key) { - keys.press(key, gtk4::gdk::ModifierType::empty()).expect("the key press"); + keys.press(key, gtk4::gdk::ModifierType::empty()) + .expect("the key press"); } /// A key typed with Shift, such as `:` or `G` on a US layout. @@ -2021,7 +2024,11 @@ mod real_keys { ) .expect("the key press"); wait_until(WAIT, || sent(&harness.fake).len() == 2).expect("Ctrl+Shift+Return to run in insert mode"); - assert_eq!(buffer_text(&harness.editor.buffer()), "select 1", "Return went into the text"); + assert_eq!( + buffer_text(&harness.editor.buffer()), + "select 1", + "Return went into the text" + ); } /// Normal mode binds Ctrl+F to a page down, and the command line diff --git a/linux/crates/app/src/ui/editor/vim_mode.rs b/linux/crates/app/src/ui/editor/vim_mode.rs index a944dc1c24..f254b415c2 100644 --- a/linux/crates/app/src/ui/editor/vim_mode.rs +++ b/linux/crates/app/src/ui/editor/vim_mode.rs @@ -271,10 +271,7 @@ impl SqlEditor { fn reload_file(&self, force: bool) { let Some(file) = self.file() else { - self.emit_by_name::<()>( - "toast", - &[&crate::i18n::gettext("This tab is not saved to a file.")], - ); + self.emit_by_name::<()>("toast", &[&crate::i18n::gettext("This tab is not saved to a file.")]); return; }; if force || !self.has_unsaved_file_changes() { @@ -356,7 +353,10 @@ mod tests { (":w!", TabCommand::ForceWrite(None)), (":write!", TabCommand::ForceWrite(None)), (":w! a.sql", TabCommand::ForceWrite(Some("a.sql".to_owned()))), - (":w! my query.sql ", TabCommand::ForceWrite(Some("my query.sql".to_owned()))), + ( + ":w! my query.sql ", + TabCommand::ForceWrite(Some("my query.sql".to_owned())), + ), (":e!", TabCommand::ForceEdit(None)), (":edit!", TabCommand::ForceEdit(None)), (":e! a.sql", TabCommand::ForceEdit(Some("a.sql".to_owned()))), From fc1959cdd3e703a114fa12e8bb9e3a538502b2b3 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Fri, 23 Oct 2026 11:33:52 +0700 Subject: [PATCH 08/11] docs(linux): changelog fragments for Vim mode --- linux/changelog.d/3804.added.md | 1 + linux/changelog.d/3804.fixed.md | 1 + 2 files changed, 2 insertions(+) create mode 100644 linux/changelog.d/3804.added.md create mode 100644 linux/changelog.d/3804.fixed.md diff --git a/linux/changelog.d/3804.added.md b/linux/changelog.d/3804.added.md new file mode 100644 index 0000000000..9ee7988390 --- /dev/null +++ b/linux/changelog.d/3804.added.md @@ -0,0 +1 @@ +The SQL editor has Vim keybindings, turned on in Preferences, and :w, :e, :q and :wq act on the tab. diff --git a/linux/changelog.d/3804.fixed.md b/linux/changelog.d/3804.fixed.md new file mode 100644 index 0000000000..beee3e9acc --- /dev/null +++ b/linux/changelog.d/3804.fixed.md @@ -0,0 +1 @@ +Open SQL File replaced a tab's file without asking when its text was deleted and not saved. From e7843d38d1b9204548df1d5ecb842fe05a7c888b Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Fri, 23 Oct 2026 11:43:43 +0700 Subject: [PATCH 09/11] fix(linux-editor): read :w and :e with only a trailing space as the tab's own file --- linux/crates/app/src/ui/editor/tests.rs | 21 +++++++++++++++++++++ linux/crates/app/src/ui/editor/vim_mode.rs | 18 ++++++++++++++++-- 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/linux/crates/app/src/ui/editor/tests.rs b/linux/crates/app/src/ui/editor/tests.rs index 6f88ee0784..c60784a160 100644 --- a/linux/crates/app/src/ui/editor/tests.rs +++ b/linux/crates/app/src/ui/editor/tests.rs @@ -1328,6 +1328,27 @@ fn vim_write_saves_the_tabs_file_as_ctrl_s_does() { ); } +#[gtk4::test] +fn vim_write_with_a_trailing_space_saves_the_tabs_file() { + let root = tempfile::tempdir().expect("tempdir"); + let path = root.path().join("daily.sql"); + std::fs::write(&path, "SELECT 1;").expect("seed"); + let harness = editor_with(answering(), ""); + open_path(&harness, &path); + let buffer = harness.editor.buffer(); + buffer.insert(&mut buffer.end_iter(), "\nSELECT 2;"); + let context = vim(&harness); + + assert!(vim_command(&context, ":write ")); + wait_until(WAIT, || !harness.editor.has_unsaved_file_changes()).expect("the save to land"); + + assert_eq!( + std::fs::read_to_string(&path).expect("read back"), + "SELECT 1;\nSELECT 2;" + ); + assert_eq!(harness.editor.file().and_then(|file| file.path()), Some(path)); +} + #[gtk4::test] fn vim_write_quit_closes_once_the_file_is_written() { let root = tempfile::tempdir().expect("tempdir"); diff --git a/linux/crates/app/src/ui/editor/vim_mode.rs b/linux/crates/app/src/ui/editor/vim_mode.rs index f254b415c2..fdd6d22011 100644 --- a/linux/crates/app/src/ui/editor/vim_mode.rs +++ b/linux/crates/app/src/ui/editor/vim_mode.rs @@ -175,10 +175,12 @@ impl SqlEditor { editor.run_tab_command(command); true }); + // Command completion leaves a space after the name, so `:w ` and + // `:e ` arrive with an empty path. let editor = self.downgrade(); context.connect_write(move |_, _, path| { if let Some(editor) = editor.upgrade() { - match path { + match named(path) { Some(path) => editor.write_named(path, false), None => editor.save(), } @@ -187,7 +189,7 @@ impl SqlEditor { let editor = self.downgrade(); context.connect_edit(move |_, _, path| { if let Some(editor) = editor.upgrade() { - match path { + match named(path) { Some(path) => editor.edit_named(path, false), None => editor.reload_file(false), } @@ -287,6 +289,10 @@ impl SqlEditor { } } +fn named(path: Option<&str>) -> Option<&str> { + path.map(str::trim).filter(|path| !path.is_empty()) +} + fn leaves_insert(keyval: gdk::Key, state: gdk::ModifierType) -> bool { let control = state.contains(gdk::ModifierType::CONTROL_MASK); keyval == gdk::Key::Escape || (control && matches!(keyval, gdk::Key::bracketleft | gdk::Key::c)) @@ -418,6 +424,14 @@ mod tests { ); } + #[test] + fn an_empty_path_names_no_file() { + assert_eq!(named(None), None); + assert_eq!(named(Some("")), None); + assert_eq!(named(Some(" ")), None); + assert_eq!(named(Some(" a.sql ")), Some("a.sql")); + } + #[test] fn escape_and_its_control_twins_leave_insert() { let ctrl = gdk::ModifierType::CONTROL_MASK; From e03495cef2ee3bcb3f3af2e2e6e04dccca5bd31d Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Fri, 23 Oct 2026 12:40:56 +0700 Subject: [PATCH 10/11] fix(linux-editor): keep completion to Vim's insert mode --- linux/README.md | 2 +- .../app/src/ui/editor/editor_actions.rs | 2 +- linux/crates/app/src/ui/editor/tests.rs | 57 +++++++++++++---- linux/crates/app/src/ui/editor/vim_mode.rs | 62 +++++++++++-------- 4 files changed, 83 insertions(+), 40 deletions(-) diff --git a/linux/README.md b/linux/README.md index 79c47503ac..ccd36d4b1f 100644 --- a/linux/README.md +++ b/linux/README.md @@ -257,7 +257,7 @@ The editor's own Ctrl shortcuts work in every mode: Ctrl+Return runs, Ctrl+Shift+Return runs the statement at the cursor, and find, find and replace, format, toggle comment, open and save keep their keys. Every other key goes to Vim, Escape too, so while Vim is on the **Stop** button -stops a running query. +stops a running query. Completion opens in insert mode only. ## SQL Server with Windows integrated auth diff --git a/linux/crates/app/src/ui/editor/editor_actions.rs b/linux/crates/app/src/ui/editor/editor_actions.rs index a06c24d4e0..c89b717d05 100644 --- a/linux/crates/app/src/ui/editor/editor_actions.rs +++ b/linux/crates/app/src/ui/editor/editor_actions.rs @@ -111,7 +111,7 @@ impl SqlEditor { /// GtkSourceView parents its completion list and other assistants to /// the view as popovers, and its Escape hides every visible one. -pub(super) fn close_view_popups(view: &impl IsA) -> bool { +fn close_view_popups(view: &impl IsA) -> bool { let mut closed = false; let mut child = view.as_ref().first_child(); while let Some(widget) = child { diff --git a/linux/crates/app/src/ui/editor/tests.rs b/linux/crates/app/src/ui/editor/tests.rs index c60784a160..f72e9cb7fc 100644 --- a/linux/crates/app/src/ui/editor/tests.rs +++ b/linux/crates/app/src/ui/editor/tests.rs @@ -2124,32 +2124,63 @@ mod real_keys { wait_until(KEY_WAIT, || closes.count() == 1).expect(":q to ask for the close"); } + fn completion_list_shown(view: &sourceview5::View) -> bool { + let mut child = view.first_child(); + while let Some(widget) = child { + if widget.is::() && widget.is_visible() { + return true; + } + child = widget.next_sibling(); + } + false + } + + /// Each key is waited for, so the list opens in insert mode and not + /// while Vim is still taking the keys before it. #[gtk4::test] - fn escape_from_insert_mode_closes_the_completion_list() { + fn leaving_insert_mode_closes_the_completion_list_for_good() { use gtk4::gdk::Key; let harness = editor_with(answering(), "SELECT 1;\n"); let (_window, keys) = in_vim(&harness); let view = harness.editor.imp().source_view.get(); - let shown_popup = || { - let mut child = view.first_child(); - while let Some(widget) = child { - if widget.is::() && widget.is_visible() { - return true; - } - child = widget.next_sibling(); - } - false - }; + let buffer = harness.editor.buffer(); + let mode = harness.editor.imp().vim_command_bar.get(); shifted(&keys, Key::G); press(&keys, Key::o); + wait_until(KEY_WAIT, || !mode.label().is_empty() && buffer.line_count() == 3).expect("o to open a line"); shifted(&keys, Key::S); shifted(&keys, Key::E); + wait_until(KEY_WAIT, || buffer_text(&buffer) == "SELECT 1;\n\nSE").expect("insert mode to type"); view.completion().show(); - wait_until(KEY_WAIT, shown_popup).expect("the completion list"); + wait_until(KEY_WAIT, || completion_list_shown(&view)).expect("the completion list"); press(&keys, Key::Escape); + wait_until(KEY_WAIT, || mode.label().is_empty()).expect("Escape to leave insert mode"); + wait_until(KEY_WAIT, || !completion_list_shown(&view)).expect("the list to close"); + + press(&keys, Key::x); + wait_until(KEY_WAIT, || buffer_text(&buffer) == "SELECT 1;\n\nS").expect("x to delete in normal mode"); + view.completion().show(); + crate::test_support::drain_main_context(); + assert!(!completion_list_shown(&view), "the list opened in normal mode"); + } + + #[gtk4::test] + fn completion_comes_back_when_vim_is_turned_off() { + let harness = editor_with(answering(), "SE"); + let (_window, _keys) = in_vim(&harness); + let view = harness.editor.imp().source_view.get(); + let buffer = harness.editor.buffer(); + buffer.place_cursor(&buffer.end_iter()); + + harness + .settings + .get() + .set_vim_keybindings(false) + .expect("turn Vim off"); + view.completion().show(); - wait_until(KEY_WAIT, || !shown_popup()).expect("Escape to close the list"); + wait_until(KEY_WAIT, || completion_list_shown(&view)).expect("the list without Vim"); } } diff --git a/linux/crates/app/src/ui/editor/vim_mode.rs b/linux/crates/app/src/ui/editor/vim_mode.rs index fdd6d22011..8f94844530 100644 --- a/linux/crates/app/src/ui/editor/vim_mode.rs +++ b/linux/crates/app/src/ui/editor/vim_mode.rs @@ -1,9 +1,11 @@ +use std::cell::Cell; use std::path::PathBuf; use std::rc::Rc; use gtk4::prelude::*; use gtk4::subclass::prelude::*; use gtk4::{gdk, gio, glib}; +use sourceview5::prelude::*; use tablepro_storage::AppSettings; use tablepro_storage::settings::keys; @@ -14,6 +16,8 @@ pub(crate) struct VimMode { context: sourceview5::VimIMContext, keys: gtk4::EventControllerKey, bindings: [glib::Binding; 2], + mode: glib::SignalHandlerId, + completion_blocked: Rc>, } /// GtkSourceView runs `:w` and `:e` itself and leaves these to the @@ -106,9 +110,8 @@ impl SqlEditor { keys.set_name(Some("tablepro-vim")); keys.set_propagation_phase(gtk4::PropagationPhase::Capture); let filter = context.clone(); - let popups = view.downgrade(); let kept = crate::application::editor_kept_keys(); - keys.connect_key_pressed(move |keys, keyval, _, state| { + keys.connect_key_pressed(move |keys, _, _, _| { let Some(event) = keys.current_event() else { return glib::Propagation::Proceed; }; @@ -118,22 +121,24 @@ impl SqlEditor { { return glib::Propagation::Proceed; } - if !filter.filter_keypress(&event) { - return glib::Propagation::Proceed; - } - // Vim takes Escape before the view, whose own Escape hides - // the completion list. - if leaves_insert(keyval, state) - && let Some(view) = popups.upgrade() - { - super::editor_actions::close_view_popups(&view); + if filter.filter_keypress(&event) { + glib::Propagation::Stop + } else { + glib::Propagation::Proceed } - glib::Propagation::Stop }); // GTK runs a widget's last controller first, so this one runs // before the editor's own keys, which it lets through. view.add_controller(keys.clone()); + // Completion belongs to insert mode: a list left open in normal + // mode would follow motions and edits that are not typing. Vim + // turns overwrite off in insert mode only. + let completion_blocked = Rc::new(Cell::new(false)); + follow_insert_mode(&view, &completion_blocked); + let blocked = completion_blocked.clone(); + let mode = view.connect_overwrite_notify(move |view| follow_insert_mode(view, &blocked)); + imp.text_pane.set_reveal_bottom_bars(true); // Escape is Vim's now; the Stop button still stops a run. imp.stop_button.reset_property(gtk4::AccessibleProperty::KeyShortcuts); @@ -141,6 +146,8 @@ impl SqlEditor { context, keys, bindings, + mode, + completion_blocked, })); } @@ -151,11 +158,15 @@ impl SqlEditor { }; let view = imp.source_view.get(); view.remove_controller(&vim.keys); + view.disconnect(vim.mode); for binding in vim.bindings { binding.unbind(); } vim.context.reset(); vim.context.set_client_widget(None::<>k4::Widget>); + if vim.completion_blocked.get() { + view.completion().unblock_interactive(); + } // Normal mode draws a block cursor through overwrite, and the // command line hides the cursor. view.set_overwrite(false); @@ -293,9 +304,20 @@ fn named(path: Option<&str>) -> Option<&str> { path.map(str::trim).filter(|path| !path.is_empty()) } -fn leaves_insert(keyval: gdk::Key, state: gdk::ModifierType) -> bool { - let control = state.contains(gdk::ModifierType::CONTROL_MASK); - keyval == gdk::Key::Escape || (control && matches!(keyval, gdk::Key::bracketleft | gdk::Key::c)) +/// Blocking cancels the list and its pending results too, which only +/// hiding the list would leave to reopen it. +fn follow_insert_mode(view: &sourceview5::View, blocked: &Cell) { + let block = view.overwrites(); + if block == blocked.get() { + return; + } + let completion = view.completion(); + if block { + completion.block_interactive(); + } else { + completion.unblock_interactive(); + } + blocked.set(block); } /// A relative name is next to the tab's file, which may be a remote @@ -431,14 +453,4 @@ mod tests { assert_eq!(named(Some(" ")), None); assert_eq!(named(Some(" a.sql ")), Some("a.sql")); } - - #[test] - fn escape_and_its_control_twins_leave_insert() { - let ctrl = gdk::ModifierType::CONTROL_MASK; - assert!(leaves_insert(gdk::Key::Escape, gdk::ModifierType::empty())); - assert!(leaves_insert(gdk::Key::bracketleft, ctrl)); - assert!(leaves_insert(gdk::Key::c, ctrl)); - assert!(!leaves_insert(gdk::Key::c, gdk::ModifierType::empty())); - assert!(!leaves_insert(gdk::Key::bracketleft, gdk::ModifierType::empty())); - } } From 4b0d6a72be170038c9d013fcd5fb130aa109924c Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Fri, 23 Oct 2026 12:57:41 +0700 Subject: [PATCH 11/11] fix(linux-editor): create a new :w name in one exclusive write --- linux/crates/app/src/ui/editor/editor_file.rs | 50 +++++++++++++++---- linux/crates/app/src/ui/editor/tests.rs | 13 ++--- linux/crates/app/src/ui/editor/vim_mode.rs | 28 ++--------- linux/crates/app/src/ui/file_write.rs | 48 ++++++++++++++++++ 4 files changed, 98 insertions(+), 41 deletions(-) diff --git a/linux/crates/app/src/ui/editor/editor_file.rs b/linux/crates/app/src/ui/editor/editor_file.rs index d138e43833..6944963031 100644 --- a/linux/crates/app/src/ui/editor/editor_file.rs +++ b/linux/crates/app/src/ui/editor/editor_file.rs @@ -143,23 +143,38 @@ impl SqlEditor { } } + async fn write_to(&self, file: &gio::File) -> Result<(), String> { + self.write_with(file, false) + .await + .map_err(|failure| failure.message(file)) + } + + /// As Save As to a name no file has yet; one that appears meanwhile + /// is left alone and fails with `Exists`. + pub(super) async fn write_new(&self, file: &gio::File) -> Result<(), WriteFailure> { + self.write_with(file, true).await + } + /// Text typed while the write runs is not in the file, so the tab /// stays changed. - async fn write_to(&self, file: &gio::File) -> Result<(), String> { + async fn write_with(&self, file: &gio::File, create: bool) -> Result<(), WriteFailure> { let imp = self.imp(); if imp.saving.replace(true) { - return Err(crate::i18n::gettext("This tab is already being saved.")); + return Err(WriteFailure::Busy); } let buffer = self.buffer(); let edits = imp.edits.get(); - let written = crate::ui::file_write::replace_file(file, buffer_text(&buffer).into_bytes()).await; + let contents = buffer_text(&buffer).into_bytes(); + let written = match create { + true => crate::ui::file_write::create_file(file, contents).await, + false => crate::ui::file_write::replace_file(file, contents).await, + }; imp.saving.set(false); if let Err(error) = written { - tracing::warn!(%error, "could not save an editor tab to its file"); - return Err(crate::i18n::gettext_f( - "“{name}” could not be saved: {error}", - &[("name", &file_name(file)), ("error", error.message())], - )); + if !error.matches(gio::IOErrorEnum::Exists) { + tracing::warn!(%error, "could not save an editor tab to its file"); + } + return Err(WriteFailure::Io(error)); } imp.file.replace(Some(file.clone())); if imp.edits.get() == edits { @@ -169,7 +184,7 @@ impl SqlEditor { Ok(()) } - fn alert_save_failed(&self, message: &str) { + pub(super) fn alert_save_failed(&self, message: &str) { let dialog = adw::AlertDialog::new(Some(&crate::i18n::gettext("Could Not Save File")), Some(message)); dialog.add_response("close", &crate::i18n::gettext("_Close")); dialog.set_default_response(Some("close")); @@ -178,6 +193,23 @@ impl SqlEditor { } } +pub(super) enum WriteFailure { + Busy, + Io(glib::Error), +} + +impl WriteFailure { + pub(super) fn message(&self, file: &gio::File) -> String { + match self { + Self::Busy => crate::i18n::gettext("This tab is already being saved."), + Self::Io(error) => crate::i18n::gettext_f( + "“{name}” could not be saved: {error}", + &[("name", &file_name(file)), ("error", error.message())], + ), + } + } +} + pub(super) fn sql_filter() -> gtk4::FileFilter { let filter = gtk4::FileFilter::new(); filter.set_name(Some(&crate::i18n::gettext("SQL files"))); diff --git a/linux/crates/app/src/ui/editor/tests.rs b/linux/crates/app/src/ui/editor/tests.rs index f72e9cb7fc..1948d567d7 100644 --- a/linux/crates/app/src/ui/editor/tests.rs +++ b/linux/crates/app/src/ui/editor/tests.rs @@ -1448,7 +1448,7 @@ fn vim_write_to_a_name_leaves_another_file_alone() { } #[gtk4::test] -fn vim_write_to_a_new_name_that_fails_leaves_no_file_behind() { +fn vim_write_to_a_new_name_during_a_save_writes_nothing() { let root = tempfile::tempdir().expect("tempdir"); let path = root.path().join("daily.sql"); std::fs::write(&path, "SELECT 1;").expect("seed"); @@ -1459,10 +1459,9 @@ fn vim_write_to_a_new_name_that_fails_leaves_no_file_behind() { harness.editor.imp().saving.set(true); assert!(vim_command(&context, ":w weekly.sql")); - wait_until(WAIT, || crate::workspace::visible_alert(&window).is_some()).expect("the failed write to say so"); - let claimed = root.path().join("weekly.sql"); + wait_until(WAIT, || crate::workspace::visible_alert(&window).is_some()).expect("the refusal to say so"); - wait_until(WAIT, || !claimed.exists()).expect("the claimed name removed again"); + assert!(!root.path().join("weekly.sql").exists(), "a file was made"); assert_eq!(harness.editor.file().and_then(|file| file.path()), Some(path)); } @@ -2174,11 +2173,7 @@ mod real_keys { let buffer = harness.editor.buffer(); buffer.place_cursor(&buffer.end_iter()); - harness - .settings - .get() - .set_vim_keybindings(false) - .expect("turn Vim off"); + harness.settings.get().set_vim_keybindings(false).expect("turn Vim off"); view.completion().show(); wait_until(KEY_WAIT, || completion_list_shown(&view)).expect("the list without Vim"); diff --git a/linux/crates/app/src/ui/editor/vim_mode.rs b/linux/crates/app/src/ui/editor/vim_mode.rs index 8f94844530..443dd5688e 100644 --- a/linux/crates/app/src/ui/editor/vim_mode.rs +++ b/linux/crates/app/src/ui/editor/vim_mode.rs @@ -10,6 +10,7 @@ use tablepro_storage::AppSettings; use tablepro_storage::settings::keys; use super::SqlEditor; +use super::editor_file::WriteFailure; use crate::ui::handler_guard::HandlerGuard; pub(crate) struct VimMode { @@ -247,11 +248,9 @@ impl SqlEditor { editor.write_reporting(&file).await; return; } - // Claimed in one step, so a second tab's write to the same - // new name cannot land between the check and the write. - let claimed = match claim_new_file(&file).await { - Ok(()) => true, - Err(error) if error.matches(gio::IOErrorEnum::Exists) => { + match editor.write_new(&file).await { + Ok(()) => {} + Err(WriteFailure::Io(error)) if error.matches(gio::IOErrorEnum::Exists) => { editor.emit_by_name::<()>( "toast", &[&crate::i18n::gettext_f( @@ -259,16 +258,8 @@ impl SqlEditor { &[("name", &super::file_name(&file))], )], ); - return; } - // The write meets the same error and says it. - Err(_) => false, - }; - if !editor.write_reporting(&file).await - && claimed - && let Err(error) = file.delete_future(glib::Priority::DEFAULT).await - { - tracing::warn!(%error, "could not remove the empty file a failed :w left"); + Err(failure) => editor.alert_save_failed(&failure.message(&file)), } }); } @@ -342,15 +333,6 @@ fn typed_file(typed: &str, tab_file: Option<&gio::File>) -> gio::File { .resolve_relative_path(path) } -/// An empty file, created only when no file has the name yet. PRIVATE -/// as `file_write::replace_file` makes a new file, which keeps the mode. -async fn claim_new_file(file: &gio::File) -> Result<(), glib::Error> { - let stream = file - .create_future(gio::FileCreateFlags::PRIVATE, glib::Priority::DEFAULT) - .await?; - stream.close_future(glib::Priority::DEFAULT).await -} - #[cfg(test)] mod tests { use super::*; diff --git a/linux/crates/app/src/ui/file_write.rs b/linux/crates/app/src/ui/file_write.rs index da37c3ca01..54de1172e8 100644 --- a/linux/crates/app/src/ui/file_write.rs +++ b/linux/crates/app/src/ui/file_write.rs @@ -15,6 +15,19 @@ pub(crate) async fn replace_file(file: &gio::File, contents: Vec) -> Result< .map_err(|(_, error)| error) } +/// Write `contents` to a file of that name only when none exists, so a +/// file that appears meanwhile is an `Exists` error and is left alone. +/// New, so private, as `replace_file` makes one. +pub(crate) async fn create_file(file: &gio::File, contents: Vec) -> Result<(), glib::Error> { + let stream = file + .create_future(gio::FileCreateFlags::PRIVATE, glib::Priority::DEFAULT) + .await?; + match stream.write_all_future(contents, glib::Priority::DEFAULT).await { + Ok((_, _, None)) => stream.close_future(glib::Priority::DEFAULT).await, + Ok((_, _, Some(error))) | Err((_, error)) => Err(error), + } +} + #[cfg(test)] mod tests { use std::time::Duration; @@ -69,6 +82,41 @@ mod tests { assert_eq!(mode & 0o777, 0o640, "{mode:o}"); } + fn create(file: gio::File, contents: &[u8]) -> Result<(), glib::Error> { + let contents = contents.to_vec(); + crate::test_support::finish_on_main( + Duration::from_secs(10), + async move { create_file(&file, contents).await }, + ) + .expect("the write to end") + } + + #[gtk4::test] + fn a_new_file_is_created_with_its_contents_for_its_owner_only() { + use std::os::unix::fs::PermissionsExt; + + let root = tempfile::tempdir().expect("tempdir"); + let path = root.path().join("weekly.sql"); + + create(gio::File::for_path(&path), b"SELECT 1;").expect("written"); + + assert_eq!(std::fs::read(&path).expect("read back"), b"SELECT 1;"); + let mode = std::fs::metadata(&path).expect("the file").permissions().mode(); + assert_eq!(mode & 0o777, 0o600, "{mode:o}"); + } + + #[gtk4::test] + fn creating_leaves_a_file_already_there_alone() { + let root = tempfile::tempdir().expect("tempdir"); + let path = root.path().join("weekly.sql"); + std::fs::write(&path, "SELECT 7;").expect("seed"); + + let error = create(gio::File::for_path(&path), b"SELECT 1;").expect_err("a file is there"); + + assert!(error.matches(gio::IOErrorEnum::Exists), "{error}"); + assert_eq!(std::fs::read(&path).expect("read back"), b"SELECT 7;"); + } + #[gtk4::test] fn a_folder_that_is_not_there_is_an_error() { let root = tempfile::tempdir().expect("tempdir");