Skip to content

Commit 2fa5b49

Browse files
datlechinclaude
andcommitted
refactor(linux): A4 driver_id helper + A5 StatusKind enum
Two High-tier audit items the per-day sprint slices missed. A4 (latent SQL-quoting bug): The fallback `current_driver_id.clone().unwrap_or_else(|| "postgres".to_string())` was duplicated at 7 call sites across browse.rs and row_ops.rs. If anything ever produced a non-Postgres connection without setting current_driver_id, quote_ident would silently corrupt SQL. Collapsed to a single `App::driver_id(&self) -> &str` helper in app/mod.rs that logs `tracing::warn!` on the fallback path so the bug becomes visible the first time it triggers. Call sites now just `self.driver_id().to_string()`. A5 (i18n correctness): `set_status_page` reverse-engineered caller intent by sniffing localised title strings: if title.eq_ignore_ascii_case("failed") || title.to_lowercase().contains("error") Breaks the moment a translation uses different vocabulary for "Failed" / "Error" / "No connection". Replaced with an explicit `StatusKind` enum (Info / Error / Disconnected) and `kind.icon()` returning the symbolic icon name. All 4 callers updated to pass the kind explicitly: - browse.rs `on_load_failed` → Error - connection.rs `on_connected` → Info - editor_tabs.rs `on_open_editor` → Disconnected - editor_tabs.rs `show_welcome_or_grid_after_editor_close` → Info Verification: - cargo clippy --workspace --all-targets -- -D warnings: zero warnings - cargo test --workspace --lib: 43 pass, 1 ignored - cargo fmt --all -- --check: clean Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
1 parent dd24e13 commit 2fa5b49

6 files changed

Lines changed: 57 additions & 21 deletions

File tree

linux/crates/app/src/ui/app/browse.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ use crate::services::database_service;
88
use crate::ui::editor::update_schema_buffer;
99
use crate::ui::grid::build_column_view;
1010

11-
use super::{App, AppMsg, ExportFormat, clear_box, qualified_label, render_csv, render_json};
11+
use super::{App, AppMsg, ExportFormat, StatusKind, clear_box, qualified_label, render_csv, render_json};
1212

1313
impl App {
1414
pub(super) fn on_select_table(&mut self, schema: Option<String>, name: String, sender: ComponentSender<Self>) {
@@ -33,7 +33,7 @@ impl App {
3333
let Some(conn) = database_service::instance().active() else {
3434
return;
3535
};
36-
let driver_id = self.current_driver_id.clone().unwrap_or_else(|| "postgres".to_string());
36+
let driver_id = self.driver_id().to_string();
3737
let table_for_async = table.clone();
3838
let sender_clone = sender.clone();
3939
sender.command(move |_, shutdown| {
@@ -206,7 +206,7 @@ impl App {
206206
pub(super) fn on_load_failed(&self, msg: String) {
207207
tracing::warn!(error = %msg, "load failed");
208208
self.set_row_op_in_flight(false);
209-
self.set_status_page(&crate::tr!("Failed"), &msg);
209+
self.set_status_page(StatusKind::Error, &crate::tr!("Failed"), &msg);
210210
}
211211

212212
pub(super) fn fetch_current_page(&self, sender: ComponentSender<App>) {
@@ -220,7 +220,7 @@ impl App {
220220
sender.input(AppMsg::LoadFailed("no active connection".into()));
221221
return;
222222
};
223-
let driver_id = self.current_driver_id.clone().unwrap_or_else(|| "postgres".to_string());
223+
let driver_id = self.driver_id().to_string();
224224
let order_by = self.current_sort.and_then(|(idx, asc)| {
225225
self.current_columns.get(idx).map(|c| {
226226
let name = tablepro_core::sql_dialect::quote_ident(&driver_id, &c.name);

linux/crates/app/src/ui/app/connection.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ use crate::services::database_service::ConnectionHealth;
99
use crate::services::{connection_service, database_service};
1010
use crate::ui::connect_dialog::{ConnectDialog, ConnectDialogInit, ConnectDialogOutput};
1111

12-
use super::{App, AppMsg, qualified_label};
12+
use super::{App, AppMsg, StatusKind, qualified_label};
1313

1414
impl App {
1515
pub(super) fn on_open_connect(&mut self, sender: ComponentSender<Self>) {
@@ -41,6 +41,7 @@ impl App {
4141
self.push_schema_words();
4242
self.refresh_window_title();
4343
self.set_status_page(
44+
StatusKind::Info,
4445
&crate::tr!("Select a table"),
4546
&crate::tr!("Connected to {driver}. Pick a table from the left to load up to 100,000 rows.")
4647
.replace("{driver}", &driver_id),

linux/crates/app/src/ui/app/editor_tabs.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,12 +8,13 @@ use crate::services::database_service;
88
use crate::ui::editor::{SqlEditorInput, derive_tab_label};
99
use crate::ui::history_dialog::{HistoryDialog, HistoryDialogInit, HistoryDialogOutput};
1010

11-
use super::{App, AppMsg, create_editor_tab_slot, default_tab_label, read_tab_id};
11+
use super::{App, AppMsg, StatusKind, create_editor_tab_slot, default_tab_label, read_tab_id};
1212

1313
impl App {
1414
pub(super) fn on_open_editor(&mut self, sender: ComponentSender<Self>) {
1515
if database_service::instance().active().is_none() {
1616
self.set_status_page(
17+
StatusKind::Disconnected,
1718
&crate::tr!("No connection"),
1819
&crate::tr!("Connect to a database first to run SQL."),
1920
);
@@ -191,6 +192,7 @@ impl App {
191192
pub(super) fn show_welcome_or_grid_after_editor_close(&mut self, sender: ComponentSender<Self>) {
192193
if self.connected {
193194
self.set_status_page(
195+
StatusKind::Info,
194196
&crate::tr!("Select a table"),
195197
&crate::tr!("Pick a table from the left to load rows."),
196198
);

linux/crates/app/src/ui/app/mod.rs

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -187,6 +187,46 @@ pub struct UndoBatch {
187187
pub statements: Vec<(String, Vec<Value>)>,
188188
}
189189

190+
/// Determines which icon and styling adw::StatusPage uses.
191+
///
192+
/// Replaces the previous title-string sniffing in `set_status_page`,
193+
/// which broke the moment a translation used different vocabulary
194+
/// for "Failed" / "Error" / "No connection".
195+
#[derive(Debug, Clone, Copy)]
196+
pub(super) enum StatusKind {
197+
Info,
198+
Error,
199+
Disconnected,
200+
}
201+
202+
impl StatusKind {
203+
fn icon(self) -> &'static str {
204+
match self {
205+
StatusKind::Info => "view-grid-symbolic",
206+
StatusKind::Error => "dialog-error-symbolic",
207+
StatusKind::Disconnected => "network-server-symbolic",
208+
}
209+
}
210+
}
211+
212+
impl App {
213+
/// The active driver id, or "postgres" if no connection is active.
214+
///
215+
/// Single fallback site (was duplicated at 7 call sites). The
216+
/// tracing::warn! makes the latent bug visible if anything ever
217+
/// asks for the driver id without an active connection — today
218+
/// that would silently corrupt SQL quoting on non-Postgres drivers.
219+
pub(super) fn driver_id(&self) -> &str {
220+
match self.current_driver_id.as_deref() {
221+
Some(id) => id,
222+
None => {
223+
tracing::warn!("driver_id called without active connection; falling back to postgres");
224+
"postgres"
225+
}
226+
}
227+
}
228+
}
229+
190230
#[relm4::component(pub)]
191231
impl SimpleComponent for App {
192232
type Init = Arc<DriverRegistry>;

linux/crates/app/src/ui/app/row_ops.rs

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ impl App {
3131
if self.current_columns.is_empty() {
3232
return;
3333
}
34-
let driver_id = self.current_driver_id.clone().unwrap_or_else(|| "postgres".to_string());
34+
let driver_id = self.driver_id().to_string();
3535
let dialog = InsertDialog::builder()
3636
.launch(InsertDialogInit {
3737
table: table.clone(),
@@ -74,7 +74,7 @@ impl App {
7474
return;
7575
}
7676
let row = result.rows[position].clone();
77-
let driver_id = self.current_driver_id.clone().unwrap_or_else(|| "postgres".to_string());
77+
let driver_id = self.driver_id().to_string();
7878
let dialog = EditDialog::builder()
7979
.launch(EditDialogInit {
8080
table,
@@ -123,7 +123,7 @@ impl App {
123123
);
124124
return;
125125
}
126-
let driver_id = self.current_driver_id.clone().unwrap_or_else(|| "postgres".to_string());
126+
let driver_id = self.driver_id().to_string();
127127

128128
let preview = if positions.len() == 1 {
129129
let row = &result.rows[positions[0] as usize];
@@ -383,7 +383,7 @@ impl App {
383383
);
384384
return;
385385
}
386-
let driver_id = self.current_driver_id.clone().unwrap_or_else(|| "postgres".to_string());
386+
let driver_id = self.driver_id().to_string();
387387
let preview = result
388388
.rows
389389
.get(row_position as usize)
@@ -413,7 +413,7 @@ impl App {
413413
let Some(row) = result.rows.get(row_position as usize) else {
414414
return;
415415
};
416-
let driver_id = self.current_driver_id.clone().unwrap_or_else(|| "postgres".to_string());
416+
let driver_id = self.driver_id().to_string();
417417
let cols: Vec<String> = self
418418
.current_columns
419419
.iter()

linux/crates/app/src/ui/app/status_pages.rs

Lines changed: 3 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
use relm4::adw::prelude::*;
22
use relm4::{ComponentController, ComponentSender, adw, gtk};
33

4-
use super::{App, AppMsg, UndoBatch, build_shortcuts_window};
4+
use super::{App, AppMsg, StatusKind, UndoBatch, build_shortcuts_window};
55

66
impl App {
77
pub(super) fn show_welcome_page(&self, _sender: ComponentSender<Self>) {
@@ -39,18 +39,11 @@ impl App {
3939
self.content_holder.set_content(Some(&outer));
4040
}
4141

42-
pub(super) fn set_status_page(&self, title: &str, description: &str) {
43-
let icon = if title.eq_ignore_ascii_case("failed") || title.to_lowercase().contains("error") {
44-
"dialog-error-symbolic"
45-
} else if title.contains("No connection") {
46-
"network-server-symbolic"
47-
} else {
48-
"view-grid-symbolic"
49-
};
42+
pub(super) fn set_status_page(&self, kind: StatusKind, title: &str, description: &str) {
5043
let page = adw::StatusPage::builder()
5144
.title(title)
5245
.description(description)
53-
.icon_name(icon)
46+
.icon_name(kind.icon())
5447
.build();
5548
self.content_holder.set_content(Some(&page));
5649
}

0 commit comments

Comments
 (0)