feat(linux): sidebar refresh, save to .sql, and visible storage errors

linux/crates/app/src/application/accelerators.rs

@@ -106,7 +106,14 @@ pub const ACCELERATORS: &[Accelerator] = &[
     ),
     stock("tab-view.select-tenth", &["<Alt>0"], Unlisted),
     local(BrowseTab, BUBBLE, "browse.filter", &["<Primary>f"], Listed(BrowseTable)),
-    local(BrowseTab, BUBBLE, "browse.refresh", &["F5"], Listed(BrowseTable)),
+    // F5 as in GNOME Files, and Ctrl+R as the GNOME HIG names Reload.
+    local(
+        BrowseTab,
+        BUBBLE,
+        "browse.refresh",
+        &["F5", "<Primary>r"],
+        Listed(BrowseTable),
+    ),
     local(BrowseTab, BUBBLE, "browse.save", &["<Primary>s"], Listed(BrowseTable)),
     local(BrowseTab, BUBBLE, "browse.undo", &["<Primary>z"], Listed(BrowseTable)),
     local(
@@ -226,6 +233,27 @@ pub const ACCELERATORS: &[Accelerator] = &[
         &["<Primary>slash"],
         Listed(ShortcutSection::SqlEditor),
     ),
+    local(
+        SqlEditorTab,
+        BUBBLE,
+        "editor.open-file",
+        &["<Primary>o"],
+        Listed(ShortcutSection::SqlEditor),
+    ),
+    local(
+        SqlEditorTab,
+        BUBBLE,
+        "editor.save",
+        &["<Primary>s"],
+        Listed(ShortcutSection::SqlEditor),
+    ),
+    local(
+        SqlEditorTab,
+        BUBBLE,
+        "editor.save-as",
+        &["<Primary><Shift>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(
@@ -256,7 +284,6 @@ pub const ACCELERATORS: &[Accelerator] = &[
         &["<Primary><Shift>g"],
         Listed(ShortcutSection::SqlEditor),
     ),
-    application("win.save-query", &["<Primary><Shift>s"], ShortcutSection::SqlEditor),
     local(
         StructureTab,
         BUBBLE,

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(),
     )

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(|| {

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");

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,18 +11,88 @@
 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),
             _ => None,
         }
     }
 }
+
+/// 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.")
+        );
+    }
+}

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())
         }
     }

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

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));
+    }
+}

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::close_popup_or_stop),