From 8c0c30cda252d28bb077fc24797a7d28da9bc72f Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Sat, 17 Oct 2026 18:14:38 +0700 Subject: [PATCH 01/14] feat(linux): refresh the table list from the sidebar and after editor DDL --- .../app/src/ui/editor/catalog_change.rs | 110 ++++++++++++++++++ linux/crates/app/src/ui/editor/imp.rs | 3 + linux/crates/app/src/ui/editor/mod.rs | 15 ++- linux/crates/app/src/ui/editor/tests.rs | 24 ++++ linux/crates/app/src/ui/icons.rs | 2 + linux/crates/app/src/ui/workspace/imp.rs | 4 + linux/crates/app/src/ui/workspace/mod.rs | 9 ++ linux/crates/app/src/ui/workspace/shell.rs | 5 + .../app/src/ui/workspace/sidebar/mod.rs | 35 +++++- .../app/src/ui/workspace/sidebar/tests.rs | 99 +++++++++++++--- .../app/src/ui/workspace/workspace_actions.rs | 3 +- .../session/src/session/schema_catalog.rs | 31 +++++ linux/crates/session/src/session/session.rs | 4 + linux/data/resources/ui/workspace.ui | 15 +++ linux/docs/state-management.md | 2 +- linux/po/POTFILES.in | 1 + 16 files changed, 337 insertions(+), 25 deletions(-) create mode 100644 linux/crates/app/src/ui/editor/catalog_change.rs diff --git a/linux/crates/app/src/ui/editor/catalog_change.rs b/linux/crates/app/src/ui/editor/catalog_change.rs new file mode 100644 index 0000000000..54c6e75fb3 --- /dev/null +++ b/linux/crates/app/src/ui/editor/catalog_change.rs @@ -0,0 +1,110 @@ +use tablepro_core::sql_syntax::SqlGrammar; +use tablepro_core::sql_syntax::script::{LexicalSettings, ScriptPlan, ScriptTokenKind}; + +/// Words that add, rename or remove what the sidebar lists. They count +/// anywhere in a statement: a SQL Server batch needs no `;` between its +/// statements, and these words appear in no other kind of statement. +const SCHEMA_VERBS: [&str; 7] = ["CREATE", "ALTER", "DROP", "RENAME", "ATTACH", "DETACH", "IMPORT"]; + +/// DDL run inside a transaction reaches the window's own connection only +/// once it commits. +const TRANSACTION_ENDS: [&str; 2] = ["COMMIT", "END"]; + +/// Whether running `text` can change the tables the catalog lists. A +/// commit counts as the statement's first word, and `SELECT … INTO` +/// counts too, since PostgreSQL and SQL Server create its target table. +pub(super) fn changes_catalog(text: &str, grammar: SqlGrammar) -> bool { + let plan = ScriptPlan::build(text, grammar, LexicalSettings::default_for(grammar)); + let is = |word: &str, set: &[&str]| set.iter().any(|verb| word.eq_ignore_ascii_case(verb)); + plan.statements().iter().any(|statement| { + let words: Vec<&str> = plan + .tokens(statement.range.clone()) + .filter(|token| token.kind == ScriptTokenKind::Word) + .filter_map(|token| text.get(token.range)) + .collect(); + let Some(first) = words.first() else { + return false; + }; + words.iter().any(|word| is(word, &SCHEMA_VERBS)) + || is(first, &TRANSACTION_ENDS) + || (is(first, &["SELECT", "WITH"]) && words.iter().any(|word| is(word, &["INTO"]))) + }) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn ddl_changes_the_catalog_in_every_grammar() { + for grammar in SqlGrammar::ALL { + for sql in [ + "CREATE TABLE orders (id int)", + "create view v as select 1", + "ALTER TABLE orders ADD note text", + "DROP TABLE orders", + "-- a note first\nCREATE INDEX i ON orders (id)", + "SELECT 1; DROP TABLE orders", + ] { + assert!(changes_catalog(sql, grammar), "{grammar:?}: {sql}"); + } + } + } + + #[test] + fn reads_and_row_writes_leave_it_alone() { + for grammar in SqlGrammar::ALL { + for sql in [ + "SELECT * FROM orders", + "INSERT INTO orders VALUES (1)", + "UPDATE orders SET id = 2", + "DELETE FROM orders", + "SELECT 'CREATE TABLE x' AS text", + "-- DROP TABLE orders\nSELECT 1", + "", + ] { + assert!(!changes_catalog(sql, grammar), "{grammar:?}: {sql}"); + } + } + } + + #[test] + fn ddl_after_another_statement_in_a_sql_server_batch_counts() { + assert!(changes_catalog( + "INSERT INTO log VALUES (1)\nCREATE TABLE orders (id int)", + SqlGrammar::MsSql + )); + } + + #[test] + fn a_case_end_is_not_a_commit() { + assert!(!changes_catalog( + "SELECT CASE WHEN id > 1 THEN 'a' END FROM orders", + SqlGrammar::PostgreSql + )); + } + + #[test] + fn a_commit_publishes_ddl_run_in_a_transaction() { + assert!(changes_catalog("COMMIT", SqlGrammar::PostgreSql)); + assert!(changes_catalog("END", SqlGrammar::PostgreSql)); + assert!(changes_catalog("COMMIT TRANSACTION", SqlGrammar::MsSql)); + assert!(!changes_catalog("BEGIN", SqlGrammar::PostgreSql)); + } + + #[test] + fn select_into_creates_its_target() { + assert!(changes_catalog("SELECT * INTO archive FROM orders", SqlGrammar::MsSql)); + assert!(changes_catalog( + "SELECT id INTO TEMP t FROM orders", + SqlGrammar::PostgreSql + )); + assert!(!changes_catalog("SELECT id FROM orders", SqlGrammar::PostgreSql)); + } + + #[test] + fn sqlite_attach_and_rename_count() { + assert!(changes_catalog("ATTACH DATABASE 'x.db' AS x", SqlGrammar::Sqlite)); + assert!(changes_catalog("RENAME TABLE a TO b", SqlGrammar::MySql)); + } +} diff --git a/linux/crates/app/src/ui/editor/imp.rs b/linux/crates/app/src/ui/editor/imp.rs index 610873e722..b530a7ef8d 100644 --- a/linux/crates/app/src/ui/editor/imp.rs +++ b/linux/crates/app/src/ui/editor/imp.rs @@ -51,6 +51,9 @@ pub struct SqlEditor { pub grid_sink: OnceCell, /// Whether the last run left a transaction open, as last reported. pub transaction_dirty: Cell, + /// Whether the run in flight, or the last one, can change what the + /// catalog lists. + pub changes_catalog: Cell, pub shut_down: Cell, /// Theme and font handlers on the app-wide style manager and /// settings, which would otherwise outlive the tab. diff --git a/linux/crates/app/src/ui/editor/mod.rs b/linux/crates/app/src/ui/editor/mod.rs index f427c7721a..a5b84bdf5f 100644 --- a/linux/crates/app/src/ui/editor/mod.rs +++ b/linux/crates/app/src/ui/editor/mod.rs @@ -3,6 +3,7 @@ //! gets a line in the results list. mod buffer_edit; +mod catalog_change; mod completion; mod editor_actions; mod editor_session_slot; @@ -134,6 +135,12 @@ impl SqlEditor { self.imp().transaction_dirty.get() } + /// Whether the last run sent a statement that can change what the + /// catalog lists, so the window reads it again once the run ends. + pub(crate) fn last_run_changes_catalog(&self) -> bool { + self.imp().changes_catalog.get() + } + /// Stop the running query. The results list says what ran before. pub fn stop(&self) { if let Some(running) = self.imp().running.borrow().as_ref() { @@ -372,11 +379,17 @@ impl SqlEditor { .line(), ) .unwrap_or_default(); - let (batches, policy, warnings) = plan_batches(&scoped, self.grammar(), base_line); + let grammar = self.grammar(); + let (batches, policy, warnings) = plan_batches(&scoped, grammar, base_line); if batches.is_empty() { self.say(&crate::i18n::gettext("There is nothing to run.")); return; } + imp.changes_catalog.set( + batches + .iter() + .any(|batch| catalog_change::changes_catalog(&batch.text, grammar)), + ); let statements = batches.iter().map(|batch| UserSql::new(batch.text.as_str())).collect(); let run = ScriptRun { batches, diff --git a/linux/crates/app/src/ui/editor/tests.rs b/linux/crates/app/src/ui/editor/tests.rs index 2c8b62ac58..ada3c097c3 100644 --- a/linux/crates/app/src/ui/editor/tests.rs +++ b/linux/crates/app/src/ui/editor/tests.rs @@ -124,6 +124,30 @@ fn selected_text_is_the_only_sql_sent() { assert_eq!(sent(&harness.fake), ["SELECT 2"]); } +#[gtk4::test] +fn only_a_run_that_sent_ddl_says_the_catalog_changed() { + let harness = editor_with(answering(), "SELECT 1;"); + run_and_wait(&harness); + assert!( + !harness.editor.last_run_changes_catalog(), + "a SELECT changed the catalog" + ); + + harness + .editor + .buffer() + .set_text("SELECT 1;\nCREATE TABLE orders (id int);"); + run_and_wait(&harness); + assert!(harness.editor.last_run_changes_catalog()); + + harness.editor.buffer().set_text("UPDATE orders SET id = 2;"); + run_and_wait(&harness); + assert!( + !harness.editor.last_run_changes_catalog(), + "the last run's answer outlived it" + ); +} + #[gtk4::test] fn run_at_cursor_selects_executed_statement() { let harness = editor_with(answering(), "SELECT 'tên';\nSELECT 2 FROM t;\nSELECT 3;"); diff --git a/linux/crates/app/src/ui/icons.rs b/linux/crates/app/src/ui/icons.rs index ce292c2685..50a9b04a20 100644 --- a/linux/crates/app/src/ui/icons.rs +++ b/linux/crates/app/src/ui/icons.rs @@ -44,6 +44,7 @@ pub const VIEW_LIST: &str = "view-list-symbolic"; pub const VIEW_MORE: &str = "view-more-symbolic"; pub const VIEW_PAGED: &str = "view-paged-symbolic"; pub const VIEW_PIN: &str = "view-pin-symbolic"; +pub const VIEW_REFRESH: &str = "view-refresh-symbolic"; pub const VIEW_REVEAL: &str = "view-reveal-symbolic"; pub const VIEW_SORT_ASCENDING: &str = "view-sort-ascending-symbolic"; pub const WINDOW_CLOSE: &str = "window-close-symbolic"; @@ -98,6 +99,7 @@ pub const ALL: &[&str] = &[ VIEW_MORE, VIEW_PAGED, VIEW_PIN, + VIEW_REFRESH, VIEW_REVEAL, VIEW_SORT_ASCENDING, WINDOW_CLOSE, diff --git a/linux/crates/app/src/ui/workspace/imp.rs b/linux/crates/app/src/ui/workspace/imp.rs index 9d6d917dfd..ccc04d4477 100644 --- a/linux/crates/app/src/ui/workspace/imp.rs +++ b/linux/crates/app/src/ui/workspace/imp.rs @@ -34,10 +34,14 @@ pub struct Workspace { #[template_child] pub table_search_toggle: TemplateChild, #[template_child] + pub refresh_tables_button: TemplateChild, + #[template_child] pub table_search_bar: TemplateChild, #[template_child] pub table_search: TemplateChild, #[template_child] + pub catalog_banner: TemplateChild, + #[template_child] pub sidebar_slot: TemplateChild, #[template_child] pub content_root: TemplateChild, diff --git a/linux/crates/app/src/ui/workspace/mod.rs b/linux/crates/app/src/ui/workspace/mod.rs index f39644dee9..a4898f9047 100644 --- a/linux/crates/app/src/ui/workspace/mod.rs +++ b/linux/crates/app/src/ui/workspace/mod.rs @@ -121,6 +121,9 @@ pub(crate) struct WorkspaceModel { all_schemas_listed: bool, toast_overlay: adw::ToastOverlay, reconnect_banner: adw::Banner, + /// Says the sidebar's list could not be read again, until a read + /// lands. + catalog_banner: adw::Banner, row_op_spinner: adw::Spinner, table_search: gtk::SearchEntry, /// Removes a changed host key from known_hosts. `None` without ssh. @@ -358,6 +361,9 @@ pub(crate) enum WorkspaceInput { CatalogChanged(ObjectListSnapshot), /// A refresh of that list failed, and the list stays as it was. CatalogReadFailed(String), + /// Read the session's tables again: the sidebar's Refresh, or a run + /// in the editor that changed them. + RefreshCatalog, /// The default schema's list landed, as a reconnect commits it. DefaultListCommitted(ObjectListSnapshot), /// The schemas the sidebar's sections are made from, read for @@ -545,6 +551,7 @@ impl Workspace { all_schemas_listed: false, toast_overlay: imp.toast_overlay.get(), reconnect_banner: imp.reconnect_banner.get(), + catalog_banner: imp.catalog_banner.get(), row_op_spinner: imp.row_op_spinner.get(), table_search: imp.table_search.get(), ssh: services.ssh(), @@ -910,6 +917,7 @@ impl WorkspaceModel { } WorkspaceInput::CatalogChanged(snapshot) => self.on_catalog_changed(snapshot), WorkspaceInput::CatalogReadFailed(message) => self.on_catalog_read_failed(&message), + WorkspaceInput::RefreshCatalog => self.refresh_catalog(), WorkspaceInput::DefaultListCommitted(snapshot) => self.on_default_list_committed(snapshot), WorkspaceInput::SidebarRead { read, objects, schemas } => self.on_sidebar_read(read, &objects, schemas), WorkspaceInput::LinkChanged => self.on_link_changed(), @@ -944,6 +952,7 @@ impl WorkspaceModel { WorkspaceInput::EditorTabRunStateChanged(id, running) => self.on_editor_tab_run_state_changed(id, running), WorkspaceInput::EditorTabRunFinished(id, outcome) => { self.report_work(id, crate::workspace::WorkKind::Query, outcome); + self.on_editor_run_finished(id); } WorkspaceInput::EditorTabContentChanged(id) => self.on_editor_tab_content_changed(id), WorkspaceInput::WorkspaceLoaded(loaded) => self.on_workspace_loaded(loaded), diff --git a/linux/crates/app/src/ui/workspace/shell.rs b/linux/crates/app/src/ui/workspace/shell.rs index ef8d8d70d0..a30c714bfa 100644 --- a/linux/crates/app/src/ui/workspace/shell.rs +++ b/linux/crates/app/src/ui/workspace/shell.rs @@ -48,6 +48,11 @@ pub(super) fn describe_buttons(workspace: &Workspace) { &crate::i18n::gettext("Search Tables"), None, ); + describe_icon_button( + &imp.refresh_tables_button.get(), + &crate::i18n::gettext("Refresh Tables"), + Some("win.refresh-tables"), + ); describe_icon_button( &imp.sidebar_toggle.get(), &crate::i18n::gettext("Show Sidebar"), diff --git a/linux/crates/app/src/ui/workspace/sidebar/mod.rs b/linux/crates/app/src/ui/workspace/sidebar/mod.rs index a78a1785c1..13b652787c 100644 --- a/linux/crates/app/src/ui/workspace/sidebar/mod.rs +++ b/linux/crates/app/src/ui/workspace/sidebar/mod.rs @@ -189,14 +189,41 @@ impl WorkspaceModel { /// first read. pub(super) fn on_catalog_changed(&mut self, snapshot: ObjectListSnapshot) { self.all_schemas_listed = true; + self.catalog_banner.set_revealed(false); self.read_sidebar(snapshot.objects); } + /// The list on screen stays, under a banner that says why it could + /// not be read again and stays until a read lands. The banner wraps + /// to three lines, so the whole reason is in its tooltip too. pub(super) fn on_catalog_read_failed(&self, message: &str) { - self.show_toast(&crate::i18n::gettext_f( - "Could not refresh the table list: {error}", - &[("error", message)], - )); + let title = crate::i18n::gettext_f("Could not refresh the table list: {error}", &[("error", message)]); + self.catalog_banner.set_title(&title); + self.catalog_banner.set_tooltip_text(Some(&title)); + self.catalog_banner.set_revealed(true); + } + + /// Read the session's tables again, and every table's columns on + /// their next ask: what changed the schema did not say which tables + /// it touched. + pub(super) fn refresh_catalog(&self) { + let Some(session) = self.context.active.get() else { + return; + }; + session.invalidate_columns(); + self.on_schema_changed(None, None); + } + + /// A run that sent DDL, or a commit that may publish it, changes what + /// the sidebar and the completion list. + pub(super) fn on_editor_run_finished(&self, id: uuid::Uuid) { + let changed = matches!( + self.workspace_tabs.borrow().get(&id), + Some(super::WorkspaceTab::Editor(slot)) if slot.editor.last_run_changes_catalog() + ); + if changed { + self.refresh_catalog(); + } } /// A reconnect committed the default schema's list and its schemas, diff --git a/linux/crates/app/src/ui/workspace/sidebar/tests.rs b/linux/crates/app/src/ui/workspace/sidebar/tests.rs index e919be8294..4b68c19dc4 100644 --- a/linux/crates/app/src/ui/workspace/sidebar/tests.rs +++ b/linux/crates/app/src/ui/workspace/sidebar/tests.rs @@ -568,39 +568,102 @@ fn sidebar_lists_every_schema_after_the_connect() { } #[gtk4::test] -fn catalog_error_keeps_list_and_toasts() { +fn catalog_error_keeps_list_and_shows_why_until_a_read_lands() { let test = TestWindow::connected_with("SidebarCatalogError", |script| { script.list_objects.once(FakeReply::ok(vec![ object("public", "orders"), object("public", "users"), ])); - script - .list_objects - .once(FakeReply::err(DriverError::Protocol("catalog gone".to_owned()))); + for _ in 0..2 { + script + .list_objects + .once(FakeReply::err(DriverError::Protocol("catalog gone".to_owned()))); + } + for _ in 0..3 { + script.list_objects.once(FakeReply::ok(vec![ + object("public", "orders"), + object("public", "users"), + object("public", "carts"), + ])); + } }); let workspace = test.workspace(); wait_until(WAIT, || workspace.read(|model| model.sidebar.shown()) == Some(2)).expect("the tables listed"); - let toasts_before = toast_count(&workspace); + let banner = workspace.imp().catalog_banner.get(); + assert!(!banner.is_revealed(), "a banner before anything failed"); - let input = workspace.input().expect("the workspace's input"); - input.deliver(WorkspaceInput::SchemaChanged { - schema: Some("public".to_owned()), - table: None, - }); + WidgetExt::activate_action(&test.window, "win.refresh-tables", None).expect("win.refresh-tables"); - wait_until(WAIT, || toast_count(&workspace) > toasts_before).expect("a toast for the failed refresh"); + wait_until(WAIT, || banner.is_revealed()).expect("a banner for the failed refresh"); + assert!( + banner.title().starts_with("Could not refresh the table list: "), + "{}", + banner.title() + ); + assert_eq!(banner.action_name().as_deref(), Some("win.refresh-tables")); assert_eq!( workspace.read(|model| model.sidebar.shown()), Some(2), "the failure emptied the list" ); + + WidgetExt::activate_action(&test.window, "win.refresh-tables", None).expect("the banner's Retry"); + + wait_until(WAIT, || workspace.read(|model| model.sidebar.shown()) == Some(3)).expect("the list read again"); + assert!(!banner.is_revealed(), "the banner stayed after a read landed"); } -fn toast_count(workspace: &crate::ui::workspace::Workspace) -> usize { - descendants(&workspace.imp().toast_overlay.get()) - .into_iter() - .filter(|widget| widget.css_name() == "toast") - .count() +#[gtk4::test] +fn the_refresh_button_reads_the_tables_again() { + let test = TestWindow::connected_with("SidebarRefresh", |script| { + script.list_objects.once(FakeReply::ok(vec![object("public", "users")])); + for _ in 0..3 { + script.list_objects.once(FakeReply::ok(vec![ + object("public", "users"), + object("public", "orders"), + ])); + } + }); + let workspace = test.workspace(); + wait_until(WAIT, || workspace.read(|model| model.sidebar.shown()) == Some(1)).expect("the table listed"); + let button = workspace.imp().refresh_tables_button.get(); + assert_eq!(button.action_name().as_deref(), Some("win.refresh-tables")); + crate::test_support::assert_labelled(&button).expect("the refresh button is named"); + + button.emit_clicked(); + + wait_until(WAIT, || workspace.read(|model| model.sidebar.shown()) == Some(2)).expect("the new table listed"); +} + +#[gtk4::test] +fn ddl_run_in_the_editor_reads_the_tables_again() { + use tablepro_core::EditorSession; + use tablepro_test_support::{FakeEditorSession, FakeRun, Replies}; + + let mut runs = Replies::default(); + runs.always(FakeRun::completed(Vec::new())); + let editor_session: std::sync::Arc = std::sync::Arc::new(FakeEditorSession::new(runs)); + let test = TestWindow::connected_with("SidebarEditorDdl", |script| { + script.open_editor_session.always(FakeReply::ok(editor_session)); + script.list_objects.once(FakeReply::ok(vec![object("public", "users")])); + for _ in 0..3 { + script.list_objects.once(FakeReply::ok(vec![ + object("public", "users"), + object("public", "orders"), + ])); + } + }); + let workspace = test.workspace(); + wait_until(WAIT, || workspace.read(|model| model.sidebar.shown()) == Some(1)).expect("the table listed"); + workspace.with_model(|model| model.open_editor_tab(Some("CREATE TABLE orders (id int)".to_owned()))); + let editor = crate::ui::session_window::tests::editors(&test.window) + .pop() + .expect("the editor tab"); + + editor.run(); + + wait_until(WAIT, || workspace.read(|model| model.sidebar.shown()) == Some(2)) + .expect("the created table listed without a reconnect"); } #[gtk4::test] @@ -649,14 +712,14 @@ fn a_reconnect_names_tabs_by_its_schemas_when_the_list_fails() { .read(|model| model.context.active.get()) .flatten() .expect("the session"); - let toasts_before = toast_count(&workspace); + let banner = workspace.imp().catalog_banner.get(); session.catalog().commit( vec![schema("public", true), schema("sales", false)], vec![object("public", "users")], ); - wait_until(WAIT, || toast_count(&workspace) > toasts_before).expect("the list read to fail"); + wait_until(WAIT, || banner.is_revealed()).expect("the list read to fail"); wait_until(WAIT, || naming() == Some(TableNaming::SchemaQualified)) .expect("the reconnect's schemas never named the tabs"); assert_eq!( diff --git a/linux/crates/app/src/ui/workspace/workspace_actions.rs b/linux/crates/app/src/ui/workspace/workspace_actions.rs index 11a27dae62..d18a598c7c 100644 --- a/linux/crates/app/src/ui/workspace/workspace_actions.rs +++ b/linux/crates/app/src/ui/workspace/workspace_actions.rs @@ -22,8 +22,9 @@ const HISTORY_ACTIONS: [&str; 2] = ["show-history", "show-saved-queries"]; /// Each one sends its input through `input`, so one activated after the /// workspace is gone is dropped rather than handled. pub(super) fn install(window: &adw::ApplicationWindow, input: &WorkspaceSender) -> Vec<&'static str> { - let entries: [(&'static str, ActionInput); 6] = [ + let entries: [(&'static str, ActionInput); 7] = [ ("new-editor-tab", || WorkspaceInput::NewEditorTab), + ("refresh-tables", || WorkspaceInput::RefreshCatalog), ("reopen-closed-tab", || WorkspaceInput::ReopenClosedTab), ("save-query", || WorkspaceInput::SaveActiveQuery), ("show-history", || WorkspaceInput::ShowHistory), diff --git a/linux/crates/session/src/session/schema_catalog.rs b/linux/crates/session/src/session/schema_catalog.rs index 4a43d8f271..5ffda77174 100644 --- a/linux/crates/session/src/session/schema_catalog.rs +++ b/linux/crates/session/src/session/schema_catalog.rs @@ -228,6 +228,13 @@ impl SchemaCatalog { } } + /// Something changed the schema without saying which tables: every + /// table's columns are read again on the next ask, and until then + /// the old ones stay. + pub fn invalidate_columns(&self) { + mark_all_stale(&mut self.lock().columns); + } + /// The scope's object list now and after every change. Subscribing /// does not read anything; `objects` or `refresh_objects` does. pub fn subscribe(&self, scope: ObjectScope) -> watch::Receiver { @@ -611,6 +618,30 @@ mod tests { assert_eq!(CallCounters::get(&harness.connection.counters().fetch_columns), 2); } + #[tokio::test(start_paused = true)] + async fn invalidating_every_table_reads_each_again_on_its_next_ask() { + let mut script = ConnectionScript::default(); + script + .fetch_columns + .when_sql_contains("users", FakeReply::ok(vec![column("id")])) + .when_sql_contains("orders", FakeReply::ok(vec![column("total")])); + let harness = harness(script); + let (users, orders) = (TableRef::unqualified("users"), TableRef::unqualified("orders")); + harness.catalog.columns(users.clone()).await.unwrap(); + harness.catalog.columns(orders.clone()).await.unwrap(); + + harness.catalog.invalidate_columns(); + + assert_eq!( + names(&harness.catalog.cached_columns(&users).unwrap(), |c| &c.name), + ["id"], + "the invalidation emptied the list" + ); + harness.catalog.columns(users).await.unwrap(); + harness.catalog.columns(orders).await.unwrap(); + assert_eq!(CallCounters::get(&harness.connection.counters().fetch_columns), 4); + } + #[tokio::test(start_paused = true)] async fn a_fetch_started_before_an_invalidation_does_not_land() { let mut script = ConnectionScript::default(); diff --git a/linux/crates/session/src/session/session.rs b/linux/crates/session/src/session/session.rs index 1f3cec1bde..b25d0d1ee2 100644 --- a/linux/crates/session/src/session/session.rs +++ b/linux/crates/session/src/session/session.rs @@ -200,6 +200,10 @@ impl Session { self.catalog.invalidate_table(table); } + pub fn invalidate_columns(&self) { + self.catalog.invalidate_columns(); + } + pub fn subscribe_objects(&self, scope: ObjectScope) -> watch::Receiver { self.catalog.subscribe(scope) } diff --git a/linux/data/resources/ui/workspace.ui b/linux/data/resources/ui/workspace.ui index 00f5c86400..2e30ae08a3 100644 --- a/linux/data/resources/ui/workspace.ui +++ b/linux/data/resources/ui/workspace.ui @@ -50,6 +50,12 @@ system-search-symbolic + + + view-refresh-symbolic + win.refresh-tables + + @@ -67,6 +73,15 @@ + + + + false + Retry + win.refresh-tables + + true diff --git a/linux/docs/state-management.md b/linux/docs/state-management.md index cfda3a4037..b5a29ab721 100644 --- a/linux/docs/state-management.md +++ b/linux/docs/state-management.md @@ -35,7 +35,7 @@ Windows share nothing but `AppServices`. CI fails on a `thread_local!` under `cr **Content stays through a failed read.** A browse or structure tab tracks its read with `ContentLoad`. The spinner shows only until the first read lands; a read of the content on screen keeps it there and spins the tab's page in the tab bar, and a failure then leaves it under a `StaleBanner` with Retry. The error page, with Try Again, is for a tab with nothing to show, or whose failed read asked for other rows than the ones on screen. -**The catalog belongs to the session.** `SchemaCatalog` (`crates/session/src/session/schema_catalog.rs`) holds the schemas, each scope's objects and each table's columns. Views read through `Session::schemas`, `objects` and `columns`, or `refresh_objects` and `refresh_columns` to fetch again. Callers that ask for the same thing at once share one fetch, a fetch runs as its own task so a view that closes does not stop it, and a failed refresh keeps the list it had. A schema change calls `invalidate_table`, which marks the table stale and keeps its data until the next fetch lands. +**The catalog belongs to the session.** `SchemaCatalog` (`crates/session/src/session/schema_catalog.rs`) holds the schemas, each scope's objects and each table's columns. Views read through `Session::schemas`, `objects` and `columns`, or `refresh_objects` and `refresh_columns` to fetch again. Callers that ask for the same thing at once share one fetch, a fetch runs as its own task so a view that closes does not stop it, and a failed refresh keeps the list it had. A schema change calls `invalidate_table`, which marks the table stale and keeps its data until the next fetch lands. The sidebar's Refresh, and an editor run that sent DDL or a commit (`catalog_change::changes_catalog`), cannot name the tables they touched: they call `invalidate_columns`, which does the same for every table, and read the object lists again. A failed read leaves the sidebar's list under a banner with Retry until a read lands. **Saves go through the workspace.** `WorkspaceHandle` starts, stops and settles every save. Its `SaveLedger` records each running save with its tab, its kind, a cancel token and a process hold, and refuses a second save of the same kind for the same tab. Stop Saving, or a close that chose to stop, cancels the token, which reaches the driver through `run_cancellable`; the save settles when the driver answers. diff --git a/linux/po/POTFILES.in b/linux/po/POTFILES.in index 66ad367755..984ffd1c54 100644 --- a/linux/po/POTFILES.in +++ b/linux/po/POTFILES.in @@ -187,6 +187,7 @@ crates/app/src/ui/credential_prompt/prompt_text.rs crates/app/src/ui/credential_prompt/prompter.rs crates/app/src/ui/delete_connection_dialog.rs crates/app/src/ui/editor/buffer_edit.rs +crates/app/src/ui/editor/catalog_change.rs crates/app/src/ui/editor/completion/catalog_read.rs crates/app/src/ui/editor/completion/completion_scope.rs crates/app/src/ui/editor/completion/fuzzy.rs From 938333cde3aa4f5db274b109f2867ec5e778b3cf Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Sat, 17 Oct 2026 18:15:23 +0700 Subject: [PATCH 02/14] feat(linux): save the SQL editor to a .sql file with Ctrl+S --- .../app/src/application/accelerators.rs | 22 +- .../app/src/services/connection_deletion.rs | 2 +- linux/crates/app/src/test_support.rs | 10 + .../app/src/ui/editor/editor_actions.rs | 2 + linux/crates/app/src/ui/editor/editor_file.rs | 197 ++++++++++++++++++ linux/crates/app/src/ui/editor/imp.rs | 9 +- linux/crates/app/src/ui/editor/mod.rs | 12 +- linux/crates/app/src/ui/editor/tests.rs | 83 +++++++- .../crates/app/src/ui/workspace/close_flow.rs | 6 +- .../app/src/ui/workspace/close_tab_tests.rs | 79 +++++++ .../app/src/ui/workspace/editor_label.rs | 12 ++ linux/crates/app/src/ui/workspace/mod.rs | 5 + linux/crates/app/src/ui/workspace/saves.rs | 2 +- .../app/src/ui/workspace/tab_factory.rs | 34 ++- linux/crates/app/src/ui/workspace/tab_spec.rs | 3 + .../crates/app/src/ui/workspace/tab_title.rs | 18 ++ .../app/src/ui/workspace/tab_title_tests.rs | 51 ++++- .../app/src/ui/workspace/window_tabs.rs | 23 +- .../src/ui/workspace/workspace_persistence.rs | 12 +- .../app/src/ui/workspace/workspace_tabs.rs | 47 ++++- linux/crates/app/src/workspace/close_guard.rs | 10 +- linux/crates/app/src/workspace/close_step.rs | 26 ++- linux/crates/app/src/workspace/dirty_kind.rs | 12 ++ .../app/src/workspace/editor_file_record.rs | 11 + linux/crates/app/src/workspace/mod.rs | 2 + .../app/src/workspace/save_rejection.rs | 3 + linux/crates/app/src/workspace/save_start.rs | 2 + .../crates/app/src/workspace/tab_directory.rs | 12 +- linux/crates/app/src/workspace/tab_entry.rs | 2 + linux/crates/app/src/workspace/tab_record.rs | 6 +- .../app/src/workspace/test_workspace.rs | 3 + .../app/src/workspace/workspace_handle.rs | 34 ++- .../app/src/workspace/workspace_record.rs | 10 +- .../src/workspace/workspace_record_builder.rs | 8 +- .../app/src/workspace/workspace_store.rs | 12 +- linux/data/resources/shortcuts-dialog.ui | 24 ++- linux/data/resources/ui/sql-editor.ui | 13 ++ linux/docs/state-management.md | 6 + linux/po/POTFILES.in | 2 + 39 files changed, 762 insertions(+), 65 deletions(-) create mode 100644 linux/crates/app/src/ui/editor/editor_file.rs create mode 100644 linux/crates/app/src/workspace/editor_file_record.rs diff --git a/linux/crates/app/src/application/accelerators.rs b/linux/crates/app/src/application/accelerators.rs index 110bcd4b30..601df9e9f8 100644 --- a/linux/crates/app/src/application/accelerators.rs +++ b/linux/crates/app/src/application/accelerators.rs @@ -223,6 +223,27 @@ pub const ACCELERATORS: &[Accelerator] = &[ &["slash"], Listed(ShortcutSection::SqlEditor), ), + local( + SqlEditorTab, + BUBBLE, + "editor.open-file", + &["o"], + Listed(ShortcutSection::SqlEditor), + ), + local( + SqlEditorTab, + BUBBLE, + "editor.save", + &["s"], + Listed(ShortcutSection::SqlEditor), + ), + local( + SqlEditorTab, + BUBBLE, + "editor.save-as", + &["s"], + Listed(ShortcutSection::SqlEditor), + ), // GtkSearchEntry binds Ctrl+G and Ctrl+Shift+G itself, and the find // bar follows its next-match and previous-match signals. local( @@ -253,7 +274,6 @@ pub const ACCELERATORS: &[Accelerator] = &[ &["g"], Listed(ShortcutSection::SqlEditor), ), - application("win.save-query", &["s"], ShortcutSection::SqlEditor), local( StructureTab, BUBBLE, diff --git a/linux/crates/app/src/services/connection_deletion.rs b/linux/crates/app/src/services/connection_deletion.rs index 2171d0c8b3..08107783d8 100644 --- a/linux/crates/app/src/services/connection_deletion.rs +++ b/linux/crates/app/src/services/connection_deletion.rs @@ -243,7 +243,7 @@ mod tests { &key, WorkspaceRecord { active_tab: Some(0), - tabs: vec![TabRecord::Editor { draft }], + tabs: vec![TabRecord::Editor { draft, file: None }], }, ) .expect("scheduled"); diff --git a/linux/crates/app/src/test_support.rs b/linux/crates/app/src/test_support.rs index 822301c502..22536db370 100644 --- a/linux/crates/app/src/test_support.rs +++ b/linux/crates/app/src/test_support.rs @@ -52,6 +52,16 @@ pub(crate) fn paused_tasks() -> tablepro_session::runtime::Tasks { tablepro_session::runtime::Tasks::new(tokio::runtime::Handle::current()) } +/// Write a file a test opens. Kept here because CI's check for blocking +/// file I/O reads every UI file not named `tests.rs`. +pub(crate) fn write_text_file(path: &std::path::Path, text: &str) { + std::fs::write(path, text).expect("write the test's file"); +} + +pub(crate) fn read_text_file(path: &std::path::Path) -> String { + std::fs::read_to_string(path).expect("read the test's file") +} + /// A runtime a GTK test can borrow `Tasks` from. /// /// A `#[gtk4::test]` has no ambient tokio runtime, so it owns one for diff --git a/linux/crates/app/src/ui/editor/editor_actions.rs b/linux/crates/app/src/ui/editor/editor_actions.rs index 850cd96317..e7e8818489 100644 --- a/linux/crates/app/src/ui/editor/editor_actions.rs +++ b/linux/crates/app/src/ui/editor/editor_actions.rs @@ -23,6 +23,8 @@ impl SqlEditor { }; actions.add_action_entries([ entry("open-file", SqlEditor::open_file), + entry("save", SqlEditor::save), + entry("save-as", SqlEditor::save_as), entry("run", SqlEditor::run), entry("run-statement", SqlEditor::run_at_cursor), entry("cancel", SqlEditor::stop), diff --git a/linux/crates/app/src/ui/editor/editor_file.rs b/linux/crates/app/src/ui/editor/editor_file.rs new file mode 100644 index 0000000000..28d60b19cb --- /dev/null +++ b/linux/crates/app/src/ui/editor/editor_file.rs @@ -0,0 +1,197 @@ +//! The `.sql` file an editor tab is opened from or saved to. + +use gtk4::prelude::*; +use gtk4::subclass::prelude::*; +use gtk4::{gio, glib}; +use libadwaita as adw; +use libadwaita::prelude::*; + +use super::{SqlEditor, buffer_text, export_name_for_query}; + +impl SqlEditor { + /// The file the tab's text is saved to, once it was opened from or + /// saved to one. + pub(crate) fn file(&self) -> Option { + self.imp().file.borrow().clone() + } + + /// Whether the text differs from what the tab's file last held. + pub(crate) fn has_unsaved_file_changes(&self) -> bool { + self.imp().file.borrow().is_some() && self.buffer().is_modified() + } + + /// Point a restored tab at its file again, with its text changed + /// since the last save or not. + pub(crate) fn restore_file(&self, file: gio::File, modified: bool) { + self.imp().file.replace(Some(file)); + self.buffer().set_modified(modified); + self.emit_by_name::<()>("file-changed", &[]); + } + + /// A file or its changed state changed. + pub(crate) fn connect_file_changed(&self, f: F) -> glib::SignalHandlerId { + self.connect_closure( + "file-changed", + false, + glib::closure_local!(move |editor: &Self| f(editor)), + ) + } + + /// The buffer's own modified flag says whether the text changed + /// since the file was read or written; Undo back to that point + /// clears it again. + pub(super) fn follow_file_state(&self, buffer: &sourceview5::Buffer) { + let weak = self.downgrade(); + buffer.connect_modified_changed(move |_| { + if let Some(editor) = weak.upgrade() + && editor.imp().file.borrow().is_some() + { + editor.emit_by_name::<()>("file-changed", &[]); + } + }); + let weak = self.downgrade(); + buffer.connect_changed(move |_| { + if let Some(editor) = weak.upgrade() { + let edits = &editor.imp().edits; + edits.set(edits.get().wrapping_add(1)); + } + }); + } + + /// The text now came whole from `file`. + pub(super) fn opened_file(&self, file: gio::File) { + self.imp().file.replace(Some(file)); + self.buffer().set_modified(false); + self.emit_by_name::<()>("file-changed", &[]); + } + + /// 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(), + } + } + + /// Ctrl+Shift+S: ask where to write, starting from the tab's file. + pub(super) fn save_as(&self) { + let filter = sql_filter(); + let filters = gio::ListStore::new::(); + filters.append(&filter); + let dialog = gtk4::FileDialog::builder() + .title(crate::i18n::gettext("Save SQL File")) + .modal(true) + .default_filter(&filter) + .filters(&filters) + .build(); + match self.file() { + Some(file) => dialog.set_initial_file(Some(&file)), + 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); + } + }); + } + + /// Write the text to the tab's file, for a close that chose Save. + /// The error is the sentence to show. + pub(crate) fn save_to_file(&self) -> impl Future> + use<> { + let editor = self.clone(); + async move { + match editor.file() { + Some(file) => editor.write_to(&file).await, + None => Ok(()), + } + } + } + + 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); + } + }); + } + + /// 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> { + let imp = self.imp(); + if imp.saving.replace(true) { + return Err(crate::i18n::gettext("This tab is already being saved.")); + } + let buffer = self.buffer(); + let edits = imp.edits.get(); + let written = crate::ui::file_write::replace_file(file, buffer_text(&buffer).into_bytes()).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())], + )); + } + imp.file.replace(Some(file.clone())); + if imp.edits.get() == edits { + buffer.set_modified(false); + } + self.emit_by_name::<()>("file-changed", &[]); + Ok(()) + } + + 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")); + dialog.set_close_response("close"); + dialog.present(Some(self)); + } +} + +pub(super) fn sql_filter() -> gtk4::FileFilter { + let filter = gtk4::FileFilter::new(); + filter.set_name(Some(&crate::i18n::gettext("SQL files"))); + filter.add_suffix("sql"); + filter.add_mime_type("application/sql"); + filter +} + +/// What a tab bound to `file` is called. +pub(crate) fn file_name(file: &gio::File) -> String { + file.basename() + .map(|name| name.to_string_lossy().into_owned()) + .unwrap_or_else(|| file.parse_name().to_string()) +} + +/// A file name for a tab that has none, from its first statement. +fn suggested_file_name(text: &str) -> String { + let stem = match text.trim().is_empty() { + true => crate::i18n::gettext("query"), + false => export_name_for_query(text), + }; + format!("{stem}.sql") +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn a_new_file_is_named_after_its_first_statement() { + assert_eq!(suggested_file_name("SELECT * FROM users;"), "select-from-users.sql"); + assert_eq!(suggested_file_name(" \n"), "query.sql"); + } + + #[gtk4::test] + fn a_file_is_named_by_its_last_part() { + assert_eq!(file_name(&gio::File::for_path("/tmp/reports/daily.sql")), "daily.sql"); + } +} diff --git a/linux/crates/app/src/ui/editor/imp.rs b/linux/crates/app/src/ui/editor/imp.rs index b530a7ef8d..fe943e740e 100644 --- a/linux/crates/app/src/ui/editor/imp.rs +++ b/linux/crates/app/src/ui/editor/imp.rs @@ -2,9 +2,9 @@ use std::cell::{Cell, OnceCell, RefCell}; use std::sync::{Arc, OnceLock}; use glib::subclass::Signal; -use gtk4::glib; use gtk4::prelude::*; use gtk4::subclass::prelude::*; +use gtk4::{gio, glib}; use libadwaita as adw; use libadwaita::subclass::prelude::*; use tokio_util::sync::CancellationToken; @@ -55,6 +55,12 @@ pub struct SqlEditor { /// catalog lists. pub changes_catalog: Cell, pub shut_down: Cell, + /// The file the text was opened from or last saved to. + pub file: RefCell>, + pub saving: Cell, + /// Counts every change to the buffer, so a save knows whether the + /// text it wrote is still what the tab holds. + pub edits: Cell, /// Theme and font handlers on the app-wide style manager and /// settings, which would otherwise outlive the tab. pub(super) app_handlers: RefCell>, @@ -103,6 +109,7 @@ impl ObjectImpl for SqlEditor { Signal::builder("transaction-changed") .param_types([bool::static_type()]) .build(), + Signal::builder("file-changed").build(), ] }) } diff --git a/linux/crates/app/src/ui/editor/mod.rs b/linux/crates/app/src/ui/editor/mod.rs index a5b84bdf5f..36e760251e 100644 --- a/linux/crates/app/src/ui/editor/mod.rs +++ b/linux/crates/app/src/ui/editor/mod.rs @@ -6,6 +6,7 @@ mod buffer_edit; mod catalog_change; mod completion; mod editor_actions; +mod editor_file; mod editor_session_slot; mod find_bar; mod find_state; @@ -43,6 +44,7 @@ use tablepro_session::runtime::{ProcessHold, Tasks}; use tokio_util::sync::CancellationToken; use self::buffer_edit::{CursorAnchor, replace_all_undoable}; +pub(crate) use self::editor_file::file_name; use self::imp::RunningScript; use self::result_widgets::PageActions; use self::run_log::{EntryKind, RunEvent, RunLog}; @@ -113,6 +115,7 @@ impl SqlEditor { editor.connect_theme(&init.settings); editor.connect_completion(&buffer); editor.connect_buffer(&buffer); + editor.follow_file_state(&buffer); editor.install_actions(); editor.install_drop_target(); editor.connect_buttons(); @@ -255,7 +258,8 @@ impl SqlEditor { } /// Read a dropped or chosen file and put it in the buffer: the whole - /// buffer when it is empty, at the cursor otherwise. + /// buffer when it is empty, at the cursor otherwise. A file that + /// fills the buffer is the one Save writes back to. pub async fn insert_file(&self, file: gio::File, replace: bool) { let text = match sql_file::load_text(&file).await { Ok(text) => text, @@ -267,6 +271,7 @@ impl SqlEditor { let buffer = self.buffer(); if replace || buffer_text(&buffer).trim().is_empty() { replace_all_undoable(&buffer, &text); + self.opened_file(file); } else { buffer.begin_user_action(); buffer.insert_at_cursor(&text); @@ -795,10 +800,7 @@ impl SqlEditor { } fn choose_file(&self) { - let filter = gtk4::FileFilter::new(); - filter.set_name(Some(&crate::i18n::gettext("SQL files"))); - filter.add_suffix("sql"); - filter.add_mime_type("application/sql"); + let filter = editor_file::sql_filter(); let filters = gio::ListStore::new::(); filters.append(&filter); let dialog = gtk4::FileDialog::builder() diff --git a/linux/crates/app/src/ui/editor/tests.rs b/linux/crates/app/src/ui/editor/tests.rs index ada3c097c3..e929044830 100644 --- a/linux/crates/app/src/ui/editor/tests.rs +++ b/linux/crates/app/src/ui/editor/tests.rs @@ -882,6 +882,85 @@ fn the_editor_menu_shows_the_tables_local_keys() { assert!(checked > 0, "no menu item has a local key, so the check reads nothing"); } +/// Load `path` into the whole buffer, as Open SQL File does. +fn open_path(harness: &Harness, path: &std::path::Path) { + let (editor, file) = (harness.editor.clone(), gio::File::for_path(path)); + crate::test_support::finish_on_main(WAIT, async move { editor.insert_file(file, true).await }) + .expect("the file to load"); +} + +#[gtk4::test] +fn ctrl_s_writes_the_opened_file_and_marks_the_text_saved() { + 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(), ""); + let changes = SignalLog::connect(&harness.editor, "file-changed", |_| ()); + open_path(&harness, &path); + assert_eq!(harness.editor.file().and_then(|file| file.path()), Some(path.clone())); + assert!( + !harness.editor.has_unsaved_file_changes(), + "the text just read counts as changed" + ); + + let buffer = harness.editor.buffer(); + buffer.insert(&mut buffer.end_iter(), "\nSELECT 2;"); + assert!(harness.editor.has_unsaved_file_changes()); + + assert!(press_on_tab(&harness, "s"), "Ctrl+S did nothing"); + 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!(changes.count() >= 3, "the workspace was not told: {}", changes.count()); +} + +#[gtk4::test] +fn undo_back_to_the_saved_text_is_saved_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); + let buffer = harness.editor.buffer(); + + buffer.begin_user_action(); + buffer.insert(&mut buffer.end_iter(), " -- note"); + buffer.end_user_action(); + assert!(harness.editor.has_unsaved_file_changes()); + buffer.undo(); + + assert!(!harness.editor.has_unsaved_file_changes()); +} + +#[gtk4::test] +fn a_file_that_cannot_be_written_keeps_the_text_changed_and_says_why() { + 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 saving = harness.editor.save_to_file(); + let failed = crate::test_support::finish_on_main(WAIT, saving).expect("the save to end"); + + let message = failed.expect_err("a folder that is not there cannot be written to"); + assert!(message.starts_with("“daily.sql” could not be saved: "), "{message}"); + assert!(harness.editor.has_unsaved_file_changes()); +} + +#[gtk4::test] +fn a_tab_with_no_file_has_nothing_unsaved_to_ask_about() { + let harness = editor_with(answering(), "SELECT 1;"); + let buffer = harness.editor.buffer(); + buffer.insert(&mut buffer.end_iter(), "\nSELECT 2;"); + + assert!(harness.editor.file().is_none()); + assert!(!harness.editor.has_unsaved_file_changes()); +} + fn find_bar(harness: &Harness) -> find_bar::FindBar { harness.editor.imp().find_bar.get() } @@ -1100,7 +1179,7 @@ fn escape_hides_bar_and_disables_highlight() { /// from the focus binds it first. The search entry binds Ctrl+G, which /// the bar follows through the entry's own signals. #[gtk4::test] -fn find_keys_are_not_class_bindings_inside_the_tab() { +fn tab_keys_are_not_class_bindings_inside_the_tab() { use crate::application::{ACCELERATORS, AcceleratorScope, ShortcutOwner}; use crate::test_support::shortcut_keys::{class_keys_on_focus_paths, keystroke}; let harness = editor_with(answering(), "SELECT 1"); @@ -1133,7 +1212,7 @@ fn find_keys_are_not_class_bindings_inside_the_tab() { checked += 1; } } - assert_eq!(checked, 4); + assert_eq!(checked, 7); } /// These press real keys on the X display every test shares; nextest runs diff --git a/linux/crates/app/src/ui/workspace/close_flow.rs b/linux/crates/app/src/ui/workspace/close_flow.rs index 7ac36e62ad..d0e74a827f 100644 --- a/linux/crates/app/src/ui/workspace/close_flow.rs +++ b/linux/crates/app/src/ui/workspace/close_flow.rs @@ -147,11 +147,12 @@ impl WorkspaceModel { /// Mark each tab that closing would lose something in, and tell the /// window whether a logout would, so it holds the session back. pub(super) fn refresh_unsaved(&self) { - let unsaved: HashSet = self.workspace_handle.dirty_tabs().iter().map(|tab| tab.id).collect(); + let dirty_tabs = self.workspace_handle.dirty_tabs(); + let unsaved: HashSet = dirty_tabs.iter().map(|tab| tab.id).collect(); for (id, tab) in self.workspace_tabs.borrow().iter() { apply_unsaved_icon(tab.page(), unsaved.contains(id)); } - let dirty = !unsaved.is_empty(); + let dirty = dirty_tabs.iter().any(|tab| tab.kind.lost_with_the_window()); if self.reported_dirty.replace(Some(dirty)) == Some(dirty) { return; } @@ -237,6 +238,7 @@ mod tests { id, title: "Query 1".to_owned(), open_transaction: true, + unsaved_file: false, }] ); diff --git a/linux/crates/app/src/ui/workspace/close_tab_tests.rs b/linux/crates/app/src/ui/workspace/close_tab_tests.rs index 4aca3f6181..d56c39fab1 100644 --- a/linux/crates/app/src/ui/workspace/close_tab_tests.rs +++ b/linux/crates/app/src/ui/workspace/close_tab_tests.rs @@ -2,6 +2,7 @@ use gtk4 as gtk; use gtk4::glib; use gtk4::prelude::*; use gtk4::subclass::prelude::*; +use libadwaita::prelude::*; use tablepro_core::column::{ColumnDefault, ColumnType}; use tablepro_core::meta::{RowCountEstimate, RowIdentity, TableEstimate}; use tablepro_core::{ColumnInfo, QueryResult, Value}; @@ -173,3 +174,81 @@ fn a_tab_closed_with_the_focus_in_it_is_freed_when_the_next_takes_none() { "the focus was left nowhere" ); } + +/// An editor tab saved to `path`, whose text has changed since. +fn editor_on_file(test: &TestWindow, path: &std::path::Path) -> (Uuid, crate::ui::editor::SqlEditor) { + let workspace = test.workspace(); + workspace.with_model(|model| { + model.open_tab(super::tab_spec::TabSpec::Editor { + initial_query: Some("SELECT 2;".to_owned()), + draft: None, + file: Some(crate::workspace::EditorFileRecord { + uri: gtk::gio::File::for_path(path).uri().to_string(), + modified: true, + }), + }) + }); + let id = workspace.selected_tab().expect("the editor in front"); + let editor = workspace + .imp() + .tab_view + .selected_page() + .and_then(|page| page.child().downcast::().ok()) + .expect("an editor tab in front"); + (id, editor) +} + +fn tab_is_open(test: &TestWindow, id: Uuid) -> bool { + test.workspace() + .read(|model| model.workspace_tabs.borrow().contains_key(&id)) + .unwrap_or(false) +} + +#[gtk4::test] +fn closing_a_tab_with_unsaved_file_changes_saves_them_when_asked() { + let test = TestWindow::connected("CloseUnsavedFileSave"); + let path = test.dir().join("daily.sql"); + crate::test_support::write_text_file(&path, "SELECT 1;"); + let (id, _editor) = editor_on_file(&test, &path); + + WidgetExt::activate_action(&test.window, "win.close-current", None).expect("win.close-current"); + + 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, "save")); + wait_until(WAIT, || !tab_is_open(&test, id)).expect("the tab closes once saved"); + assert_eq!(crate::test_support::read_text_file(&path), "SELECT 2;"); +} + +#[gtk4::test] +fn discarding_unsaved_file_changes_closes_the_tab_and_leaves_the_file() { + let test = TestWindow::connected("CloseUnsavedFileDiscard"); + let path = test.dir().join("daily.sql"); + crate::test_support::write_text_file(&path, "SELECT 1;"); + let (id, _editor) = editor_on_file(&test, &path); + + WidgetExt::activate_action(&test.window, "win.close-current", None).expect("win.close-current"); + + wait_until(WAIT, || visible_alert(&test.window).is_some()).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!(visible_alert(&test.window).is_none(), "the close asked again"); + assert_eq!(crate::test_support::read_text_file(&path), "SELECT 1;"); +} + +/// The window keeps the text with the tab's draft and restores both. +#[gtk4::test] +fn closing_the_window_keeps_unsaved_file_changes_without_asking() { + let test = TestWindow::connected("CloseWindowUnsavedFile"); + let path = test.dir().join("daily.sql"); + crate::test_support::write_text_file(&path, "SELECT 1;"); + let (_id, editor) = editor_on_file(&test, &path); + assert!(editor.has_unsaved_file_changes()); + drop(editor); + + test.window.close(); + + wait_until(WAIT, || test.app.windows().is_empty()).expect("the window closes"); +} diff --git a/linux/crates/app/src/ui/workspace/editor_label.rs b/linux/crates/app/src/ui/workspace/editor_label.rs index 63f9740a06..9b18d05d49 100644 --- a/linux/crates/app/src/ui/workspace/editor_label.rs +++ b/linux/crates/app/src/ui/workspace/editor_label.rs @@ -1,3 +1,6 @@ +use gtk4::gio; +use gtk4::prelude::*; + /// What an editor tab is named after. #[derive(Debug, Clone, PartialEq, Eq)] pub(crate) enum EditorLabel { @@ -6,9 +9,18 @@ pub(crate) enum EditorLabel { /// The start of its script, at most the 200 characters the tab /// reads, and whether the script goes on past it. Query { prefix: String, truncated: bool }, + /// The `.sql` file it is saved to: its name, and where it is. + File { name: String, location: String }, } impl EditorLabel { + pub(crate) fn file(file: &gio::File) -> Self { + Self::File { + name: crate::ui::editor::file_name(file), + location: file.parse_name().to_string(), + } + } + /// A blank script is named by `ordinal`, so a label is never empty. pub(crate) fn of(ordinal: usize, prefix: &str, truncated: bool) -> Self { if !truncated && prefix.trim().is_empty() { diff --git a/linux/crates/app/src/ui/workspace/mod.rs b/linux/crates/app/src/ui/workspace/mod.rs index a4898f9047..e6c26cabab 100644 --- a/linux/crates/app/src/ui/workspace/mod.rs +++ b/linux/crates/app/src/ui/workspace/mod.rs @@ -198,6 +198,7 @@ struct WorkspaceTabParts { pub enum ClosedTabDescriptor { Editor { query: String, + file: Option, }, Table { schema: Option, @@ -385,6 +386,9 @@ pub(crate) enum WorkspaceInput { EditorTabRunStateChanged(Uuid, bool), EditorTabRunFinished(Uuid, crate::workspace::WorkOutcome), EditorTabContentChanged(Uuid), + /// An editor tab was saved, opened a file, or its text came to + /// differ from its file's or match it again. + EditorTabFileChanged(Uuid), /// The session's saved tabs and their editor text, read off the GTK /// thread. WorkspaceLoaded(workspace_tabs::LoadedWorkspace), @@ -955,6 +959,7 @@ impl WorkspaceModel { self.on_editor_run_finished(id); } WorkspaceInput::EditorTabContentChanged(id) => self.on_editor_tab_content_changed(id), + WorkspaceInput::EditorTabFileChanged(id) => self.on_editor_tab_file_changed(id), WorkspaceInput::WorkspaceLoaded(loaded) => self.on_workspace_loaded(loaded), WorkspaceInput::ResetWorkspaceLayout => self.on_reset_workspace_layout(), WorkspaceInput::WorkspaceLayoutReset(outcome) => self.on_workspace_layout_reset(outcome), diff --git a/linux/crates/app/src/ui/workspace/saves.rs b/linux/crates/app/src/ui/workspace/saves.rs index fd235f68bf..e1cb119244 100644 --- a/linux/crates/app/src/ui/workspace/saves.rs +++ b/linux/crates/app/src/ui/workspace/saves.rs @@ -30,7 +30,7 @@ impl WorkspaceModel { body: rejection.message(), }); } - SaveStart::NothingToSave | SaveStart::Rejected(_) => {} + SaveStart::Saved | SaveStart::NothingToSave | SaveStart::Rejected(_) => {} } } }); diff --git a/linux/crates/app/src/ui/workspace/tab_factory.rs b/linux/crates/app/src/ui/workspace/tab_factory.rs index d7b8208916..7f0c91d502 100644 --- a/linux/crates/app/src/ui/workspace/tab_factory.rs +++ b/linux/crates/app/src/ui/workspace/tab_factory.rs @@ -3,7 +3,7 @@ use std::collections::HashSet; use std::rc::Rc; use gtk4 as gtk; -use gtk4::glib; +use gtk4::{gio, glib}; use libadwaita as adw; use libadwaita::prelude::*; use tablepro_core::meta::TableRef; @@ -28,7 +28,7 @@ use crate::services::structure_tracker_registry::StructureTrackerRegistry; use crate::ui::browse_tab::{BrowseTab, BrowseTabInit}; use crate::ui::editor::{SqlEditor, SqlEditorInit}; use crate::ui::structure_tab::{StructureMode, StructureTab, StructureTabInit}; -use crate::workspace::DraftWriter; +use crate::workspace::{DraftWriter, EditorFileRecord}; /// What a factory builds its tabs from: the workspace's session, stores /// and trackers. @@ -68,7 +68,11 @@ impl TabFactory { pub(super) fn create(&self, spec: TabSpec) -> Option { let id = Uuid::new_v4(); let slot = match spec { - TabSpec::Editor { initial_query, draft } => self.editor(id, initial_query, draft), + TabSpec::Editor { + initial_query, + draft, + file, + } => self.editor(id, initial_query, draft, file), TabSpec::Table { table, page } => self.table(id, table, page)?, TabSpec::Structure { target } => self.structure(id, target)?, }; @@ -78,7 +82,7 @@ impl TabFactory { /// The overview's new-tab button needs its page back at once. pub(super) fn create_editor(&self) -> adw::TabPage { let id = Uuid::new_v4(); - let slot = self.editor(id, None, None); + let slot = self.editor(id, None, None, None); self.insert(id, slot) } @@ -96,7 +100,13 @@ impl TabFactory { page } - fn editor(&self, id: Uuid, initial_query: Option, draft: Option) -> WorkspaceTab { + fn editor( + &self, + id: Uuid, + initial_query: Option, + draft: Option, + file: Option, + ) -> WorkspaceTab { let parts = &self.parts; // Such a tab shows the editor's sample query, which names nothing. let opened_empty = initial_query.is_none(); @@ -110,11 +120,17 @@ impl TabFactory { close_timeout: parts.context.close_timeout(), }); connect_editor(&editor, id, &parts.input, &parts.toasts); + // After the signals are connected, so the workspace marks a tab + // restored with unsaved changes. + if let Some(file) = file { + editor.restore_file(gio::File::for_uri(&file.uri), file.modified); + } let ordinal = self.free_ordinal(); let page = self.append(&editor, id); - let label = match opened_empty { - true => EditorLabel::Untitled(ordinal), - false => EditorPreview::read(&editor.buffer()).label(ordinal), + let label = match (opened_empty, editor.file()) { + (_, Some(file)) => EditorLabel::file(&file), + (true, None) => EditorLabel::Untitled(ordinal), + (false, None) => EditorPreview::read(&editor.buffer()).label(ordinal), }; let draft = draft.unwrap_or_else(|| { // Text a new tab opens with is written even if the user @@ -311,6 +327,8 @@ pub(super) fn connect_editor( }); let content = input.clone(); editor.connect_content_changed(move |_| content.deliver(WorkspaceInput::EditorTabContentChanged(tab_id))); + let file = input.clone(); + editor.connect_file_changed(move |_| file.deliver(WorkspaceInput::EditorTabFileChanged(tab_id))); let transaction = input.clone(); editor.connect_transaction_changed(move |_, _| transaction.deliver(WorkspaceInput::EditorTransactionChanged)); let toast = input.clone(); diff --git a/linux/crates/app/src/ui/workspace/tab_spec.rs b/linux/crates/app/src/ui/workspace/tab_spec.rs index c9c247649a..4b4423f25c 100644 --- a/linux/crates/app/src/ui/workspace/tab_spec.rs +++ b/linux/crates/app/src/ui/workspace/tab_spec.rs @@ -2,6 +2,7 @@ use tablepro_core::meta::TableRef; use tablepro_storage::DraftId; use super::structure_target::StructureTarget; +use crate::workspace::EditorFileRecord; /// A tab to open. Every way a tab opens builds one of these: the sidebar, /// the overview's new-tab button, a restore and a reopen. @@ -12,6 +13,8 @@ pub(crate) enum TabSpec { /// The draft a restored tab keeps writing to. `None` gives a new /// tab a new draft, written at once. draft: Option, + /// The `.sql` file a restored or reopened tab is saved to. + file: Option, }, Table { table: TableRef, diff --git a/linux/crates/app/src/ui/workspace/tab_title.rs b/linux/crates/app/src/ui/workspace/tab_title.rs index 180f183a41..95d77f1f48 100644 --- a/linux/crates/app/src/ui/workspace/tab_title.rs +++ b/linux/crates/app/src/ui/workspace/tab_title.rs @@ -34,6 +34,10 @@ pub(crate) fn resolve_tab_title(identity: &TabIdentity, naming: TableNaming) -> label: numbered_query(*ordinal), tooltip: None, }, + TabIdentity::Editor(EditorLabel::File { name, location }) => TabTitle { + label: name.clone(), + tooltip: Some(location.clone()), + }, TabIdentity::Editor(EditorLabel::Query { prefix, truncated }) => { let label = derive_tab_label(prefix); let text = prefix.trim(); @@ -147,6 +151,20 @@ mod tests { assert!(tooltip.ends_with('…'), "{tooltip}"); } + #[test] + fn a_tab_saved_to_a_file_is_named_after_it() { + let title = resolve_tab_title( + &TabIdentity::Editor(EditorLabel::File { + name: "daily.sql".to_owned(), + location: "/home/ada/reports/daily.sql".to_owned(), + }), + TableNaming::Bare, + ); + + assert_eq!(title.label, "daily.sql"); + assert_eq!(title.tooltip.as_deref(), Some("/home/ada/reports/daily.sql")); + } + #[test] fn a_script_its_label_shows_whole_has_no_tooltip() { let title = resolve_tab_title( diff --git a/linux/crates/app/src/ui/workspace/tab_title_tests.rs b/linux/crates/app/src/ui/workspace/tab_title_tests.rs index a1b4d14f17..464bd61550 100644 --- a/linux/crates/app/src/ui/workspace/tab_title_tests.rs +++ b/linux/crates/app/src/ui/workspace/tab_title_tests.rs @@ -137,7 +137,13 @@ fn restored_workspace_selects_saved_active_tab() { &test.key(), WorkspaceRecord { active_tab: Some(1), - tabs: vec![TabRecord::Editor { draft: DraftId::new() }, orders], + tabs: vec![ + TabRecord::Editor { + draft: DraftId::new(), + file: None, + }, + orders, + ], }, ) .expect("the record to queue"); @@ -163,6 +169,49 @@ fn restored_workspace_selects_saved_active_tab() { assert_eq!(selected, Some((StructureMode::Edit, "orders".to_owned()))); } +#[gtk4::test] +fn a_restored_editor_tab_keeps_its_file_and_its_unsaved_changes() { + let test = TestWindow::picker("RestoreEditorFile"); + let path = test.dir().join("daily.sql"); + crate::test_support::write_text_file(&path, "SELECT 1;"); + let store = test.services.storage.workspaces().clone(); + store + .schedule( + &test.key(), + WorkspaceRecord { + active_tab: Some(0), + tabs: vec![TabRecord::Editor { + draft: DraftId::new(), + file: Some(crate::workspace::EditorFileRecord { + uri: gio::File::for_path(&path).uri().to_string(), + modified: true, + }), + }], + }, + ) + .expect("the record to queue"); + finish_on_main(WAIT, store.flush(&test.key())) + .expect("the flush to end") + .expect("the record written"); + + test.window + .send(SessionWindowInput::OpenSaved(Box::new(test.saved.clone()))); + test.wait_for_page("workspace"); + let workspace = test.workspace(); + wait_until(WAIT, || workspace.imp().tab_view.n_pages() == 1).expect("the editor tab restored"); + + let page = workspace.imp().tab_view.nth_page(0); + let editor = editors(&test.window).pop().expect("the editor"); + assert_eq!(editor.file().and_then(|file| file.path()), Some(path.clone())); + assert!( + editor.has_unsaved_file_changes(), + "the restore forgot the text was not saved" + ); + assert_eq!(page.title(), "daily.sql"); + assert_eq!(page.tooltip().as_deref(), Some(path.to_string_lossy().as_ref())); + wait_until(WAIT, || modified(&page)).expect("the unsaved icon"); +} + #[gtk4::test] fn window_title_follows_the_tab_in_front() { let test = TestWindow::connected("WindowTitle"); diff --git a/linux/crates/app/src/ui/workspace/window_tabs.rs b/linux/crates/app/src/ui/workspace/window_tabs.rs index deb2c83766..8282c7ab84 100644 --- a/linux/crates/app/src/ui/workspace/window_tabs.rs +++ b/linux/crates/app/src/ui/workspace/window_tabs.rs @@ -1,12 +1,15 @@ use std::rc::Rc; +use futures::future::LocalBoxFuture; use gtk4::glib; use libadwaita as adw; use libadwaita::prelude::*; +use uuid::Uuid; use super::WorkspaceTab; use super::tab_index::TabIndex; use super::workspace_persistence::WorkspaceTabs; +use crate::ui::editor::SqlEditor; use crate::workspace::{TabDirectory, TabEntry}; /// The window's tabs as the close prompts read them: in tab bar order, @@ -40,16 +43,26 @@ impl TabDirectory for WindowTabs { .filter_map(|index| pages.item(index).and_downcast::()) .filter_map(|page| { let id = self.index.id_of(&page)?; - let tab = tabs.get(&id)?; + let editor = match tabs.get(&id)? { + WorkspaceTab::Editor(slot) => Some(&slot.editor), + WorkspaceTab::Structure(_) | WorkspaceTab::Table(_) => None, + }; Some(TabEntry { id, title: page.title().to_string(), - open_transaction: matches!( - tab, - WorkspaceTab::Editor(slot) if slot.editor.has_open_transaction() - ), + open_transaction: editor.is_some_and(SqlEditor::has_open_transaction), + unsaved_file: editor.is_some_and(SqlEditor::has_unsaved_file_changes), }) }) .collect() } + + fn save_file(&self, tab: Uuid) -> Option>> { + let tabs = self.tabs.borrow(); + let WorkspaceTab::Editor(slot) = tabs.get(&tab)? else { + return None; + }; + slot.editor.file()?; + Some(Box::pin(slot.editor.save_to_file())) + } } diff --git a/linux/crates/app/src/ui/workspace/workspace_persistence.rs b/linux/crates/app/src/ui/workspace/workspace_persistence.rs index 63c6d2cadc..56ed03e6bc 100644 --- a/linux/crates/app/src/ui/workspace/workspace_persistence.rs +++ b/linux/crates/app/src/ui/workspace/workspace_persistence.rs @@ -138,7 +138,10 @@ pub(super) fn workspace_record( fn tab_record(tab: &WorkspaceTab) -> Option { match tab { - WorkspaceTab::Editor(slot) => Some(TabRecord::Editor { draft: slot.draft }), + WorkspaceTab::Editor(slot) => Some(TabRecord::Editor { + draft: slot.draft, + file: super::workspace_tabs::editor_file_record(&slot.editor), + }), WorkspaceTab::Structure(slot) => match slot.mode { // A draft for a table that does not exist yet has nothing to // reopen. @@ -269,7 +272,7 @@ mod tests { .tabs .iter() .filter_map(|tab| match tab { - TabRecord::Editor { draft } => Some(*draft), + TabRecord::Editor { draft, .. } => Some(*draft), _ => None, }) .collect() @@ -358,7 +361,10 @@ mod tests { schema: Some("public".to_owned()), table: "users".to_owned(), }, - TabRecord::Editor { draft: DraftId::new() }, + TabRecord::Editor { + draft: DraftId::new(), + file: None, + }, ], }; let mut opened = Vec::new(); diff --git a/linux/crates/app/src/ui/workspace/workspace_tabs.rs b/linux/crates/app/src/ui/workspace/workspace_tabs.rs index 70fc3c1883..5a401d1090 100644 --- a/linux/crates/app/src/ui/workspace/workspace_tabs.rs +++ b/linux/crates/app/src/ui/workspace/workspace_tabs.rs @@ -15,9 +15,10 @@ use uuid::Uuid; use crate::ui::browse_tab::{BrowseTabInput, PagingInput, SaveInput}; use crate::ui::structure_tab::StructureMode; use crate::workspace::{ - RestoredDrafts, TabRecord, WorkspaceFileName, WorkspaceLoad, WorkspaceRecord, WorkspaceSaveError, + EditorFileRecord, RestoredDrafts, TabRecord, WorkspaceFileName, WorkspaceLoad, WorkspaceRecord, WorkspaceSaveError, }; +use super::editor_label::EditorLabel; use super::structure_target::StructureTarget; use super::tab_factory::EditorPreview; use super::tab_index::TabIndex; @@ -120,9 +121,10 @@ impl WorkspaceModel { }, // Its text is on disk already, or there is none to keep, so // nothing is written until the user types. - TabRecord::Editor { draft } => TabSpec::Editor { + TabRecord::Editor { draft, file } => TabSpec::Editor { initial_query: Some(drafts.remove(&draft).unwrap_or_default()), draft: Some(draft), + file, }, }; self.open_tab(spec) @@ -175,6 +177,7 @@ impl WorkspaceModel { self.open_tab(TabSpec::Editor { initial_query, draft: None, + file: None, }); } @@ -457,12 +460,31 @@ impl WorkspaceModel { return; }; let buffer = slot.editor.buffer(); - slot.label = EditorPreview::read(&buffer).label(slot.ordinal); + if slot.editor.file().is_none() { + slot.label = EditorPreview::read(&buffer).label(slot.ordinal); + } self.drafts.schedule(&self.draft_scope(), slot.draft, &buffer); } self.retitle_tab(id); } + /// A tab saved to a file is named after it, shows whether its text + /// is saved, and is restored with it. + pub(super) fn on_editor_tab_file_changed(&self, id: Uuid) { + { + let mut tabs = self.workspace_tabs.borrow_mut(); + let Some(WorkspaceTab::Editor(slot)) = tabs.get_mut(&id) else { + return; + }; + if let Some(file) = slot.editor.file() { + slot.label = EditorLabel::file(&file); + } + } + self.retitle_tab(id); + self.refresh_unsaved(); + self.persist_workspace_state(); + } + pub(super) fn on_editor_tab_run_state_changed(&self, id: Uuid, running: bool) { if let Some(WorkspaceTab::Editor(slot)) = self.workspace_tabs.borrow().get(&id) { slot.page.set_loading(running); @@ -550,6 +572,7 @@ fn describe_closed_tab(slot: &WorkspaceTab) -> Option { match slot { WorkspaceTab::Editor(s) => Some(ClosedTabDescriptor::Editor { query: whole_text(&s.editor.buffer()), + file: editor_file_record(&s.editor), }), WorkspaceTab::Table(s) => Some(ClosedTabDescriptor::Table { schema: s.schema.clone(), @@ -589,9 +612,13 @@ impl WorkspaceModel { return; }; match descriptor { - ClosedTabDescriptor::Editor { query } => { + ClosedTabDescriptor::Editor { query, file } => { let initial = if query.is_empty() { None } else { Some(query) }; - self.open_editor_tab(initial); + self.open_tab(TabSpec::Editor { + initial_query: initial, + draft: None, + file, + }); } ClosedTabDescriptor::Table { schema, @@ -618,6 +645,14 @@ impl WorkspaceModel { } } +/// The file an editor tab is saved to, as its record keeps it. +pub(super) fn editor_file_record(editor: &crate::ui::editor::SqlEditor) -> Option { + editor.file().map(|file| EditorFileRecord { + uri: file.uri().to_string(), + modified: editor.has_unsaved_file_changes(), + }) +} + /// The whole script, for the few places that need all of it at once. fn whole_text(buffer: >k::TextBuffer) -> String { buffer.text(&buffer.start_iter(), &buffer.end_iter(), false).to_string() @@ -632,7 +667,7 @@ pub(crate) struct LoadedWorkspace { fn saved_drafts(record: &WorkspaceRecord) -> impl Iterator + '_ { record.tabs.iter().filter_map(|tab| match tab { - TabRecord::Editor { draft } => Some(*draft), + TabRecord::Editor { draft, .. } => Some(*draft), _ => None, }) } diff --git a/linux/crates/app/src/workspace/close_guard.rs b/linux/crates/app/src/workspace/close_guard.rs index ce2ec0e974..90b6009a1c 100644 --- a/linux/crates/app/src/workspace/close_guard.rs +++ b/linux/crates/app/src/workspace/close_guard.rs @@ -20,8 +20,9 @@ use super::{CloseDecision, CloseScope, CloseStep, DirtyTab, SaveRejection, SaveS pub(crate) struct CloseGuard; impl CloseGuard { - /// `acknowledged` holds the tabs whose open transactions the user - /// already agreed to lose, and gains the ones agreed to here. + /// `acknowledged` holds the tabs whose open transactions or file + /// changes the user already agreed to lose, and gains the ones + /// agreed to here. pub(crate) async fn run( parent: >k::Widget, handle: &WorkspaceHandle, @@ -61,6 +62,9 @@ impl CloseGuard { SaveChoice::Cancel => return CloseDecision::Cancelled, SaveChoice::Proceed { save, discard } => { handle.discard(&discard, false); + // A file's text has no tracker to clear: it goes + // with the tab, once the user agreed to that. + acknowledged.extend(discard.iter().copied()); let (refused, declined) = start_saves(parent, handle, &save).await; if !refused.is_empty() { show_refused(parent, &tabs, &refused); @@ -90,7 +94,7 @@ async fn start_saves( match start { SaveStart::Rejected(SaveRejection::ReviewDeclined) => return (refused, true), SaveStart::Rejected(rejection) => refused.push((*tab, rejection)), - SaveStart::Started(_) | SaveStart::NothingToSave => {} + SaveStart::Started(_) | SaveStart::Saved | SaveStart::NothingToSave => {} } } } diff --git a/linux/crates/app/src/workspace/close_step.rs b/linux/crates/app/src/workspace/close_step.rs index dcc8fec06b..1d5bd2b4c4 100644 --- a/linux/crates/app/src/workspace/close_step.rs +++ b/linux/crates/app/src/workspace/close_step.rs @@ -40,7 +40,16 @@ pub fn next_close_step(scope: CloseScope, saves_in_flight: usize, dirty: &[Dirty if !transactions.is_empty() { return CloseStep::ConfirmOpenTransactions(transactions); } - let unsaved = of_kind(|kind| matches!(kind, DirtyKind::Unsaved)); + let tab_close = matches!(scope, CloseScope::Tab(_)); + let unsaved: Vec = in_scope + .iter() + .filter(|tab| match tab.kind { + DirtyKind::Unsaved => true, + DirtyKind::UnsavedFile => tab_close, + _ => false, + }) + .map(|tab| (*tab).clone()) + .collect(); if !unsaved.is_empty() { return CloseStep::AskSaveChanges(unsaved); } @@ -100,4 +109,19 @@ mod tests { CloseStep::Approve ); } + + #[test] + fn an_unsaved_file_is_asked_about_only_when_its_tab_closes() { + let file = tab(DirtyKind::UnsavedFile); + + assert_eq!( + next_close_step(CloseScope::Tab(file.id), 0, std::slice::from_ref(&file)), + CloseStep::AskSaveChanges(vec![file.clone()]) + ); + assert_eq!( + next_close_step(CloseScope::Window, 0, std::slice::from_ref(&file)), + CloseStep::Approve, + "the window keeps the text in its drafts, so it has nothing to ask" + ); + } } diff --git a/linux/crates/app/src/workspace/dirty_kind.rs b/linux/crates/app/src/workspace/dirty_kind.rs index b07213e865..de66ac9594 100644 --- a/linux/crates/app/src/workspace/dirty_kind.rs +++ b/linux/crates/app/src/workspace/dirty_kind.rs @@ -9,4 +9,16 @@ pub enum DirtyKind { /// An editor tab's transaction, which the server rolls back when its /// connection ends. OpenTransaction, + /// An editor tab's text differs from its file. Closing the window + /// keeps the text with the tab's other drafts, so only closing the + /// tab loses it. + UnsavedFile, +} + +impl DirtyKind { + /// Whether closing the whole window loses it too, which a logout + /// would. + pub fn lost_with_the_window(&self) -> bool { + !matches!(self, Self::UnsavedFile) + } } diff --git a/linux/crates/app/src/workspace/editor_file_record.rs b/linux/crates/app/src/workspace/editor_file_record.rs new file mode 100644 index 0000000000..eb4771f95f --- /dev/null +++ b/linux/crates/app/src/workspace/editor_file_record.rs @@ -0,0 +1,11 @@ +use serde::{Deserialize, Serialize}; + +/// The `.sql` file an editor tab is saved to, kept with the tab. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct EditorFileRecord { + /// A URI rather than a path, so a file the document portal handed + /// over reads back the same way as a local one. + pub uri: String, + /// The tab's text, which its draft keeps, differs from the file's. + pub modified: bool, +} diff --git a/linux/crates/app/src/workspace/mod.rs b/linux/crates/app/src/workspace/mod.rs index c10e2443d3..07134a3a5e 100644 --- a/linux/crates/app/src/workspace/mod.rs +++ b/linux/crates/app/src/workspace/mod.rs @@ -16,6 +16,7 @@ pub(crate) mod dialog_response; mod dirty_kind; mod dirty_tab; mod draft_writer; +mod editor_file_record; mod engine_profile; mod inhibit_reason; mod legacy_tab_file; @@ -80,6 +81,7 @@ pub(crate) use dialog_response::{answer, visible_alert}; pub use dirty_kind::DirtyKind; pub use dirty_tab::DirtyTab; pub use draft_writer::DraftWriter; +pub use editor_file_record::EditorFileRecord; pub use engine_profile::EngineProfile; pub use inhibit_reason::InhibitReason; pub(crate) use legacy_tab_file::remove_legacy_tab_file; diff --git a/linux/crates/app/src/workspace/save_rejection.rs b/linux/crates/app/src/workspace/save_rejection.rs index 6508bdc698..7b5dce3c6f 100644 --- a/linux/crates/app/src/workspace/save_rejection.rs +++ b/linux/crates/app/src/workspace/save_rejection.rs @@ -33,6 +33,8 @@ pub enum SaveRejection { /// A review of the same changes is open. InReview, NoConnection, + /// An editor tab's file could not be written; the sentence says why. + FileNotSaved(String), } impl SaveRejection { @@ -63,6 +65,7 @@ impl SaveRejection { Self::ReviewDeclined => crate::i18n::gettext("The changes were not applied."), Self::InReview => crate::i18n::gettext("The review of these changes is still open."), Self::NoConnection => crate::i18n::gettext("No active connection."), + Self::FileNotSaved(message) => message.clone(), } } diff --git a/linux/crates/app/src/workspace/save_start.rs b/linux/crates/app/src/workspace/save_start.rs index 6d55c01f40..bb16cecb62 100644 --- a/linux/crates/app/src/workspace/save_start.rs +++ b/linux/crates/app/src/workspace/save_start.rs @@ -4,6 +4,8 @@ use super::{SaveId, SaveRejection}; #[derive(Debug, Clone, PartialEq, Eq)] pub enum SaveStart { Started(SaveId), + /// Written at once, with nothing left running: an editor tab's file. + Saved, NothingToSave, Rejected(SaveRejection), } diff --git a/linux/crates/app/src/workspace/tab_directory.rs b/linux/crates/app/src/workspace/tab_directory.rs index 12a1ed1e86..b237a570d5 100644 --- a/linux/crates/app/src/workspace/tab_directory.rs +++ b/linux/crates/app/src/workspace/tab_directory.rs @@ -1,8 +1,18 @@ +use futures::future::LocalBoxFuture; +use uuid::Uuid; + use super::TabEntry; /// What the workspace needs from the window's tabs that no tracker -/// holds: their order, their names and their open transactions. +/// holds: their order, their names, their open transactions and the +/// files their text is saved to. pub trait TabDirectory { /// Every open tab, in the order the tab bar shows them. fn tabs(&self) -> Vec; + + /// Write `tab`'s text to its file, or `None` when it has none. The + /// error is the sentence to show. + fn save_file(&self, _tab: Uuid) -> Option>> { + None + } } diff --git a/linux/crates/app/src/workspace/tab_entry.rs b/linux/crates/app/src/workspace/tab_entry.rs index 65f664c3d1..8abf624431 100644 --- a/linux/crates/app/src/workspace/tab_entry.rs +++ b/linux/crates/app/src/workspace/tab_entry.rs @@ -8,4 +8,6 @@ pub struct TabEntry { pub title: String, /// The tab's editor connection holds a transaction the user opened. pub open_transaction: bool, + /// The tab's text differs from the `.sql` file it is saved to. + pub unsaved_file: bool, } diff --git a/linux/crates/app/src/workspace/tab_record.rs b/linux/crates/app/src/workspace/tab_record.rs index ed929056a8..d8d1e7fc56 100644 --- a/linux/crates/app/src/workspace/tab_record.rs +++ b/linux/crates/app/src/workspace/tab_record.rs @@ -3,7 +3,7 @@ use std::ops::RangeInclusive; use serde::{Deserialize, Serialize}; use tablepro_storage::DraftId; -use super::SortRecord; +use super::{EditorFileRecord, SortRecord}; /// What the page size preference allows. const PAGE_SIZES: RangeInclusive = 100..=100_000; @@ -27,6 +27,8 @@ pub enum TabRecord { /// costs the record one id. Editor { draft: DraftId, + #[serde(default, skip_serializing_if = "Option::is_none")] + file: Option, }, } @@ -59,7 +61,7 @@ impl TabRecord { }) } Self::Structure { schema, table } => (!table.is_empty()).then_some(Self::Structure { schema, table }), - Self::Editor { draft } => Some(Self::Editor { draft }), + Self::Editor { draft, file } => Some(Self::Editor { draft, file }), } } } diff --git a/linux/crates/app/src/workspace/test_workspace.rs b/linux/crates/app/src/workspace/test_workspace.rs index b426545b82..60fe7f5e9d 100644 --- a/linux/crates/app/src/workspace/test_workspace.rs +++ b/linux/crates/app/src/workspace/test_workspace.rs @@ -165,6 +165,7 @@ impl Workspace { id: tab, title: "t".to_owned(), open_transaction: false, + unsaved_file: false, }); let settled = Rc::new(RefCell::new(Vec::new())); let events = Rc::new(RefCell::new(Vec::new())); @@ -212,6 +213,7 @@ impl Workspace { id, title: "Query 1".to_owned(), open_transaction: true, + unsaved_file: false, }); id } @@ -225,6 +227,7 @@ impl Workspace { id, title: title.to_owned(), open_transaction: false, + unsaved_file: false, }); tracker } diff --git a/linux/crates/app/src/workspace/workspace_handle.rs b/linux/crates/app/src/workspace/workspace_handle.rs index 3b885ee383..86782a2c11 100644 --- a/linux/crates/app/src/workspace/workspace_handle.rs +++ b/linux/crates/app/src/workspace/workspace_handle.rs @@ -202,23 +202,26 @@ impl WorkspaceHandle { }); } - /// Every tab closing would lose something in, in tab order. + /// Every tab closing would lose something in, in tab order. An + /// editor tab can have both an open transaction and an unsaved file, + /// and is listed once for each. pub fn dirty_tabs(&self) -> Vec { self.0 .deps .directory .tabs() .into_iter() - .filter_map(|entry| { - let kind = match self.pending_state(entry.id) { - PendingState::OutcomeUnknown(cause) => DirtyKind::OutcomeUnknown(cause), - PendingState::Unsaved | PendingState::Saving => DirtyKind::Unsaved, - PendingState::Clean if entry.open_transaction => DirtyKind::OpenTransaction, - PendingState::Clean => return None, + .flat_map(|entry| { + let pending = match self.pending_state(entry.id) { + PendingState::OutcomeUnknown(cause) => Some(DirtyKind::OutcomeUnknown(cause)), + PendingState::Unsaved | PendingState::Saving => Some(DirtyKind::Unsaved), + PendingState::Clean if entry.open_transaction => Some(DirtyKind::OpenTransaction), + PendingState::Clean => None, }; - Some(DirtyTab { + let file = entry.unsaved_file.then_some(DirtyKind::UnsavedFile); + [pending, file].into_iter().flatten().map(move |kind| DirtyTab { id: entry.id, - title: entry.title, + title: entry.title.clone(), kind, }) }) @@ -240,7 +243,10 @@ impl WorkspaceHandle { let dirty: Vec = self .dirty_tabs() .into_iter() - .filter(|tab| !(tab.kind == DirtyKind::OpenTransaction && acknowledged.contains(&tab.id))) + .filter(|tab| { + let agreed_to_lose = matches!(tab.kind, DirtyKind::OpenTransaction | DirtyKind::UnsavedFile); + !(agreed_to_lose && acknowledged.contains(&tab.id)) + }) .collect(); next_close_step(scope, self.0.ledger.in_flight_in(scope), &dirty) } @@ -275,7 +281,13 @@ impl WorkspaceHandle { Err(start) => start, }; } - SaveStart::NothingToSave + match self.0.deps.directory.save_file(tab) { + Some(saving) => match saving.await { + Ok(()) => SaveStart::Saved, + Err(message) => SaveStart::Rejected(SaveRejection::FileNotSaved(message)), + }, + None => SaveStart::NothingToSave, + } } /// The session to save through, refused before anything is taken diff --git a/linux/crates/app/src/workspace/workspace_record.rs b/linux/crates/app/src/workspace/workspace_record.rs index 95820969ab..d4aef4d6dd 100644 --- a/linux/crates/app/src/workspace/workspace_record.rs +++ b/linux/crates/app/src/workspace/workspace_record.rs @@ -83,7 +83,10 @@ mod tests { schema: None, table: "users".to_owned(), }, - TabRecord::Editor { draft: DraftId::new() }, + TabRecord::Editor { + draft: DraftId::new(), + file: None, + }, ], }; @@ -102,7 +105,10 @@ mod tests { active_tab: Some(35), tabs: (0..40) .map(|index| match index % 2 { - 0 => TabRecord::Editor { draft: DraftId::new() }, + 0 => TabRecord::Editor { + draft: DraftId::new(), + file: None, + }, _ => table(&format!("t{index}")), }) .collect(), diff --git a/linux/crates/app/src/workspace/workspace_record_builder.rs b/linux/crates/app/src/workspace/workspace_record_builder.rs index ca873c3761..3617e7bb14 100644 --- a/linux/crates/app/src/workspace/workspace_record_builder.rs +++ b/linux/crates/app/src/workspace/workspace_record_builder.rs @@ -62,7 +62,13 @@ mod tests { #[test] fn selected_new_draft_gives_no_active_tab() { - let pages = [page(Some(TabRecord::Editor { draft: DraftId::new() })), page(None)]; + let pages = [ + page(Some(TabRecord::Editor { + draft: DraftId::new(), + file: None, + })), + page(None), + ]; let record = workspace_record(&pages, Some(pages[1].id)); diff --git a/linux/crates/app/src/workspace/workspace_store.rs b/linux/crates/app/src/workspace/workspace_store.rs index b78032ab1f..91c86534e2 100644 --- a/linux/crates/app/src/workspace/workspace_store.rs +++ b/linux/crates/app/src/workspace/workspace_store.rs @@ -532,7 +532,10 @@ mod tests { let key = key(); let editor = WorkspaceRecord { active_tab: Some(0), - tabs: vec![TabRecord::Editor { draft: DraftId::new() }], + tabs: vec![TabRecord::Editor { + draft: DraftId::new(), + file: None, + }], }; fixture.store.schedule(&key, editor.clone()).expect("scheduled"); @@ -676,7 +679,12 @@ mod tests { let key = key(); let many = WorkspaceRecord { active_tab: Some(39), - tabs: (0..40).map(|_| TabRecord::Editor { draft: DraftId::new() }).collect(), + tabs: (0..40) + .map(|_| TabRecord::Editor { + draft: DraftId::new(), + file: None, + }) + .collect(), }; fixture.store.schedule(&key, many.clone()).expect("scheduled"); diff --git a/linux/data/resources/shortcuts-dialog.ui b/linux/data/resources/shortcuts-dialog.ui index 6960765c73..c2753e67a7 100644 --- a/linux/data/resources/shortcuts-dialog.ui +++ b/linux/data/resources/shortcuts-dialog.ui @@ -305,6 +305,24 @@ <Primary>slash + + + Open a SQL File + <Primary>o + + + + + Save to the SQL File + <Primary>s + + + + + Save to Another SQL File + <Primary><Shift>s + + Find @@ -329,12 +347,6 @@ <Primary><Shift>g - - - Save the Query Under a Name - win.save-query - - diff --git a/linux/data/resources/ui/sql-editor.ui b/linux/data/resources/ui/sql-editor.ui index efcf70feb4..b9bdca096c 100644 --- a/linux/data/resources/ui/sql-editor.ui +++ b/linux/data/resources/ui/sql-editor.ui @@ -8,7 +8,20 @@ Open SQL File… editor.open-file + <Primary>o + + Save + editor.save + <Primary>s + + + Save As… + editor.save-as + <Primary><Shift>s + + +
Save Query… win.save-query diff --git a/linux/docs/state-management.md b/linux/docs/state-management.md index b5a29ab721..024df2c7cf 100644 --- a/linux/docs/state-management.md +++ b/linux/docs/state-management.md @@ -116,6 +116,12 @@ Events cross to the GTK thread through a channel. `BatchCollector` turns one bat A finished run goes to `HistoryService::record` with the text that was run, `DELIMITER` and `GO` lines included, judged by the dialect of the session that ran it, not the window's current one ([storage.md](storage.md#what-goes-into-the-history)). +### The tab's file + +Open SQL File (Ctrl+O) and Save As (Ctrl+Shift+S) bind the tab to a `.sql` file, through the file chooser portal; Save (Ctrl+S) writes to it, or asks for one. The tab is named after the file, and the buffer's own modified flag says whether its text differs from the file's, so Undo back to the saved text clears it. The tab's record keeps the file's URI and that flag (`EditorFileRecord`), and its draft keeps the text, so a restore brings back both. + +Closing the tab asks about unsaved file changes through the same Save Changes prompt as row and structure edits (`DirtyKind::UnsavedFile`), and its Save writes the file. Closing the window does not ask: the draft keeps the text for the next session. + ### Open transactions Every `RunSummary` carries the session's `TransactionState`, and the last one a run reported decides whether closing the tab would lose work (`transaction_dirty`): diff --git a/linux/po/POTFILES.in b/linux/po/POTFILES.in index 984ffd1c54..7e1f7566fc 100644 --- a/linux/po/POTFILES.in +++ b/linux/po/POTFILES.in @@ -199,6 +199,7 @@ crates/app/src/ui/editor/completion/sql_proposal.rs crates/app/src/ui/editor/completion/sql_proposal_imp.rs crates/app/src/ui/editor/completion/tests.rs crates/app/src/ui/editor/editor_actions.rs +crates/app/src/ui/editor/editor_file.rs crates/app/src/ui/editor/editor_session_slot.rs crates/app/src/ui/editor/find_bar.rs crates/app/src/ui/editor/find_state.rs @@ -485,6 +486,7 @@ crates/app/src/workspace/dialog_response.rs crates/app/src/workspace/dirty_kind.rs crates/app/src/workspace/dirty_tab.rs crates/app/src/workspace/draft_writer.rs +crates/app/src/workspace/editor_file_record.rs crates/app/src/workspace/engine_profile.rs crates/app/src/workspace/inhibit_reason.rs crates/app/src/workspace/legacy_tab_file.rs From 37099919e4f4e2e26050d8ce0bb4adba77b99c78 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Sat, 17 Oct 2026 18:25:11 +0700 Subject: [PATCH 03/14] fix(linux): say why the query history could not be opened --- .../crates/app/src/application/app_actions.rs | 2 +- .../app/src/application/app_services.rs | 2 +- .../app/src/services/history_availability.rs | 74 +++++++++++ .../app/src/services/history_service.rs | 6 +- .../src/ui/preferences_dialog/general_page.rs | 20 ++- .../ui/preferences_dialog/history_group.rs | 118 ++++++++++++++---- .../app/src/ui/preferences_dialog/mod.rs | 56 ++++++--- .../app/src/ui/workspace/status_pages.rs | 104 ++++++++++++++- .../app/src/ui/workspace/workspace_actions.rs | 12 +- 9 files changed, 330 insertions(+), 64 deletions(-) diff --git a/linux/crates/app/src/application/app_actions.rs b/linux/crates/app/src/application/app_actions.rs index d6962d028a..638a6686bf 100644 --- a/linux/crates/app/src/application/app_actions.rs +++ b/linux/crates/app/src/application/app_actions.rs @@ -61,7 +61,7 @@ fn present_preferences(app: &TableProApplication) { PreferencesDialog::new( &services.settings, &services.storage, - services.history().store(), + services.history(), &services.tasks, services.file_revealer.clone(), ) diff --git a/linux/crates/app/src/application/app_services.rs b/linux/crates/app/src/application/app_services.rs index 189d694301..7d55615207 100644 --- a/linux/crates/app/src/application/app_services.rs +++ b/linux/crates/app/src/application/app_services.rs @@ -157,7 +157,7 @@ impl AppServices { /// The database migrates and prunes on open, which is too slow to /// hold up the first window. The menu items that need it stay - /// disabled until the service reports Ready. + /// disabled until the service says how the open went. pub(crate) fn history(&self) -> HistoryService { self.history .get_or_init(|| { diff --git a/linux/crates/app/src/services/history_availability.rs b/linux/crates/app/src/services/history_availability.rs index af7bf47940..3d9b83254e 100644 --- a/linux/crates/app/src/services/history_availability.rs +++ b/linux/crates/app/src/services/history_availability.rs @@ -1,3 +1,7 @@ +use std::io::ErrorKind; + +use tablepro_storage::StorageError; + /// Whether the query history is usable yet. /// /// The database opens in the background, so the menu item that needs it @@ -7,14 +11,24 @@ pub enum HistoryAvailability { Starting, Ready, + /// The database could not be opened, and why, as the user is told. Failed(String), } impl HistoryAvailability { + pub fn failed(error: &StorageError) -> Self { + Self::Failed(open_failure_text(error)) + } + pub fn is_ready(&self) -> bool { matches!(self, Self::Ready) } + /// Done opening, one way or the other. + pub fn is_settled(&self) -> bool { + !matches!(self, Self::Starting) + } + pub fn failure(&self) -> Option<&str> { match self { Self::Failed(detail) => Some(detail), @@ -22,3 +36,63 @@ impl HistoryAvailability { } } } + +/// Query history and saved queries share the one database, so this +/// names both. +fn open_failure_text(error: &StorageError) -> String { + match error { + StorageError::HistoryNewerThanApp { .. } => crate::i18n::gettext( + "The query history was written by a newer version of TablePro. Update TablePro to read it.", + ), + StorageError::HistoryMigration(_) => { + crate::i18n::gettext("The query history could not be updated for this version of TablePro.") + } + StorageError::Io { path, source } | StorageError::Permissions { path, source } => { + let path = path.display().to_string(); + match source.kind() { + ErrorKind::PermissionDenied | ErrorKind::ReadOnlyFilesystem => { + crate::i18n::gettext_f("TablePro is not allowed to write {path}.", &[("path", &path)]) + } + ErrorKind::StorageFull | ErrorKind::QuotaExceeded => { + crate::i18n::gettext_f("The disk that holds {path} is full.", &[("path", &path)]) + } + _ => crate::i18n::gettext_f("{path} could not be created.", &[("path", &path)]), + } + } + _ => crate::i18n::gettext("The query history file could not be read. It may be damaged."), + } +} + +#[cfg(test)] +mod tests { + use std::path::PathBuf; + + use super::*; + + fn io(kind: ErrorKind) -> StorageError { + StorageError::Io { + path: PathBuf::from("/home/ada/.local/state/tablepro/history.db"), + source: std::io::Error::from(kind), + } + } + + #[test] + fn the_reason_says_what_to_fix_without_the_raw_error() { + assert_eq!( + HistoryAvailability::failed(&io(ErrorKind::PermissionDenied)).failure(), + Some("TablePro is not allowed to write /home/ada/.local/state/tablepro/history.db.") + ); + assert_eq!( + HistoryAvailability::failed(&io(ErrorKind::StorageFull)).failure(), + Some("The disk that holds /home/ada/.local/state/tablepro/history.db is full.") + ); + assert_eq!( + HistoryAvailability::failed(&StorageError::HistoryNewerThanApp { version: 9 }).failure(), + Some("The query history was written by a newer version of TablePro. Update TablePro to read it.") + ); + assert_eq!( + HistoryAvailability::failed(&StorageError::NotInitialised).failure(), + Some("The query history file could not be read. It may be damaged.") + ); + } +} diff --git a/linux/crates/app/src/services/history_service.rs b/linux/crates/app/src/services/history_service.rs index 087d02345e..191d356e40 100644 --- a/linux/crates/app/src/services/history_service.rs +++ b/linux/crates/app/src/services/history_service.rs @@ -47,7 +47,7 @@ impl HistoryService { Ok(history) => history, Err(error) => { tracing::warn!(%error, "query history unavailable"); - sender.send_replace(HistoryAvailability::Failed(error.to_string())); + sender.send_replace(HistoryAvailability::failed(&error)); return; } }; @@ -129,9 +129,7 @@ impl HistoryService { async move { // A sender gone means the worker ended, which it does only // after it has said how the open went. - let _ = availability - .wait_for(|state| !matches!(state, HistoryAvailability::Starting)) - .await; + let _ = availability.wait_for(HistoryAvailability::is_settled).await; store.lock().ok().and_then(|guard| guard.clone()) } } diff --git a/linux/crates/app/src/ui/preferences_dialog/general_page.rs b/linux/crates/app/src/ui/preferences_dialog/general_page.rs index 28afaad6a4..fdbc5a4b9f 100644 --- a/linux/crates/app/src/ui/preferences_dialog/general_page.rs +++ b/linux/crates/app/src/ui/preferences_dialog/general_page.rs @@ -69,7 +69,7 @@ impl GeneralPage { &self, settings: &Rc, storage: &crate::storage::SharedStorage, - history: Option, + history: crate::services::history_service::HistoryService, tasks: &Tasks, revealer: Rc, ) { @@ -96,13 +96,18 @@ mod tests { use crate::test_support::{MemorySettings, test_runtime}; use crate::ui::file_reveal::RecordingRevealer; - fn bound_page(settings: &MemorySettings, storage: &crate::storage::SharedStorage, tasks: &Tasks) -> GeneralPage { + fn bound_page( + settings: &MemorySettings, + storage: &crate::storage::SharedStorage, + root: &tempfile::TempDir, + tasks: &Tasks, + ) -> GeneralPage { crate::register_test_resources(); let page = GeneralPage::default(); page.bind( settings.get(), storage, - None, + super::super::test_history(root, tasks), tasks, Rc::new(RecordingRevealer::succeeding()), ); @@ -122,7 +127,8 @@ mod tests { let settings = MemorySettings::new(); let runtime = test_runtime(); let storage = test_storage(&runtime.tasks()); - let page = bound_page(&settings, &storage, &runtime.tasks()); + let root = tempfile::tempdir().expect("tempdir"); + let page = bound_page(&settings, &storage, &root, &runtime.tasks()); let groups = groups(&page); let titles: Vec = groups.iter().map(|group| group.title().to_string()).collect(); @@ -138,7 +144,8 @@ mod tests { let settings = MemorySettings::new(); let runtime = test_runtime(); let storage = test_storage(&runtime.tasks()); - let page = bound_page(&settings, &storage, &runtime.tasks()); + let root = tempfile::tempdir().expect("tempdir"); + let page = bound_page(&settings, &storage, &root, &runtime.tasks()); let row = page.imp().restore_session_row.get(); assert!(row.is_active(), "the row does not show the default"); @@ -154,7 +161,8 @@ mod tests { let settings = MemorySettings::new(); let runtime = test_runtime(); let storage = test_storage(&runtime.tasks()); - let page = bound_page(&settings, &storage, &runtime.tasks()); + let root = tempfile::tempdir().expect("tempdir"); + let page = bound_page(&settings, &storage, &root, &runtime.tasks()); let imp = page.imp(); imp.data_browser_group.imp().confirm_row.set_active(false); diff --git a/linux/crates/app/src/ui/preferences_dialog/history_group.rs b/linux/crates/app/src/ui/preferences_dialog/history_group.rs index cebecbe578..7c8bac4e34 100644 --- a/linux/crates/app/src/ui/preferences_dialog/history_group.rs +++ b/linux/crates/app/src/ui/preferences_dialog/history_group.rs @@ -11,7 +11,10 @@ use libadwaita::prelude::*; use tablepro_session::runtime::Tasks; use tablepro_storage::AppSettings; use tablepro_storage::settings::keys; +use tokio::sync::watch; +use crate::services::history_availability::HistoryAvailability; +use crate::services::history_service::HistoryService; use crate::ui::file_reveal::{FileRevealer, reveal_failure_toast}; mod imp { @@ -22,6 +25,9 @@ mod imp { #[derive(Default, gtk4::CompositeTemplate)] #[template(resource = "/app/tablepro/TablePro/ui/preferences-history-group.ui")] pub struct HistoryGroup { + /// Follows whether the query database opened, for as long as + /// the group is shown. + pub availability: std::cell::RefCell>>, #[template_child] pub save_history_row: TemplateChild, #[template_child] @@ -52,6 +58,12 @@ mod imp { } impl ObjectImpl for HistoryGroup { + fn dispose(&self) { + if let Some(watch) = self.availability.take() { + watch.abort(); + } + } + fn signals() -> &'static [Signal] { static SIGNALS: OnceLock> = OnceLock::new(); SIGNALS.get_or_init(|| { @@ -87,7 +99,7 @@ impl HistoryGroup { &self, settings: &AppSettings, storage: &crate::storage::SharedStorage, - history: Option, + history: HistoryService, tasks: &Tasks, revealer: Rc, ) { @@ -110,16 +122,15 @@ impl HistoryGroup { // in the tooltip. imp.storage_row.set_tooltip_text(Some(&path)); - // The database opens on a background task, so the button stays - // insensitive until it is there to clear. - imp.clear_button.set_sensitive(history.is_some()); let group = self.downgrade(); let tasks = tasks.clone(); + let service = history.clone(); imp.clear_button.connect_clicked(move |_| { - if let Some(group) = group.upgrade() { - group.confirm_clear(history.clone(), &tasks); + if let (Some(group), Some(store)) = (group.upgrade(), service.store()) { + group.confirm_clear(store, &tasks); } }); + self.follow_availability(history.availability()); let group = self.downgrade(); imp.storage_button.connect_clicked(move |_| { @@ -139,7 +150,40 @@ impl HistoryGroup { ) } - fn confirm_clear(&self, history: Option, tasks: &Tasks) { + /// The database opens on a background task: Clear waits for it, and + /// a database that could not open says why where the group starts. + fn follow_availability(&self, mut availability: watch::Receiver) { + let group = self.downgrade(); + let following = glib::spawn_future_local(async move { + loop { + let state = availability.borrow_and_update().clone(); + let Some(group) = group.upgrade() else { + return; + }; + group.show_availability(&state); + drop(group); + if availability.changed().await.is_err() { + return; + } + } + }); + if let Some(previous) = self.imp().availability.replace(Some(following)) { + previous.abort(); + } + } + + fn show_availability(&self, state: &HistoryAvailability) { + self.imp().clear_button.set_sensitive(state.is_ready()); + let reason = state.failure().map(|reason| { + crate::i18n::gettext_f( + "Query history and saved queries cannot be used. {reason}", + &[("reason", reason)], + ) + }); + self.set_description(reason.as_deref()); + } + + fn confirm_clear(&self, history: tablepro_storage::QueryHistory, tasks: &Tasks) { let alert = adw::AlertDialog::new( Some(&crate::i18n::gettext("Clear All Query History?")), Some(&crate::i18n::gettext( @@ -159,9 +203,7 @@ impl HistoryGroup { if response != "clear" { return; } - let Some(history) = history.clone() else { - return; - }; + let history = history.clone(); let clearing = tasks.spawn_task(async move { history.clear_all().await }); let group = group.clone(); // A destructive action that reports nothing leaves the @@ -212,12 +254,19 @@ mod tests { fn bound_group( settings: &MemorySettings, storage: &crate::storage::SharedStorage, + root: &tempfile::TempDir, tasks: &Tasks, revealer: Rc, ) -> HistoryGroup { crate::register_test_resources(); let group = HistoryGroup::default(); - group.bind(settings.get(), storage, None, tasks, revealer); + group.bind( + settings.get(), + storage, + super::super::test_history(root, tasks), + tasks, + revealer, + ); group } @@ -226,9 +275,11 @@ mod tests { let settings = MemorySettings::new(); let runtime = test_runtime(); let storage = test_storage(&runtime.tasks()); + let root = tempfile::tempdir().expect("tempdir"); let group = bound_group( &settings, &storage, + &root, &runtime.tasks(), Rc::new(RecordingRevealer::succeeding()), ); @@ -247,9 +298,11 @@ mod tests { let settings = MemorySettings::new(); let runtime = test_runtime(); let storage = test_storage(&runtime.tasks()); + let root = tempfile::tempdir().expect("tempdir"); let group = bound_group( &settings, &storage, + &root, &runtime.tasks(), Rc::new(RecordingRevealer::succeeding()), ); @@ -264,7 +317,8 @@ mod tests { let runtime = test_runtime(); let storage = test_storage(&runtime.tasks()); let revealer = Rc::new(RecordingRevealer::succeeding()); - let group = bound_group(&settings, &storage, &runtime.tasks(), revealer.clone()); + let root = tempfile::tempdir().expect("tempdir"); + let group = bound_group(&settings, &storage, &root, &runtime.tasks(), revealer.clone()); group.imp().storage_button.emit_clicked(); @@ -276,9 +330,11 @@ mod tests { let settings = MemorySettings::new(); let runtime = test_runtime(); let storage = test_storage(&runtime.tasks()); + let root = tempfile::tempdir().expect("tempdir"); let group = bound_group( &settings, &storage, + &root, &runtime.tasks(), Rc::new(RecordingRevealer::failing()), ); @@ -301,9 +357,11 @@ mod tests { let settings = MemorySettings::new(); let runtime = test_runtime(); let storage = test_storage(&runtime.tasks()); + let root = tempfile::tempdir().expect("tempdir"); let group = bound_group( &settings, &storage, + &root, &runtime.tasks(), Rc::new(RecordingRevealer::succeeding()), ); @@ -316,21 +374,31 @@ mod tests { } #[gtk4::test] - fn clear_history_is_insensitive_until_the_database_opens() { - let settings = MemorySettings::new(); - let runtime = test_runtime(); - let storage = test_storage(&runtime.tasks()); - - let starting = bound_group( - &settings, - &storage, - &runtime.tasks(), - Rc::new(RecordingRevealer::succeeding()), - ); - + fn clear_follows_the_database_and_a_failure_says_why() { + crate::register_test_resources(); + let group = HistoryGroup::default(); + let (state, availability) = watch::channel(HistoryAvailability::Starting); + group.follow_availability(availability); + let clear = group.imp().clear_button.get(); + drain_main_context(); assert!( - !starting.imp().clear_button.is_sensitive(), + !clear.is_sensitive(), "Clear offered to clear a database that is not open" ); + assert!(group.description().unwrap_or_default().is_empty()); + + state.send_replace(HistoryAvailability::Ready); + drain_main_context(); + assert!(clear.is_sensitive(), "Clear stayed off after the database opened"); + + state.send_replace(HistoryAvailability::Failed( + "The disk that holds history.db is full.".to_owned(), + )); + drain_main_context(); + assert!(!clear.is_sensitive()); + assert_eq!( + group.description().as_deref(), + Some("Query history and saved queries cannot be used. The disk that holds history.db is full.") + ); } } diff --git a/linux/crates/app/src/ui/preferences_dialog/mod.rs b/linux/crates/app/src/ui/preferences_dialog/mod.rs index 5ef0c0c7b7..c30c1b531a 100644 --- a/linux/crates/app/src/ui/preferences_dialog/mod.rs +++ b/linux/crates/app/src/ui/preferences_dialog/mod.rs @@ -13,6 +13,7 @@ use libadwaita::prelude::*; use tablepro_session::runtime::Tasks; use tablepro_storage::AppSettings; +use crate::services::history_service::HistoryService; use crate::ui::file_reveal::FileRevealer; #[cfg(test)] @@ -33,7 +34,7 @@ impl PreferencesDialog { pub(crate) fn new( settings: &Rc, storage: &crate::storage::SharedStorage, - history: Option, + history: HistoryService, tasks: &Tasks, revealer: Rc, ) -> Self { @@ -52,6 +53,18 @@ impl PreferencesDialog { } } +/// A query history of its own under `root`, opening in the background +/// as the app's does. +#[cfg(test)] +fn test_history(root: &tempfile::TempDir, tasks: &Tasks) -> HistoryService { + let paths = tablepro_storage::StoragePaths::under(root.path(), "tablepro-test", "app.tablepro.TablePro.Devel"); + let preferences = crate::services::history_preferences::HistoryPreferences { + retention_days: tokio::sync::watch::channel(30).1, + save: tokio::sync::watch::channel(true).1, + }; + HistoryService::start(paths, preferences, tasks) +} + /// A storage under a temporary root, so the dialog's storage row and /// history actions stay away from the developer's own files. #[cfg(test)] @@ -67,8 +80,6 @@ fn test_storage(tasks: &Tasks) -> crate::storage::SharedStorage { #[cfg(test)] mod tests { - use tablepro_storage::StoragePaths; - use super::*; use crate::test_support::{MemorySettings, assert_labelled, descendants, test_runtime, unlabelled_icon_buttons}; use crate::ui::file_reveal::RecordingRevealer; @@ -79,7 +90,7 @@ mod tests { fn new_dialog( settings: &MemorySettings, storage: &crate::storage::SharedStorage, - history: Option, + history: HistoryService, tasks: &Tasks, ) -> PreferencesDialog { crate::register_test_resources(); @@ -105,7 +116,13 @@ mod tests { let settings = MemorySettings::new(); let runtime = test_runtime(); let storage = test_storage(&runtime.tasks()); - let dialog = new_dialog(&settings, &storage, None, &runtime.tasks()); + let root = tempfile::tempdir().expect("tempdir"); + let dialog = new_dialog( + &settings, + &storage, + test_history(&root, &runtime.tasks()), + &runtime.tasks(), + ); let [general, editor] = pages(&dialog); assert_eq!(general.title(), "General"); @@ -124,7 +141,13 @@ mod tests { let settings = MemorySettings::new(); let runtime = test_runtime(); let storage = test_storage(&runtime.tasks()); - let dialog = new_dialog(&settings, &storage, None, &runtime.tasks()); + let root = tempfile::tempdir().expect("tempdir"); + let dialog = new_dialog( + &settings, + &storage, + test_history(&root, &runtime.tasks()), + &runtime.tasks(), + ); for page in pages(&dialog) { let rows: Vec = descendants(&page) @@ -150,7 +173,13 @@ mod tests { let settings = MemorySettings::new(); let runtime = test_runtime(); let storage = test_storage(&runtime.tasks()); - let dialog = new_dialog(&settings, &storage, None, &runtime.tasks()); + let root = tempfile::tempdir().expect("tempdir"); + let dialog = new_dialog( + &settings, + &storage, + test_history(&root, &runtime.tasks()), + &runtime.tasks(), + ); let history_group = dialog.imp().general_page.history_group().downgrade(); let freed = dialog.downgrade(); @@ -170,13 +199,14 @@ mod tests { let runtime = test_runtime(); let storage = test_storage(&runtime.tasks()); let root = tempfile::tempdir().expect("tempdir"); - let paths = StoragePaths::under(root.path(), "tablepro-test", "app.tablepro.TablePro.Devel"); - let history = open_history(&runtime, &paths); + let service = test_history(&root, &runtime.tasks()); + let history = block_on(&runtime, service.ready_store()).expect("the history to open"); record_one(&runtime, &history); assert_eq!(count(&runtime, &history), 1); - let dialog = new_dialog(&settings, &storage, Some(history.clone()), &runtime.tasks()); - assert!(clear_button(&dialog).is_sensitive()); + let dialog = new_dialog(&settings, &storage, service, &runtime.tasks()); + crate::test_support::wait_until(WAIT, || clear_button(&dialog).is_sensitive()) + .expect("Clear offered once the history opened"); let window = present_on_a_window(&dialog); clear_button(&dialog).emit_clicked(); confirm_the_alert(&window); @@ -230,10 +260,6 @@ mod tests { .find(|text| text.contains("cleared") || text.contains("could not be cleared")) } - fn open_history(runtime: &crate::runtime::AppRuntime, paths: &StoragePaths) -> tablepro_storage::QueryHistory { - block_on(runtime, tablepro_storage::QueryHistory::open(paths)).expect("open the history") - } - fn record_one(runtime: &crate::runtime::AppRuntime, history: &tablepro_storage::QueryHistory) { let entry = tablepro_storage::query_history::NewEntry { query: "SELECT 1".to_owned(), diff --git a/linux/crates/app/src/ui/workspace/status_pages.rs b/linux/crates/app/src/ui/workspace/status_pages.rs index 1a15e55d74..8013e2991c 100644 --- a/linux/crates/app/src/ui/workspace/status_pages.rs +++ b/linux/crates/app/src/ui/workspace/status_pages.rs @@ -25,7 +25,21 @@ impl WorkspaceModel { dialog.present(self.dialog_parent().as_ref()); } + /// Say in an alert why the query database cannot be used. False + /// when it can. + fn alert_history_unavailable(&self, title: &str) -> bool { + let reason = self.history.availability().borrow().failure().map(str::to_owned); + let Some(reason) = reason else { + return false; + }; + self.show_error_alert(title, &reason); + true + } + pub(super) fn on_show_history(&mut self) { + if self.alert_history_unavailable(&crate::i18n::gettext("Query History Unavailable")) { + return; + } let dialog = HistoryDialog::new(HistoryDialogInit { history: self.history.clone(), connections: self.storage.connections().clone(), @@ -45,6 +59,9 @@ impl WorkspaceModel { } pub(super) fn on_show_saved_queries(&mut self) { + if self.alert_history_unavailable(&crate::i18n::gettext("Saved Queries Unavailable")) { + return; + } let dialog = SavedQueriesDialog::builder() .launch(SavedQueriesDialogInit { store: self.history.store().map(|history| history.saved_queries()), @@ -72,6 +89,9 @@ impl WorkspaceModel { /// The text is read now rather than when the dialog closes, so what /// is saved is what the user was looking at when they asked. pub(super) fn on_save_active_query(&self) { + if self.alert_history_unavailable(&crate::i18n::gettext("Could Not Save Query")) { + return; + } let Some(query) = self.active_editor_query() else { self.show_toast(&crate::i18n::gettext("Open a SQL editor tab to save a query.")); return; @@ -111,10 +131,6 @@ impl WorkspaceModel { } pub(super) fn on_save_query_named(&self, name: String, query: String) { - let Some(store) = self.history.store().map(|history| history.saved_queries()) else { - self.show_toast(&crate::i18n::gettext("The query database is not open yet.")); - return; - }; let Some(metadata) = self.context.active.metadata() else { self.show_toast(&crate::i18n::gettext( "Connect to a database first: a saved query is kept under its connection.", @@ -128,9 +144,18 @@ impl WorkspaceModel { connection_name: metadata.name, }; // The save lands even when the workspace is gone by then; only - // its toast is dropped. + // its toast is dropped. A database still opening is waited for. let input = self.input.clone(); + let (ready, availability) = (self.history.ready_store(), self.history.availability()); crate::runtime::spawn_held(&self.tasks, &*self.context.hold, async move { + let Some(store) = ready.await.map(|history| history.saved_queries()) else { + let reason = availability.borrow().failure().map(str::to_owned).unwrap_or_default(); + input.deliver(WorkspaceInput::ShowAlert { + title: crate::i18n::gettext("Could Not Save Query"), + body: reason, + }); + return; + }; let message = match store.save(new_query).await { Ok(outcome) if outcome.replaced => crate::i18n::gettext_f("Replaced “{name}”.", &[("name", &name)]), Ok(_) => crate::i18n::gettext_f("Saved “{name}”.", &[("name", &name)]), @@ -226,6 +251,75 @@ mod tests { disconnected.expect("the window back on the picker"); } + /// A history whose database cannot be read: a folder stands where + /// the file goes. + fn unreadable_history( + test: &TestWindow, + root: &tempfile::TempDir, + ) -> crate::services::history_service::HistoryService { + let paths = tablepro_storage::StoragePaths::under(root.path(), "tablepro-test", "app.tablepro.TablePro.Devel"); + std::fs::create_dir_all(paths.history_database()).expect("a folder where the database goes"); + let preferences = crate::services::history_preferences::HistoryPreferences { + retention_days: tokio::sync::watch::channel(30).1, + save: tokio::sync::watch::channel(true).1, + }; + let history = crate::services::history_service::HistoryService::start(paths, preferences, &test.services.tasks); + let availability = history.availability(); + wait_until(WAIT, || availability.borrow().is_settled()).expect("the open to end"); + history + } + + #[gtk4::test] + fn an_unreadable_history_says_why_where_it_is_opened() { + use gtk4::prelude::*; + use libadwaita::prelude::*; + + use crate::workspace::{answer, visible_alert}; + + let test = TestWindow::connected("HistoryUnreadable"); + let root = tempfile::tempdir().expect("tempdir"); + let history = unreadable_history(&test, &root); + test.workspace().with_model(|model| model.history = history); + + for (action, heading) in [ + ("win.show-history", "Query History Unavailable"), + ("win.show-saved-queries", "Saved Queries Unavailable"), + ] { + WidgetExt::activate_action(&test.window, action, None).expect(action); + wait_until(WAIT, || { + visible_alert(&test.window).is_some_and(|alert| alert.heading().as_deref() == Some(heading)) + }) + .unwrap_or_else(|_| panic!("{action} said nothing")); + let body = visible_alert(&test.window).map(|alert| alert.body().to_string()); + assert_eq!( + body.as_deref(), + Some("The query history file could not be read. It may be damaged.") + ); + assert!(answer(&test.window, "close")); + wait_until(WAIT, || visible_alert(&test.window).is_none()).expect("the alert to close"); + } + } + + #[gtk4::test] + fn saving_a_query_into_an_unreadable_history_says_why() { + use libadwaita::prelude::*; + + use crate::workspace::visible_alert; + + let test = TestWindow::connected("SaveQueryUnreadable"); + let root = tempfile::tempdir().expect("tempdir"); + let history = unreadable_history(&test, &root); + test.workspace().with_model(|model| { + model.history = history; + model.on_save_query_named("Orders".to_owned(), "SELECT 1".to_owned()); + }); + + wait_until(WAIT, || { + visible_alert(&test.window).is_some_and(|alert| alert.heading().as_deref() == Some("Could Not Save Query")) + }) + .expect("the save said why it failed"); + } + #[test] fn the_suggested_name_is_the_first_real_line() { assert_eq!( diff --git a/linux/crates/app/src/ui/workspace/workspace_actions.rs b/linux/crates/app/src/ui/workspace/workspace_actions.rs index d18a598c7c..428627e42c 100644 --- a/linux/crates/app/src/ui/workspace/workspace_actions.rs +++ b/linux/crates/app/src/ui/workspace/workspace_actions.rs @@ -14,7 +14,8 @@ use crate::services::history_availability::HistoryAvailability; type ActionInput = fn() -> WorkspaceInput; /// The two actions that need the query database, which opens on a -/// background task. +/// background task. Once it failed to open they say why when used, so +/// they are not left greyed out with no reason. const HISTORY_ACTIONS: [&str; 2] = ["show-history", "show-saved-queries"]; /// Add the workspace's actions to the window's own action map, which GTK @@ -38,7 +39,7 @@ pub(super) fn install(window: &adw::ApplicationWindow, input: &WorkspaceSender) .build() }); window.add_action_entries(actions); - // Insensitive until the query database reports it is open. + // Insensitive until the query database has opened or failed to. for name in HISTORY_ACTIONS { set_enabled(window, name, false); } @@ -68,12 +69,9 @@ pub(super) fn spawn_history_gate( return; }; for name in HISTORY_ACTIONS { - set_enabled(&window, name, state.is_ready()); + set_enabled(&window, name, state.is_settled()); } drop(window); - if let Some(failure) = state.failure() { - tracing::warn!(error = failure, "query history stays unavailable"); - } if availability.changed().await.is_err() { return; } @@ -107,7 +105,7 @@ mod tests { sender.send_replace(HistoryAvailability::Failed("no disk".to_owned())); crate::test_support::drain_main_context(); - assert!(!enabled(), "stayed enabled after the database failed"); + assert!(enabled(), "a failed database left the action greyed out with no reason"); gate.abort(); } From 90d1f8c77e079da1f79353256dd81fd832eb1460 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Sat, 17 Oct 2026 18:26:03 +0700 Subject: [PATCH 04/14] fix(linux): show unreadable saved queries as an error with Try Again --- .../crates/app/src/ui/saved_queries_dialog.rs | 117 +++++++++++++++++- 1 file changed, 116 insertions(+), 1 deletion(-) diff --git a/linux/crates/app/src/ui/saved_queries_dialog.rs b/linux/crates/app/src/ui/saved_queries_dialog.rs index 5cf5faab47..cefd3400bc 100644 --- a/linux/crates/app/src/ui/saved_queries_dialog.rs +++ b/linux/crates/app/src/ui/saved_queries_dialog.rs @@ -24,7 +24,11 @@ pub struct SavedQueriesDialog { groups: gtk::Box, stack: gtk::Stack, status_page: adw::StatusPage, + error_page: adw::StatusPage, queries: Vec, + /// Whether a list was read once. A read that fails after that keeps + /// the list on screen. + loaded: bool, } pub struct SavedQueriesDialogInit { @@ -56,6 +60,8 @@ pub enum SavedQueriesDialogOutput { #[derive(Debug)] pub enum SavedQueriesDialogCmd { Loaded(Vec), + /// The list could not be read, and why. + LoadFailed(String), Failed(String), Changed, } @@ -121,9 +127,23 @@ impl Component for SavedQueriesDialog { )) .build(); + let retry = gtk::Button::builder() + .label(crate::i18n::gettext("Try Again")) + .halign(gtk::Align::Center) + .build(); + retry.add_css_class("pill"); + let sender_for_retry = sender.clone(); + retry.connect_clicked(move |_| sender_for_retry.deliver(SavedQueriesDialogInput::Refresh)); + let error_page = adw::StatusPage::builder() + .icon_name(crate::ui::icons::DIALOG_ERROR) + .title(crate::i18n::gettext("Could Not Read Saved Queries")) + .child(&retry) + .build(); + let stack = gtk::Stack::new(); stack.add_named(&scroller, Some("list")); stack.add_named(&status_page, Some("empty")); + stack.add_named(&error_page, Some("error")); stack.set_visible_child_name("empty"); toolbar.set_content(Some(&stack)); root.set_child(Some(&toolbar)); @@ -136,7 +156,9 @@ impl Component for SavedQueriesDialog { groups, stack, status_page, + error_page, queries: Vec::new(), + loaded: false, }; sender.deliver(SavedQueriesDialogInput::Refresh); @@ -202,8 +224,16 @@ impl Component for SavedQueriesDialog { match msg { SavedQueriesDialogCmd::Loaded(queries) => { self.queries = queries; + self.loaded = true; self.render(&sender); } + SavedQueriesDialogCmd::LoadFailed(reason) if self.loaded => { + let _ = sender.output(SavedQueriesDialogOutput::ShowToast(reason)); + } + SavedQueriesDialogCmd::LoadFailed(reason) => { + self.error_page.set_description(Some(&reason)); + self.stack.set_visible_child_name("error"); + } SavedQueriesDialogCmd::Changed => sender.deliver(SavedQueriesDialogInput::Refresh), SavedQueriesDialogCmd::Failed(message) => { let _ = sender.output(SavedQueriesDialogOutput::ShowToast(message)); @@ -227,7 +257,9 @@ impl SavedQueriesDialog { Ok(queries) => SavedQueriesDialogCmd::Loaded(queries), Err(error) => { tracing::warn!(%error, "could not read the saved queries"); - SavedQueriesDialogCmd::Loaded(Vec::new()) + SavedQueriesDialogCmd::LoadFailed(crate::i18n::gettext( + "The query history file could not be read. It may be damaged.", + )) } } }); @@ -631,6 +663,89 @@ mod dialog_tests { .expect("the deleted query stayed in the list"); } + /// Take away the table the store reads, as a damaged file does, + /// through a connection of its own. + fn drop_the_table(root: &tempfile::TempDir) { + use tablepro_core::statement::{GuardedStatement, StatementBuilder}; + use tablepro_core::{ + CallOptions, ConnectEnvironment, ConnectOptions, ConnectTarget, DatabaseDriver, FileEndpoint, + ForeignKeyMode, TableRef, WorkspaceSettings, WriteBatch, WriteKind, + }; + use tokio_util::sync::CancellationToken; + + let paths = tablepro_storage::StoragePaths::under(root.path(), "tablepro", "app.tablepro.TablePro"); + let endpoint = FileEndpoint::open_existing(paths.history_database()).expect("the history file"); + let options = ConnectOptions { + target: ConnectTarget::File(endpoint), + workspace_settings: WorkspaceSettings::workspace(false), + liveness: tablepro_core::LivenessPolicy::DESKTOP, + environment: ConnectEnvironment::new("test"), + }; + let mut drop = StatementBuilder::new(); + drop.push_sql("DROP TABLE saved_queries"); + let batch = WriteBatch::new( + WriteKind::SchemaChanges, + TableRef::unqualified("saved_queries"), + vec![GuardedStatement::unguarded(drop.finish())], + ForeignKeyMode::Enforced, + ); + relm4::tokio::runtime::Builder::new_current_thread() + .enable_all() + .build() + .expect("runtime") + .block_on(async { + let connection = drivers_sqlite::SqliteDriver + .connect(options, CancellationToken::new()) + .await + .expect("connect"); + connection + .write_batch(&batch, CallOptions::new(None, CancellationToken::new())) + .await + .expect("the table dropped"); + connection.close().await.expect("close"); + }); + } + + fn visible_page(dialog: &SavedQueriesDialog) -> Option { + dialog.stack.visible_child_name().map(|name| name.to_string()) + } + + #[gtk4::test] + fn a_list_that_cannot_be_read_says_so_and_offers_try_again() { + let (root, store, local, _staging) = seeded(); + drop_the_table(&root); + let dialog = SavedQueriesDialog::builder().launch(SavedQueriesDialogInit { + store: Some(store), + active_connection: Some(local), + }); + + crate::test_support::wait_until(RENDER_WAIT, || { + visible_page(&dialog.model()).as_deref() == Some("error") + }) + .expect("the error page"); + let model = dialog.model(); + assert_eq!(model.error_page.title(), "Could Not Read Saved Queries"); + assert_eq!( + model.error_page.description().as_deref(), + Some("The query history file could not be read. It may be damaged.") + ); + let retry = model + .error_page + .child() + .and_downcast::() + .expect("a Try Again button"); + assert_eq!(retry.label().as_deref(), Some("Try Again")); + drop(model); + + retry.emit_clicked(); + crate::test_support::drain_main_context(); + assert_eq!( + visible_page(&dialog.model()).as_deref(), + Some("error"), + "Try Again found the table back" + ); + } + #[gtk4::test] fn a_dialog_with_no_database_shows_the_empty_state_rather_than_failing() { let dialog = SavedQueriesDialog::builder().launch(SavedQueriesDialogInit { From 0fdd1445f8f79908361fb20c23724660fda6708b Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Sat, 17 Oct 2026 18:28:44 +0700 Subject: [PATCH 05/14] fix(linux): show a Show CREATE TABLE failure in an alert --- .../crates/app/src/ui/workspace/structure.rs | 38 +++++++++++++++++-- 1 file changed, 35 insertions(+), 3 deletions(-) diff --git a/linux/crates/app/src/ui/workspace/structure.rs b/linux/crates/app/src/ui/workspace/structure.rs index 913262aa14..3862276567 100644 --- a/linux/crates/app/src/ui/workspace/structure.rs +++ b/linux/crates/app/src/ui/workspace/structure.rs @@ -231,10 +231,12 @@ impl WorkspaceModel { /// Sidebar "Show CREATE TABLE": the statements that recreate the /// table as it is defined now, from the dialect's /// `create_table_script`. The result lands in a fresh editor tab; no - /// Structure tab is touched. + /// Structure tab is touched. A failure can carry a long server + /// error, so it is an alert, which stays until it is read. pub(super) fn on_show_create_table(&self, schema: Option, table: String) { + let title = crate::i18n::gettext("Could Not Show CREATE TABLE"); let Some(session) = self.context.active.get() else { - self.show_toast(&crate::i18n::gettext("No active connection.")); + self.show_error_alert(&title, &crate::i18n::gettext("No active connection.")); return; }; let work = read_definition(session, TableRef::new(schema, table.clone())); @@ -257,7 +259,7 @@ impl WorkspaceModel { &[("error", &error_text::operation_error_message(&error))], ), }; - input.deliver(WorkspaceInput::ShowToast(message)); + input.deliver(WorkspaceInput::ShowAlert { title, body: message }); }); } @@ -496,4 +498,34 @@ mod tests { assert_eq!(heading.as_deref(), Some("Drop sales.orders?")); assert!(answer(&test.window, "cancel"), "the prompt has no Cancel button"); } + + /// A server's reason can be longer than a toast's one line. + #[gtk4::test] + fn a_create_table_that_cannot_be_read_says_why_in_an_alert() { + use tablepro_core::DriverError; + use tablepro_test_support::FakeReply; + + use crate::test_support::wait_until; + use crate::ui::session_window::tests::{TestWindow, WAIT}; + use crate::workspace::{answer, visible_alert}; + + let test = TestWindow::connected_with("ShowCreateFails", |script| { + script + .fetch_table_definition + .always(FakeReply::err(DriverError::server("permission denied for table users"))); + }); + test.workspace() + .with_model(|model| model.on_show_create_table(None, "users".to_owned())); + + wait_until(WAIT, || { + visible_alert(&test.window) + .is_some_and(|alert| alert.heading().as_deref() == Some("Could Not Show CREATE TABLE")) + }) + .expect("an alert for the failed read"); + let body = visible_alert(&test.window) + .map(|alert| alert.body().to_string()) + .unwrap_or_default(); + assert!(body.contains("permission denied for table users"), "{body}"); + assert!(answer(&test.window, "close")); + } } From 8e0719af591e9dda31b0d19e79101ea287680d43 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Sat, 17 Oct 2026 18:34:53 +0700 Subject: [PATCH 06/14] fix(linux): keep unreadable saved tabs in a banner, toast failing drafts --- linux/crates/app/src/ui/workspace/imp.rs | 2 + linux/crates/app/src/ui/workspace/mod.rs | 32 +++++++++++- .../app/src/ui/workspace/workspace_tabs.rs | 44 ++++++++++++----- .../crates/app/src/workspace/draft_writer.rs | 49 ++++++++++++++++++- linux/data/resources/ui/workspace.ui | 11 +++++ 5 files changed, 123 insertions(+), 15 deletions(-) diff --git a/linux/crates/app/src/ui/workspace/imp.rs b/linux/crates/app/src/ui/workspace/imp.rs index ccc04d4477..e0b2d9942b 100644 --- a/linux/crates/app/src/ui/workspace/imp.rs +++ b/linux/crates/app/src/ui/workspace/imp.rs @@ -64,6 +64,8 @@ pub struct Workspace { #[template_child] pub reconnect_banner: TemplateChild, #[template_child] + pub layout_banner: TemplateChild, + #[template_child] pub tab_stack: TemplateChild, #[template_child] pub tab_view: TemplateChild, diff --git a/linux/crates/app/src/ui/workspace/mod.rs b/linux/crates/app/src/ui/workspace/mod.rs index e6c26cabab..cbe5146248 100644 --- a/linux/crates/app/src/ui/workspace/mod.rs +++ b/linux/crates/app/src/ui/workspace/mod.rs @@ -124,6 +124,9 @@ pub(crate) struct WorkspaceModel { /// Says the sidebar's list could not be read again, until a read /// lands. catalog_banner: adw::Banner, + /// Says the saved tabs could not be read, until Reset moves them + /// aside. + layout_banner: adw::Banner, row_op_spinner: adw::Spinner, table_search: gtk::SearchEntry, /// Removes a changed host key from known_hosts. `None` without ssh. @@ -167,6 +170,8 @@ pub(crate) struct WorkspaceModel { /// kept: the close has already cleared them. closed_tabs_stack: Rc>>, _watches: SessionWatches, + /// Tells the user once when editor drafts start failing to land. + _draft_failures: tablepro_session::runtime::WatchForwarder, } /// The workspace's own slot: a tab reads it for the session, and it @@ -180,6 +185,23 @@ fn session_slot(window_context: &SessionContext, session: &Arc) -> Rc tablepro_session::runtime::WatchForwarder { + let failing = input.clone(); + tablepro_session::runtime::WatchForwarder::new( + drafts.failing(), + move |failed: bool| { + if failed { + failing.deliver(WorkspaceInput::DraftsNotSaved); + } + }, + sidebar::spawn_local, + ) +} + struct WorkspaceTabParts { tab_view: adw::TabView, index: Rc, @@ -392,9 +414,11 @@ pub(crate) enum WorkspaceInput { /// The session's saved tabs and their editor text, read off the GTK /// thread. WorkspaceLoaded(workspace_tabs::LoadedWorkspace), - /// The toast's Reset: move the unreadable saved tabs aside. + /// The banner's Reset: move the unreadable saved tabs aside. ResetWorkspaceLayout, WorkspaceLayoutReset(Result, crate::workspace::WorkspaceSaveError>), + /// An editor draft failed to land after the last one did. + DraftsNotSaved, ShowHistory, ShowSavedQueries, SaveActiveQuery, @@ -527,6 +551,7 @@ impl Workspace { actions.push(shell::SHOW_SIDEBAR); let history_gate = workspace_actions::spawn_history_gate(&window, history.availability()); let watches = SessionWatches::new(&session, &input); + let draft_failures = draft_failure_watch(&drafts, &input); let driver_id = session.metadata().driver_id.clone(); let driver_name = context @@ -556,6 +581,7 @@ impl Workspace { toast_overlay: imp.toast_overlay.get(), reconnect_banner: imp.reconnect_banner.get(), catalog_banner: imp.catalog_banner.get(), + layout_banner: imp.layout_banner.get(), row_op_spinner: imp.row_op_spinner.get(), table_search: imp.table_search.get(), ssh: services.ssh(), @@ -581,6 +607,7 @@ impl Workspace { workspace: persistence, closed_tabs_stack: Rc::new(RefCell::new(VecDeque::with_capacity(CLOSED_TABS_CAPACITY))), _watches: watches, + _draft_failures: draft_failures, }; imp.model.replace(Some(model)); workspace.register_close_blocker(exports); @@ -963,6 +990,9 @@ impl WorkspaceModel { WorkspaceInput::WorkspaceLoaded(loaded) => self.on_workspace_loaded(loaded), WorkspaceInput::ResetWorkspaceLayout => self.on_reset_workspace_layout(), WorkspaceInput::WorkspaceLayoutReset(outcome) => self.on_workspace_layout_reset(outcome), + WorkspaceInput::DraftsNotSaved => self.show_toast(&crate::i18n::gettext( + "Editor text could not be saved to disk, so it may not come back after TablePro restarts.", + )), WorkspaceInput::ShowHistory => self.on_show_history(), WorkspaceInput::ShowSavedQueries => self.on_show_saved_queries(), WorkspaceInput::ImportCsvRequested { schema, table } => self.on_import_csv_requested(schema, table), diff --git a/linux/crates/app/src/ui/workspace/workspace_tabs.rs b/linux/crates/app/src/ui/workspace/workspace_tabs.rs index 5a401d1090..15f6e37edf 100644 --- a/linux/crates/app/src/ui/workspace/workspace_tabs.rs +++ b/linux/crates/app/src/ui/workspace/workspace_tabs.rs @@ -66,7 +66,7 @@ impl WorkspaceModel { } WorkspaceLoad::Empty => self.retain_drafts(&key, std::iter::empty()), // Which drafts the file names is unknown, so none is deleted. - WorkspaceLoad::Blocked(_) => self.show_unreadable_workspace_toast(), + WorkspaceLoad::Blocked(_) => self.layout_banner.set_revealed(true), } if unreadable > 0 { let count = u32::try_from(unreadable).unwrap_or(u32::MAX); @@ -130,16 +130,6 @@ impl WorkspaceModel { self.open_tab(spec) } - fn show_unreadable_workspace_toast(&self) { - let toast = adw::Toast::builder() - .title(crate::i18n::gettext("Saved tabs could not be read")) - .use_markup(false) - .button_label(crate::i18n::gettext("Reset")) - .action_name("win.reset-workspace-layout") - .build(); - self.toast_overlay.add_toast(toast); - } - /// Move the unreadable saved tabs aside, so the open ones can be /// saved in their place. pub(super) fn on_reset_workspace_layout(&self) { @@ -157,11 +147,18 @@ impl WorkspaceModel { if let Some(moved) = moved { tracing::info!(moved = %moved.display(), "moved the unreadable saved tabs aside"); } + self.layout_banner.set_revealed(false); self.persist_workspace_state(); } + // The banner stays, so Reset can be tried again. Err(error) => { tracing::warn!(%error, "could not move the saved tabs aside"); - self.show_toast(&crate::i18n::gettext("Saved tabs could not be reset")); + self.show_error_alert( + &crate::i18n::gettext("Could Not Reset Saved Tabs"), + &crate::i18n::gettext( + "The unreadable file could not be moved aside, so the open tabs are still not saved.", + ), + ); } } } @@ -719,6 +716,29 @@ mod tests { assert!(editor.upgrade().is_none(), "the closed page's editor is still alive"); } + #[gtk4::test] + fn unreadable_saved_tabs_are_announced_until_reset() { + let test = TestWindow::picker("UnreadableSavedTabs"); + let dir = test.services.storage.paths().workspaces_dir(); + std::fs::create_dir_all(&dir).expect("the workspaces folder"); + std::fs::write( + dir.join(crate::workspace::WorkspaceFileName::for_key(&test.key()).file_name()), + b"{ not json", + ) + .expect("an unreadable file"); + + test.window + .send(SessionWindowInput::OpenSaved(Box::new(test.saved.clone()))); + test.wait_for_page("workspace"); + let banner = test.workspace().imp().layout_banner.get(); + wait_until(WAIT, || banner.is_revealed()).expect("the banner says the tabs are not saved"); + assert_eq!(banner.action_name().as_deref(), Some("win.reset-workspace-layout")); + + WidgetExt::activate_action(&test.window, "win.reset-workspace-layout", None).expect("Reset"); + + wait_until(WAIT, || !banner.is_revealed()).expect("the banner goes once the file is moved aside"); + } + #[gtk4::test] fn tab_view_freed_after_teardown() { let test = TestWindow::connected("TabViewFreed"); diff --git a/linux/crates/app/src/workspace/draft_writer.rs b/linux/crates/app/src/workspace/draft_writer.rs index fbf1f77886..cd4c84a241 100644 --- a/linux/crates/app/src/workspace/draft_writer.rs +++ b/linux/crates/app/src/workspace/draft_writer.rs @@ -8,6 +8,7 @@ use gtk4::glib; use gtk4::prelude::*; use tablepro_session::runtime::{LatestWinsWriter, Tasks}; use tablepro_storage::{DraftId, DraftScope, DraftStore}; +use tokio::sync::watch; use super::PersistDebouncer; @@ -27,6 +28,9 @@ pub struct DraftWriter { tasks: Tasks, slots: RefCell>>, failures: Arc>>, + /// Whether the last write of a draft failed. A run of failures is + /// one change of it, so the user is told once. + failing: Arc>, } struct Slot { @@ -42,9 +46,15 @@ impl DraftWriter { tasks, slots: RefCell::new(HashMap::new()), failures: Arc::new(Mutex::new(Vec::new())), + failing: Arc::new(watch::channel(false).0), }) } + /// Whether drafts are failing to land, now and after every change. + pub fn failing(&self) -> watch::Receiver { + self.failing.subscribe() + } + /// Note that this tab's text changed. The first call for an id also /// starts tracking the buffer. pub fn schedule(self: &Rc, scope: &DraftScope, id: DraftId, buffer: >k4::TextBuffer) { @@ -127,9 +137,11 @@ impl DraftWriter { } let store = self.store.clone(); let scope_for_writes = scope.clone(); - let failures = self.failures.clone(); + let (failures, failing) = (self.failures.clone(), self.failing.clone()); let writer = LatestWinsWriter::spawn(&self.tasks, move |text: &String| { - if let Err(error) = store.write_blocking(&scope_for_writes, id, text) { + let written = store.write_blocking(&scope_for_writes, id, text); + failing.send_if_modified(|failing| std::mem::replace(failing, written.is_err()) != written.is_err()); + if let Err(error) = written { record(&failures, &error); } }); @@ -255,6 +267,39 @@ mod tests { assert_eq!(fixture.read(id).as_deref(), Some("SELECT 1")); } + #[gtk4::test] + fn a_run_of_failed_writes_is_one_change_and_a_write_that_lands_ends_it() { + let runtime = test_runtime(); + let root = tempfile::tempdir().expect("tempdir"); + // A file where the drafts folder goes, so no draft can be written. + let blocked = root.path().join("drafts"); + std::fs::write(&blocked, b"not a folder").expect("seed"); + let writer = DraftWriter::new(DraftStore::new(blocked.clone()), runtime.tasks()); + let scope = DraftScope::new("workspace_state").expect("a scope"); + let mut failing = writer.failing(); + assert!(!*failing.borrow_and_update()); + let buffer = gtk4::TextBuffer::new(None); + let write = |text: &str| { + buffer.set_text(text); + writer.schedule(&scope, DraftId::new(), &buffer); + let flushing = writer.flush_all(); + crate::test_support::finish_on_main(WAIT, flushing).expect("the flush to end") + }; + + assert!(!write("SELECT 1").is_empty(), "the write landed"); + assert!(failing.has_changed().expect("the writer is there")); + assert!(*failing.borrow_and_update()); + assert!(!write("SELECT 2").is_empty()); + assert!( + !failing.has_changed().expect("the writer is there"), + "a second failure in the run changed it again" + ); + + std::fs::remove_file(&blocked).expect("unblock"); + assert!(write("SELECT 3").is_empty(), "the write failed"); + assert!(!*failing.borrow_and_update()); + } + #[gtk4::test] fn discard_deletes_file() { let fixture = Fixture::new(); diff --git a/linux/data/resources/ui/workspace.ui b/linux/data/resources/ui/workspace.ui index 2e30ae08a3..01e433d22d 100644 --- a/linux/data/resources/ui/workspace.ui +++ b/linux/data/resources/ui/workspace.ui @@ -140,6 +140,17 @@ win.reconnect + + + + false + Saved tabs could not be read, so the open tabs are not saved + Reset + win.reset-workspace-layout + Moves the unreadable file aside, so the open tabs can be saved + + crossfade From 500c4eb0d47ba3b0b865272cb80e72a71d56df8f Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Sat, 17 Oct 2026 18:36:27 +0700 Subject: [PATCH 07/14] fix(linux): require a name in query dialogs and say why a rename failed --- linux/crates/app/src/ui/mod.rs | 1 + linux/crates/app/src/ui/name_entry.rs | 39 ++++++++ .../crates/app/src/ui/saved_queries_dialog.rs | 91 ++++++++++++++++--- linux/crates/app/src/ui/workspace/mod.rs | 3 - linux/crates/app/src/ui/workspace/row_ops.rs | 42 --------- .../app/src/ui/workspace/status_pages.rs | 8 +- linux/crates/storage/src/error.rs | 3 + linux/crates/storage/src/saved_queries/mod.rs | 8 +- linux/po/POTFILES.in | 1 + 9 files changed, 131 insertions(+), 65 deletions(-) create mode 100644 linux/crates/app/src/ui/name_entry.rs diff --git a/linux/crates/app/src/ui/mod.rs b/linux/crates/app/src/ui/mod.rs index 07fcfdd166..0e5dc212b5 100644 --- a/linux/crates/app/src/ui/mod.rs +++ b/linux/crates/app/src/ui/mod.rs @@ -23,6 +23,7 @@ mod host_key_failure; mod host_key_review; pub(crate) mod icons; mod import_dialog; +mod name_entry; mod picker; mod plain_toast; mod preferences_dialog; diff --git a/linux/crates/app/src/ui/name_entry.rs b/linux/crates/app/src/ui/name_entry.rs new file mode 100644 index 0000000000..a360353dcf --- /dev/null +++ b/linux/crates/app/src/ui/name_entry.rs @@ -0,0 +1,39 @@ +use libadwaita as adw; +use libadwaita::prelude::*; + +/// Keep `response` off while `entry` holds no name, so Enter in the row +/// and the button both wait for one instead of closing on nothing. +pub(crate) fn require_name(dialog: &adw::AlertDialog, response: &str, entry: &adw::EntryRow) { + let named = |entry: &adw::EntryRow| !entry.text().trim().is_empty(); + dialog.set_response_enabled(response, named(entry)); + let dialog = dialog.downgrade(); + let response = response.to_owned(); + entry.connect_changed(move |entry| { + if let Some(dialog) = dialog.upgrade() { + dialog.set_response_enabled(&response, named(entry)); + } + }); +} + +#[cfg(test)] +mod tests { + use super::*; + + #[gtk4::test] + fn the_response_waits_for_a_name() { + adw::init().expect("libadwaita initialises under the test backend"); + let dialog = adw::AlertDialog::new(Some("Save Query"), None); + dialog.add_response("save", "Save"); + let entry = adw::EntryRow::new(); + entry.set_text(" "); + + require_name(&dialog, "save", &entry); + assert!(!dialog.is_response_enabled("save"), "a blank name can be saved"); + + entry.set_text("daily"); + assert!(dialog.is_response_enabled("save")); + + entry.set_text(""); + assert!(!dialog.is_response_enabled("save")); + } +} diff --git a/linux/crates/app/src/ui/saved_queries_dialog.rs b/linux/crates/app/src/ui/saved_queries_dialog.rs index cefd3400bc..87b773ab82 100644 --- a/linux/crates/app/src/ui/saved_queries_dialog.rs +++ b/linux/crates/app/src/ui/saved_queries_dialog.rs @@ -20,6 +20,8 @@ pub struct SavedQueriesDialog { store: Option, active_connection: Option, root: adw::Dialog, + /// The dialog's own, since it covers the window's. + toasts: adw::ToastOverlay, search: gtk::SearchEntry, groups: gtk::Box, stack: gtk::Stack, @@ -53,8 +55,6 @@ pub enum SavedQueriesDialogInput { pub enum SavedQueriesDialogOutput { OpenInNewTab(String), ReplaceCurrentTabQuery(String), - CopyToClipboard(String), - ShowToast(String), } #[derive(Debug)] @@ -146,12 +146,15 @@ impl Component for SavedQueriesDialog { stack.add_named(&error_page, Some("error")); stack.set_visible_child_name("empty"); toolbar.set_content(Some(&stack)); - root.set_child(Some(&toolbar)); + let toasts = adw::ToastOverlay::new(); + toasts.set_child(Some(&toolbar)); + root.set_child(Some(&toasts)); let model = SavedQueriesDialog { store: init.store, active_connection: init.active_connection, root: root.clone(), + toasts, search, groups, stack, @@ -183,7 +186,7 @@ impl Component for SavedQueriesDialog { } SavedQueriesDialogInput::CopySql(id) => { if let Some(query) = self.query_for(id) { - let _ = sender.output(SavedQueriesDialogOutput::CopyToClipboard(query)); + crate::ui::clipboard::copy_text(&self.root, query, |message| self.toast(message)); } } SavedQueriesDialogInput::RenameRequested(id) => self.prompt_rename(id, &sender), @@ -192,14 +195,9 @@ impl Component for SavedQueriesDialog { sender.oneshot_command(async move { match store.rename(id, &name).await { Ok(()) => SavedQueriesDialogCmd::Changed, - // The only rename that fails for a reason the - // user can act on is a name the connection - // already has. Err(error) => { tracing::warn!(%error, "could not rename the saved query"); - SavedQueriesDialogCmd::Failed(crate::i18n::gettext( - "That name is already used by another saved query.", - )) + SavedQueriesDialogCmd::Failed(rename_failure_text(&error)) } } }); @@ -227,16 +225,14 @@ impl Component for SavedQueriesDialog { self.loaded = true; self.render(&sender); } - SavedQueriesDialogCmd::LoadFailed(reason) if self.loaded => { - let _ = sender.output(SavedQueriesDialogOutput::ShowToast(reason)); - } + SavedQueriesDialogCmd::LoadFailed(reason) if self.loaded => self.toast(&reason), SavedQueriesDialogCmd::LoadFailed(reason) => { self.error_page.set_description(Some(&reason)); self.stack.set_visible_child_name("error"); } SavedQueriesDialogCmd::Changed => sender.deliver(SavedQueriesDialogInput::Refresh), SavedQueriesDialogCmd::Failed(message) => { - let _ = sender.output(SavedQueriesDialogOutput::ShowToast(message)); + self.toast(&message); sender.deliver(SavedQueriesDialogInput::Refresh); } } @@ -248,6 +244,10 @@ impl SavedQueriesDialog { &self.root } + fn toast(&self, message: &str) { + self.toasts.add_toast(crate::ui::plain_toast(message)); + } + fn load(&self, sender: &ComponentSender) { let Some(store) = self.store.clone() else { return; @@ -374,7 +374,10 @@ impl SavedQueriesDialog { return; }; let dialog = adw::AlertDialog::new(Some(&crate::i18n::gettext("Rename Saved Query")), None); - let entry = adw::EntryRow::builder().title(crate::i18n::gettext("Name")).build(); + let entry = adw::EntryRow::builder() + .title(crate::i18n::gettext("Name")) + .activates_default(true) + .build(); entry.set_text(¤t); let group = adw::PreferencesGroup::new(); group.add(&entry); @@ -384,6 +387,7 @@ impl SavedQueriesDialog { dialog.set_response_appearance("rename", adw::ResponseAppearance::Suggested); dialog.set_default_response(Some("rename")); dialog.set_close_response("cancel"); + crate::ui::name_entry::require_name(&dialog, "rename", &entry); let sender = sender.clone(); dialog.connect_response(None, move |_, response| { @@ -425,6 +429,18 @@ impl SavedQueriesDialog { } } +/// What a failed rename tells the user. A taken name is the one the +/// user can fix from the dialog. +fn rename_failure_text(error: &tablepro_storage::StorageError) -> String { + use tablepro_storage::StorageError; + match error { + StorageError::NameTaken => crate::i18n::gettext("That name is already used by another saved query."), + StorageError::TooLarge { .. } => crate::i18n::gettext("That name is too long."), + StorageError::NotFound => crate::i18n::gettext("That saved query was deleted elsewhere."), + _ => crate::i18n::gettext("The saved query could not be renamed."), + } +} + /// Everything matching the search, grouped by connection, with the /// active connection's group first and the rest in the order the store /// returned them. @@ -746,6 +762,51 @@ mod dialog_tests { ); } + #[gtk4::test] + fn copying_a_query_says_so_inside_the_dialog() { + let (_root, store, local, _staging) = seeded(); + let dialog = SavedQueriesDialog::builder().launch(SavedQueriesDialogInit { + store: Some(store), + active_connection: Some(local), + }); + crate::test_support::wait_until(RENDER_WAIT, || row_titles(&dialog.model()).len() == 2) + .expect("the saved queries never rendered"); + let id = dialog.model().queries[0].id; + + dialog + .sender() + .send(SavedQueriesDialogInput::CopySql(id)) + .expect("send"); + crate::test_support::drain_main_context(); + + let toasts = dialog.model().toasts.clone(); + assert!( + crate::test_support::descendants(&toasts) + .iter() + .filter_map(|widget| widget.downcast_ref::()) + .any(|label| label.label() == "Copied to clipboard"), + "the toast went somewhere the dialog covers" + ); + } + + #[test] + fn a_rename_failure_says_what_went_wrong() { + use tablepro_storage::StorageError; + + assert_eq!( + rename_failure_text(&StorageError::NameTaken), + "That name is already used by another saved query." + ); + assert_eq!( + rename_failure_text(&StorageError::NotFound), + "That saved query was deleted elsewhere." + ); + assert_eq!( + rename_failure_text(&StorageError::NotInitialised), + "The saved query could not be renamed." + ); + } + #[gtk4::test] fn a_dialog_with_no_database_shows_the_empty_state_rather_than_failing() { let dialog = SavedQueriesDialog::builder().launch(SavedQueriesDialogInit { diff --git a/linux/crates/app/src/ui/workspace/mod.rs b/linux/crates/app/src/ui/workspace/mod.rs index cbe5146248..5a5d71bc35 100644 --- a/linux/crates/app/src/ui/workspace/mod.rs +++ b/linux/crates/app/src/ui/workspace/mod.rs @@ -455,8 +455,6 @@ pub(crate) enum WorkspaceInput { tab: Uuid, outcome: crate::workspace::WorkOutcome, }, - /// Text to copy as it is, such as a saved query. - CopyText(String), WorkspaceTabClosed(Uuid), /// Each tab still asks about its own unsaved changes. CloseOtherWorkspaceTabs(Uuid), @@ -1024,7 +1022,6 @@ impl WorkspaceModel { WorkspaceInput::ExportEnded { tab, outcome } => { self.report_work(tab, crate::workspace::WorkKind::Export, outcome); } - WorkspaceInput::CopyText(text) => self.on_copy_text(text), WorkspaceInput::ReopenClosedTab => self.on_reopen_closed_tab(), WorkspaceInput::RefreshTab(id) => self.on_refresh_tab(id), WorkspaceInput::FocusTab(id) => self.focus_tab(id), diff --git a/linux/crates/app/src/ui/workspace/row_ops.rs b/linux/crates/app/src/ui/workspace/row_ops.rs index c31980f7bc..06493739f9 100644 --- a/linux/crates/app/src/ui/workspace/row_ops.rs +++ b/linux/crates/app/src/ui/workspace/row_ops.rs @@ -6,46 +6,4 @@ impl WorkspaceModel { pub(super) fn set_row_op_in_flight(&self, in_flight: bool) { self.row_op_spinner.set_visible(in_flight); } - - pub(super) fn on_copy_text(&self, text: String) { - crate::ui::clipboard::copy_text(&self.toast_overlay, text, |message| self.show_toast(message)); - } -} - -#[cfg(test)] -mod tests { - use super::*; - use crate::runtime::DeliverInput; - use crate::test_support::{descendants, drain_main_context}; - use crate::ui::session_window::tests::TestWindow; - use crate::ui::workspace::WorkspaceInput; - - #[gtk4::test] - fn a_copy_with_a_nul_says_so_and_leaves_the_clipboard_alone() { - let test = TestWindow::connected("CopyNul"); - let clipboard = test.window.clipboard(); - clipboard.set_text("before"); - let input = test.workspace().input().expect("the workspace's input"); - - input.deliver(WorkspaceInput::CopyText("a\0b".to_owned())); - drain_main_context(); - - assert_eq!(local_text(&clipboard).as_deref(), Some("before")); - let refusal = crate::ui::clipboard::ClipboardError::ContainsNul.message(); - assert!( - descendants(&test.workspace()) - .iter() - .filter_map(|widget| widget.downcast_ref::()) - .any(|label| label.label() == refusal), - "no toast said why" - ); - - input.deliver(WorkspaceInput::CopyText("a\tb".to_owned())); - drain_main_context(); - assert_eq!(local_text(&clipboard).as_deref(), Some("a\tb")); - } - - fn local_text(clipboard: >k4::gdk::Clipboard) -> Option { - clipboard.content()?.value(String::static_type()).ok()?.get().ok() - } } diff --git a/linux/crates/app/src/ui/workspace/status_pages.rs b/linux/crates/app/src/ui/workspace/status_pages.rs index 8013e2991c..7c7cafb374 100644 --- a/linux/crates/app/src/ui/workspace/status_pages.rs +++ b/linux/crates/app/src/ui/workspace/status_pages.rs @@ -75,8 +75,6 @@ impl WorkspaceModel { SavedQueriesDialogOutput::ReplaceCurrentTabQuery(text) => { WorkspaceInput::ReplaceActiveTabQuery(text) } - SavedQueriesDialogOutput::CopyToClipboard(text) => WorkspaceInput::CopyText(text), - SavedQueriesDialogOutput::ShowToast(text) => WorkspaceInput::ShowToast(text), }); } }); @@ -102,7 +100,10 @@ impl WorkspaceModel { } let dialog = adw::AlertDialog::new(Some(&crate::i18n::gettext("Save Query")), None); - let entry = adw::EntryRow::builder().title(crate::i18n::gettext("Name")).build(); + let entry = adw::EntryRow::builder() + .title(crate::i18n::gettext("Name")) + .activates_default(true) + .build(); entry.set_text(&suggested_query_name(&query)); let group = adw::PreferencesGroup::new(); group.add(&entry); @@ -112,6 +113,7 @@ impl WorkspaceModel { dialog.set_response_appearance("save", adw::ResponseAppearance::Suggested); dialog.set_default_response(Some("save")); dialog.set_close_response("cancel"); + crate::ui::name_entry::require_name(&dialog, "save", &entry); let input = self.input.clone(); dialog.connect_response(None, move |_, response| { diff --git a/linux/crates/storage/src/error.rs b/linux/crates/storage/src/error.rs index 5ae6f29443..2d4f2439e2 100644 --- a/linux/crates/storage/src/error.rs +++ b/linux/crates/storage/src/error.rs @@ -44,6 +44,9 @@ pub enum StorageError { #[error("a name is required")] EmptyName, + #[error("another saved query of the connection has that name")] + NameTaken, + #[error("export encoding failed: {0}")] Encode(#[from] tablepro_core::export::RenderError), diff --git a/linux/crates/storage/src/saved_queries/mod.rs b/linux/crates/storage/src/saved_queries/mod.rs index f2f7a26e1d..ac1f2c329b 100644 --- a/linux/crates/storage/src/saved_queries/mod.rs +++ b/linux/crates/storage/src/saved_queries/mod.rs @@ -155,7 +155,11 @@ impl SavedQueries { .bind(to_unix(SystemTime::now())) .bind(id) .execute(&self.pool) - .await? + .await + .map_err(|error| match &error { + sqlx::Error::Database(database) if database.is_unique_violation() => StorageError::NameTaken, + _ => StorageError::Database(error), + })? .rows_affected(); match affected { 0 => Err(StorageError::NotFound), @@ -390,7 +394,7 @@ mod tests { let error = store.rename(saved.id, "taken").await.expect_err("the name is taken"); - assert!(matches!(error, StorageError::Database(_)), "{error:?}"); + assert!(matches!(error, StorageError::NameTaken), "{error:?}"); // The failed rename left both queries as they were. assert_eq!(store.list(Some(connection)).await.expect("list").len(), 2); } diff --git a/linux/po/POTFILES.in b/linux/po/POTFILES.in index 7e1f7566fc..b7bffb2b26 100644 --- a/linux/po/POTFILES.in +++ b/linux/po/POTFILES.in @@ -335,6 +335,7 @@ crates/app/src/ui/host_key_review.rs crates/app/src/ui/icons.rs crates/app/src/ui/import_dialog.rs crates/app/src/ui/mod.rs +crates/app/src/ui/name_entry.rs crates/app/src/ui/picker/connection_menu.rs crates/app/src/ui/picker/connection_row.rs crates/app/src/ui/picker/mod.rs From 0d23d12e7ab6b635d812e4db3038f3502048fce9 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Sat, 17 Oct 2026 18:36:38 +0700 Subject: [PATCH 08/14] fix(linux): say that clearing history keeps the saved queries --- linux/crates/app/src/ui/history_dialog/mod.rs | 2 +- .../crates/app/src/ui/history_dialog/tests.rs | 37 +++++++++++++++++++ .../ui/preferences_dialog/history_group.rs | 2 +- .../resources/ui/preferences-history-group.ui | 2 +- 4 files changed, 40 insertions(+), 3 deletions(-) diff --git a/linux/crates/app/src/ui/history_dialog/mod.rs b/linux/crates/app/src/ui/history_dialog/mod.rs index d6cce01f49..c625690fa0 100644 --- a/linux/crates/app/src/ui/history_dialog/mod.rs +++ b/linux/crates/app/src/ui/history_dialog/mod.rs @@ -667,7 +667,7 @@ impl HistoryDialog { let alert = adw::AlertDialog::new( Some(&gettext("Clear All Query History?")), Some(&gettext( - "This permanently deletes every saved query, including pinned ones.", + "This permanently deletes every history entry, including pinned ones. Saved queries are kept.", )), ); alert.add_response("cancel", &gettext("Cancel")); diff --git a/linux/crates/app/src/ui/history_dialog/tests.rs b/linux/crates/app/src/ui/history_dialog/tests.rs index e7c47cf10e..c743a204d0 100644 --- a/linux/crates/app/src/ui/history_dialog/tests.rs +++ b/linux/crates/app/src/ui/history_dialog/tests.rs @@ -698,6 +698,43 @@ fn deleting_a_row_already_gone_removes_it_without_undo() { ); } +#[gtk4::test] +fn clear_all_says_saved_queries_are_kept_and_keeps_them() { + use crate::workspace::{answer, visible_alert}; + use tablepro_storage::saved_queries::NewSavedQuery; + + let harness = Harness::new(); + harness.record("SELECT 1"); + let saved = harness.store.saved_queries(); + block_on( + &harness.runtime, + saved.save(NewSavedQuery { + name: "daily".to_owned(), + query: "SELECT 1".to_owned(), + connection_id: Uuid::new_v4(), + connection_name: "local".to_owned(), + }), + ) + .expect("a saved query"); + let dialog = harness.presented_dialog(); + wait_until(WAIT, || entries(&dialog).len() == 1).expect("the first page"); + + dialog.activate_action("history.clear-all", None).expect("the action"); + wait_until(WAIT, || visible_alert(&harness.window).is_some()).expect("the confirmation"); + let body = visible_alert(&harness.window) + .map(|alert| alert.body().to_string()) + .unwrap_or_default(); + assert_eq!( + body, + "This permanently deletes every history entry, including pinned ones. Saved queries are kept." + ); + assert!(answer(&harness.window, "clear")); + + wait_until(WAIT, || harness.stored_queries().is_empty()).expect("the history cleared"); + let kept = block_on(&harness.runtime, saved.list(None)).expect("the saved queries"); + assert_eq!(kept.len(), 1, "clearing the history took the saved queries too"); +} + #[gtk4::test] fn undo_that_fails_says_so() { let harness = Harness::new(); diff --git a/linux/crates/app/src/ui/preferences_dialog/history_group.rs b/linux/crates/app/src/ui/preferences_dialog/history_group.rs index 7c8bac4e34..3a59bb6c98 100644 --- a/linux/crates/app/src/ui/preferences_dialog/history_group.rs +++ b/linux/crates/app/src/ui/preferences_dialog/history_group.rs @@ -187,7 +187,7 @@ impl HistoryGroup { let alert = adw::AlertDialog::new( Some(&crate::i18n::gettext("Clear All Query History?")), Some(&crate::i18n::gettext( - "This permanently deletes every saved query, including pinned ones.", + "This permanently deletes every history entry, including pinned ones. Saved queries are kept.", )), ); alert.add_response("cancel", &crate::i18n::gettext("Cancel")); diff --git a/linux/data/resources/ui/preferences-history-group.ui b/linux/data/resources/ui/preferences-history-group.ui index 80cca2738f..4081677755 100644 --- a/linux/data/resources/ui/preferences-history-group.ui +++ b/linux/data/resources/ui/preferences-history-group.ui @@ -27,7 +27,7 @@ Clear query history - Removes every saved query, including pinned ones. + Removes every history entry, including pinned ones. Saved queries are kept.