Repository navigation
Conversation
Two user-visible additions on top of the inline Save/Discard flow:
- Grid factory now plumbs `TabGridContext { tab_id, pk_col_indices }`
through `build_column_view` → `build_column` → `connect_bind`. At
bind time the cell extracts the row's PK values, builds a `RowKey`,
and queries the per-tab change tracker for the corresponding cell
/ row state. Applies one of:
- tp-cell-modified (yellow tint) for edited cells
- tp-row-pending-delete (red strikethrough) for marked-delete
- tp-row-pending-insert (green tint) for draft rows
Stale classes are removed on every bind so widget recycling never
shows a misclassified state.
- BrowseTab subscriber: when the tracker emits PendingCountChanged,
the FilterListModel fires items_changed(0, n, n) which forces all
visible cells to re-bind and pick up the new CSS class. Cheap
because GTK only re-binds the visible viewport.
- Editor + read-only callers pass `TabGridContext::default()`
(tab_id = None) so they don't trigger any tracker lookup.
- Window-level Ctrl+S triggers AppMsg::SaveActiveBrowseTab → dispatches
CommitSave to the active Browse tab. No-op on Editor tabs / when
no tab is active. Listed in the shortcuts window under General.
Three more pieces of the inline-spreadsheet UX: - Window-level Ctrl+Z / Ctrl+Y / Ctrl+Shift+Z bindings dispatch to the active Browse tab's tracker undo/redo. The tracker pops an UndoOp, applies the inverse operation, and emits ChangedRows / PendingCountChanged events that the BrowseTab subscriber re-renders via items_changed. Undo of a CellEdit reverts the pending value; undo of an Insert pulls the draft from the buffer; undo of a Delete unmarks. No-op for Editor tabs / when no tab is active. Listed in the shortcuts window. - Toolbar Delete button no longer shows the confirm-then-DELETE dialog. It marks every selected row for pending deletion via tracker.track_delete (red strikethrough via the tp-row-pending- delete CSS class). User commits the batch by clicking Save or reverts via Discard / Ctrl+Z. Tables without a primary key still refuse the operation with the existing alert. The previous immediate-DELETE flow stayed in App for now (still reachable via the older confirm path) — will be removed when the Insert/Edit dialogs are dropped. - Editable cells holding Value::Null now render `<NULL>` in italic dim text via the tp-null-sentinel class (defined in the CSS provider). Visually distinguishes a true NULL from an empty-string cell at a glance — matters when editing a nullable column where '' and NULL mean different things at the server. Read-only cells unchanged; their existing dim "NULL" rendering already conveys it.
Closing a Browse tab whose tracker holds pending edits now intercepts the AdwTabView close-page protocol with an AdwAlertDialog asking the user to Cancel or Discard. Cancel reverts the tab's "closing" state (close_page_finish(page, false)) so the tab stays open. Discard clears the tracker and re-fires WorkspaceTabClosed, which then completes the close cleanly because has_pending() now returns false. No "Save and close" branch — the user can cancel the close, hit Ctrl+S, and retry. This keeps the close path synchronous and predictable; an async Save-then-close branch would either swallow a mid-close error (silent data loss risk) or leave the user staring at a half-closed tab with a modal error. Refactored close_workspace_tab_by_id: the body that actually tears down the slot moved into `finish_close_workspace_tab`. The public entry point is now a thin wrapper that routes through the dialog when a Browse tab has pending edits and otherwise calls the helper directly.
Closing the window with any open tab holding pending changeset edits now intercepts with an AdwAlertDialog (Cancel / Discard). Cancel keeps the window open. Discard clears every tab's tracker, then re-fires close_request — the guard sees a Cell-flag set by the dialog handler, falls through, persists window state, returns Proceed. The Save-and-close branch is still skipped: an async commit during close window has bad failure modes (mid-close error → modal alert on a half-closed window, or silent-drop a save error). Users can Ctrl+S first, then close. Pairs with the per-tab close guard (eae9de9): the per-tab dialog fires when X-ing a single tab, the app-level dialog fires when closing the whole window. Together they prevent silent data loss in every close path.
Insert button now adds a green-tinted draft row at the top of the
grid instead of opening the modal Insert dialog. Cells in the draft
are editable with the same double-click flow; values flow through
the change tracker as draft-cell edits and commit atomically when
Save runs alongside any other pending updates / deletes.
- RowObject extended with `draft_id: Cell<Option<u64>>` + a
`new_draft(id, cells)` constructor + `draft_id()` accessor +
`set_cell(idx, value)` for in-place updates from the editable
cell handler. Persisted rows leave draft_id at None.
- Grid factory's filter passes draft rows through unconditionally
(their NULL cells wouldn't match search text otherwise).
connect_bind detects drafts via `row.draft_id().is_some()` and
applies tp-row-pending-insert (green tint) without consulting the
PK chain.
- BrowseTab `InsertRow` handler:
1. tracker.track_insert(default Null vec) → RowKey::Draft(N)
2. RowObject::new_draft(N, default values) prepended to the
ListStore via the new `list_store()` accessor (walks
selection → FilterListModel → ListStore).
3. Grid auto-renders the new top row via FilterListModel
forward-on-changed.
- `GridCellEdited` handler routes draft-row edits to
`tracker.track_draft_cell_edit(draft_id, col, new)` and updates
the RowObject's cell in place via `set_cell`. Persisted rows
unchanged from the previous tracker.track_cell_edit path.
- The legacy `BrowseTabOutput::OpenInsertDialog` output and its
App-side InsertDialog handlers are still wired but no longer
reachable from this tab — left in place for now and removed in
the follow-up commit that drops the modal dialog files.
The toolbar Edit Selected Row button still opens EditDialog
because the inline cell-edit + Save flow already covers its UX.
That dialog and the now-orphaned Insert dialog will both be
deleted in the follow-up cleanup commit.
…aths The inline-spreadsheet pattern (commits 376e6d2…45bf48c1) replaced every mutation flow that previously went through the InsertDialog, EditDialog, and the immediate-commit row_ops handlers. This commit removes the now-orphaned code: - Deleted `crates/app/src/ui/insert_dialog.rs` (-195 LOC) and `crates/app/src/ui/edit_dialog.rs` (-213 LOC). Insert is now an inline draft row at the top of the grid; Edit is double-click on any cell with the existing EditableLabel + tracker pipeline. - BrowseTab: removed the toolbar Edit Selected Row button and its `EditSelectedRow` input + `OpenEditDialog` output. Also dropped `OpenInsertDialog`, `DeleteSelected`, `CellEdited`, `SetCellNull`, `DeleteRowAt` outputs — all rerouted to the change tracker inside BrowseTab in earlier commits, so the App-side dispatch was already unreachable. - App/AppMsg: removed `ShowInsertDialog`, `ShowEditDialog`, `ConfirmDeleteSelected`, `InsertCommitted`, `EditCommitted`, `RowOperationCommitted`, `ExecuteUndo`, `CellEdited`, `SetCellNull`, `DeleteRowAt` variants and their handlers. `insert_dialog` / `edit_dialog` Controller fields gone. - App/row_ops: removed `on_show_insert_dialog`, `on_insert_committed`, `on_show_edit_dialog`, `on_edit_committed`, `on_confirm_delete_selected`, `on_cell_edited`, `on_set_cell_null`, `on_delete_row_at`, and the `confirm_and_execute_many` helper. Kept `on_execute_browse_transaction` (the Save flow) and `on_copy_row_as_insert` (clipboard helper). - App/mod: removed `UndoBatch` struct, `run_undo_batch`, `execute_then_refetch_for_tab`, `execute_many_then_refetch_for_tab`, `build_insert_for_row`, `value_to_sql_literal`, `preview_pk`. The `format_sql_literal` helper used by the Copy-as-INSERT flow was kept locally in row_ops.rs. - App/status_pages: removed `show_undoable_toast` (10s undo toast). The new tracker-based VecDeque undo is more powerful (50 levels, Ctrl+Z/Ctrl+Y) and survives any number of edits. Net delta: -1163 LOC. The spreadsheet pattern is now the only path for row mutations — there is no fallback to immediate-commit SQL or modal dialogs anywhere in the codebase.
… type coercion
Three bugs in the inline cell-edit pipeline that made the data grid
feel non-native to use:
- Editing a NULL cell showed the literal "<NULL>" italic sentinel
selected in the entry. If the user committed without typing
anything, the tracker captured "<NULL>" as the new value (a
literal string, not NULL). Fix: when entering edit mode the
double-click handler now clears the sentinel so the user types
into an empty entry.
- Empty input on a nullable column committed as `Value::Text("")`,
which the database treats as a non-NULL empty string — different
from NULL. Fix: empty input on nullable columns becomes
`Value::Null` (canonical SQL convention for "user cleared the
cell"). Non-nullable columns keep the empty-string semantics.
- All cell input was committed as `Value::Text(s)` regardless of
the column's data_type. INT / BOOL / DECIMAL columns relied on
the driver to coerce at execute time; if the user typed
something the driver couldn't coerce, the whole transaction
rolled back at Save with a generic error and no per-cell
attribution. Fix: new `parse_input_for_column` parses against
the column's data_type heuristically — Int, Float, Decimal,
Bool — falling back to Text for unknown types and parse failures.
Drivers still see the right native variant for the common cases.
Tracking docs: docs/audit-2026-04-27-datagrid-ux.md (sections A-G plus
follow-ups), docs/audit-2026-04-28-deep.md (sections CR/L/EF/DR).
Driver correctness:
- MySQL: is_generated heuristic now requires non-empty
generation_expression AND explicit "virtual generated" / "stored
generated" keywords (was: contains("generated") which mis-flagged
every column with DEFAULT_GENERATED expression). default_value
filter on empty string for the same trap class.
- SQLite: AUTOINCREMENT detection via sqlite_sequence (canonical)
rather than substring-matching the table DDL. Single-column-PK
gating so composite PKs no longer falsely flag INTEGER members.
default_value strips outer single quotes + unescapes doubled
apostrophes.
- Postgres: switched fetch_columns to pg_catalog (pg_attribute
attgenerated, attidentity, format_type, pg_get_expr); SERIAL
detection retained via nextval(...). default_value strips trailing
::TYPE casts then outer single quotes.
- Editable grid render now gated on ColumnsLoaded so the lossy
QueryResult ColumnInfo (no PK / generated / auto_increment info)
can't briefly classify cells as editable when they shouldn't be.
Widget lifetime + async safety:
- Calendar / SpinButton / JSON popovers stored per-cell in
POPOVER_SLOT; factory.connect_unbind drains and popdowns before
cell recycle (same defect class as the GtkPopoverMenu finalize
warning fixed earlier for context menus).
- Window-close blocks while in-flight transactions resolve via a
"Saving in progress" AdwAlertDialog with timeout polling. Counter
on App tracks inflight saves.
- Persistence write failures now logged via tracing::warn instead
of let _ = atomic_write_json swallowing errors.
- pending_focus_restore guards against clobbering when sort+save
fire in quick succession.
- Error-flash timeout uses a generation counter so concurrent
flashes don't blank each other early.
- InsertRow's deferred focus uses a self-message (FocusInsertedDraft)
so the handler reads fresh column_view state rather than a
potentially-stale capture.
- Sidebar SwitchOrAppend prefers the currently-selected tab when
multiple match; falls back to first match.
Section A-G earlier work (entangled in this commit because
browse_tab.rs / grid.rs / mod.rs span many groups):
- A1: NULL-aware WHERE clauses (IS NULL for null PK components).
- A2: optimistic-concurrency guard via rows_affected check.
- A3: lock generated/auto-increment cells read-only.
- A4 + A5: parse_input_for_column with type coercion + reject empty
on NOT NULL.
- B1-B8: F2/Enter/Tab/Shift+Tab/Delete/Ctrl+N/Ctrl+Shift+N/Space
shortcuts with shortcuts-window updates.
- C1-C6: cell focus ring, bool CheckButton, date GtkCalendar
popover, Insert auto-scroll-and-focus, drafts persist across
reloads, expanded context menu.
- D1-D8: pending banner, Saved toast, dirty • tab title, Pango
strikethrough, leftmost-cell ribbon, read-only banner, no-PK
banner.
- E1-E4: Cancel|Discard|Save dialogs, error-row scroll-and-select,
Save button disabled during commit.
- F1-F2: focus preservation across sort/save reloads.
- G1: targeted items_changed via tracker ChangedRows event.
- C2 follow-ups: numeric SpinButton popover (Int/Float),
JSON SourceView popover (sourceview5).
- E3 follow-up: red-flash CSS animation on failing row.
Verification: cargo fmt, cargo check --workspace, cargo clippy
--workspace --all-targets -- -D warnings, cargo test --workspace
--all-targets — all green. Test counts: app 59, core 21, sqlite 11
(was 5), postgres 7 (was 3), other drivers unchanged.
…n, persist debounce CR3: cache ColumnView per tab, swap model only on RowsLoaded. Within a single tab the column structure never changes (current_columns mutates only on ColumnsLoaded which fires once per (table, connection) open), so sort flips, page changes, save reloads, and F5 refreshes can reuse the existing factory + columns + selection and just refresh the underlying ListStore. Drops the per-page-change cost from O(N×M) factory recreations to O(N) row appends. CR4: cap cell display text at 10k chars in value_to_display_text. A million-character TEXT or JSON value would otherwise hang the GTK main thread inside Pango layout. Full value remains in the model and is unabridged in the JSON popover; only the in-grid preview is capped. Char-boundary safe (4 unit tests). CR6: debounce persist_workspace_state via persist_pending flag and 500ms timeout_add_local_once. Active tabs fire WorkspaceTabsChanged on every selection / drag-reorder / page-size change / state-changed event; coalescing means dozens-per-second writes collapse into a single one. teardown_workspace_tabs uses the synchronous do_persist_workspace_state_now path because the debounced timer would fire after teardown drops state. Verification: cargo fmt, cargo check --workspace, cargo clippy --workspace --all-targets -- -D warnings, cargo test --workspace --all-targets — all green. App tests went from 59 to 63 (4 new truncation tests).
- H1: empty inner-stack status page when a fresh fetch returns zero
rows on offset 0 with no drafts ("No rows yet. Press Ctrl+N to
add one.") instead of blank ColumnView
- H4: clear cached result + paginator + button sensitivity on
ShowError so the next refresh can't render against stale state
- K1: Tab / Shift+Tab traverses cells when not editing (matches
the Shortcuts window claim)
- K2: Left / Right cell navigation, complementing ColumnView's
built-in Up / Down row selection
- K6: Ctrl+C with row(s) selected serializes the selection as TSV
to the clipboard (escapes embedded \\t \\n \\r → space so paste
into spreadsheets preserves row/column structure)
- K7: Ctrl+V on the grid (focus outside any cell editor) shows a
toast — multi-row paste isn't implemented yet
- L4: tooltip on cell labels when displayed text exceeds 40 chars
so ellipsized values are still readable on hover
- Q2: dirty browse tabs raise AdwTabPage's needs-attention flag while not selected, so the tab-bar pulses for background tabs with unsaved edits (auto-cleared on tab focus by Adwaita) - Q5: prefix the OS-level window title with "• " whenever any open browse tab has pending edits — matches GNOME Text Editor's convention so dirty state is visible from Activities / Alt-Tab - Q8: tab-bar context menu (right-click on a tab) now offers "Close Other Tabs" and "Close Tabs to the Right" via AdwTabView.menu-model + a per-target action group; each close goes through the per-tab close path so dirty browse tabs still get the unsaved-changes prompt - M3: bulk-delete confirmation dialog when the user marks 10+ rows pending-delete in one shot; the snapshot of (key, row) pairs is captured pre-dialog so a background reload can't shift positions out from under the confirmed commit - K3: Page Up / Page Down navigate the paginator (Prev / Next page) inside the browse tab - K4: Ctrl+Home / Ctrl+End scroll-and-focus the first / last row of the current page - K5: Ctrl+A selects every visible row (sets up the bulk-delete flow nicely) - J1: cell-edit commit collapses embedded \\n / \\r to spaces and squashes whitespace runs for non-JSON columns, so a multi-line clipboard paste never reaches the SQL layer with literal newlines O1 (banner stacking) intentionally deferred — the fix is a design call (single banner with chevron-to-expand) rather than a one-liner, and the current stacked layout is functional.
GtkEditableLabel's editing-notify fires when focus shifts to an
IME popover during CJK / Korean / Vietnamese composition, which
the commit handler previously treated as the user finishing the
edit. The result was either an empty commit (popover took focus
before any preedit was committed) or a stale snapshot getting
locked in.
Hook into the inner GtkText delegate's preedit-changed signal
and gate the commit:
- PREEDIT_SLOT tracks "composition active" per cell widget
- editing-notify(false) defers when PREEDIT_SLOT is set
- preedit-changed("") + !is_editing() fires the deferred commit
- commit_cell_edit() factored out so both paths share the
diff-snapshot + GridMsg send
The walking is via EditableLabel::delegate() → downcast::<Text>,
which keeps the binding independent of EditableLabel's internal
Stack > Text layout.
- I4: tables with 9+ columns no longer share viewport width fractionally. When a column has no persisted width, seed it with a 120px minimum so cells render readably on first open; the user can still resize and the new value persists. - J5: NULL sentinels routed through tr! — readonly_null_sentinel() for the dim "NULL" rendering and editable_null_sentinel() for the bracketed "<NULL>" form. Translations apply consistently across display, enter-edit-mode clearing, and the focused- draft-cell entry path. - Q7: Shortcuts window's Browse table section now matches the actually-wired bindings — Tab/Shift+Tab cover both edit-mode commit and non-editing traversal, and the new entries (Left/ Right cell nav, Page Up/Down, Ctrl+Home/End, Ctrl+A, Ctrl+C TSV copy, Ctrl+S/Z/Shift+Z) are documented. L1-UX (row empty-space right-click) deferred — needs a GestureClick on the ColumnView with pick() + parent-walk to locate the row, which is more involved than the rest of this batch.
Lay the core SQL-generation foundation for the upcoming Structure workspace tab: - crates/core/src/query.rs: IndexInfo + ForeignKeyInfo structs carrying secondary-index and FK metadata (with primary flag on the implicit PK index so the UI can render it as read-only). - crates/core/src/connection.rs: extend the Connection trait with fetch_indexes() and fetch_foreign_keys() default impls returning empty Vec, so existing drivers compile before they're filled in. - crates/core/src/read_only.rs: forward the two new reads to the inner connection (read-only blocks mutations, not introspection). - crates/core/src/sql_ddl.rs: the full CREATE / DROP / ALTER DDL surface across MySQL + Postgres + SQLite — build_create_table, build_drop_table, build_rename_table, build_add_column, build_drop_column, build_rename_column, build_alter_column, build_reorder_column (MySQL only), build_create_index, build_drop_index, build_add_foreign_key, build_drop_foreign_key. Dialect quirks captured: - Postgres auto-increment renders as SERIAL / BIGSERIAL / SMALLSERIAL based on the user-typed integer width. - MySQL ALTER COLUMN routes through MODIFY COLUMN which replaces the whole definition (single-call); Postgres emits per-attribute ALTER COLUMN ... TYPE / SET NOT NULL / SET DEFAULT. - SQLite limits surface as BuildDdlError::SqliteNotSupported for alter-type / alter-nullable / alter-default / drop-FK so the UI can disable the affordances rather than fall through to a runtime driver error. - SQLite FK constraints get a leading PRAGMA foreign_keys = ON emitted by build_create_table when the columns list contains FKs. 43 unit tests cover every builder × every driver × edge cases (composite PK, schema qualification, identifier quote-doubling, empty-name validation, default expressions, auto-increment routing).
Implement the new Connection trait methods on each driver so the
Structure tab can load existing indexes and FK constraints when
opening a table for editing.
- Postgres: pg_index + pg_constraint joined with pg_class /
pg_attribute. array_agg(... ORDER BY ordinality) preserves the
column order on composite indexes / FKs. confdeltype /
confupdtype single-char codes map through pg_action_char_to_keyword
to canonical SQL action keywords ("RESTRICT", "CASCADE", etc.).
- MySQL: information_schema.statistics for indexes (grouped by
index_name in Rust since sqlx doesn't aggregate ordered column
arrays natively); key_column_usage JOIN referential_constraints
for FKs. "PRIMARY" maps to is_primary; non_unique inverts to unique.
- SQLite: PRAGMA index_list + per-index PRAGMA index_info. Implicit
PK indexes that use the rowid alias don't appear in index_list
so they're synthesised from PRAGMA table_info when no origin='pk'
row exists. PRAGMA foreign_key_list grouped by synthetic id since
SQLite doesn't store user-visible FK names — we render
"fk_{table}_{id}" stably.
- crates/app/src/services/structure_tracker.rs: per-tab DDL changeset
registry. Mirrors change_tracker.rs (thread-local registry keyed
by tab UUID, subscription channel, undo / redo stacks) but at the
schema-operation level. StructureOp covers CreateTable, RenameTable,
AddColumn, DropColumn, RenameColumn, AlterColumn, ReorderColumn
(MySQL only), AddIndex, DropIndex, AddForeignKey, DropForeignKey.
materialize() walks the op log and emits ordered DDL statements
per driver dialect. Statement ordering avoids dependency errors:
rename-table → drop-FK → drop-index → drop-column → rename-column
→ alter-column → add-column → reorder → add-index → add-FK. For
New mode (single CreateTable op) the fast path delegates straight
to build_create_table.
MySQL alter grouping: all AlterColumn ops on the same column
collapse into one MODIFY COLUMN reflecting the latest state, so
multi-attribute changes don't produce three statements that each
re-state the whole definition.
- crates/app/src/services/workspace_state.rs: extend WorkspaceTabRecord
with Structure { schema, table } variant for Edit-mode persistence.
New mode is intentionally not persisted (drafts for non-existent
tables shouldn't survive disconnect). Added #[serde(other)] Unknown
variant for forward-compat: an older binary reading newer state
silently drops unrecognised tab kinds rather than failing the
entire workspace load. clamp_connection retains-out Unknown
variants up front so they don't contribute to count or active_idx.
- crates/app/src/ui/app/workspace_tabs.rs: extend restore_workspace_tabs
with placeholder Structure / Unknown match arms (no-op for now;
the actual append_structure_tab wiring lands in the next milestone).
8 unit tests cover push/undo round-trips, clear, materialize ordering
across all op types, MySQL alter-grouping, Postgres per-attribute
emission, and registry tab-isolation.
Wire the App layer for the Structure (DDL) workspace tab. The tab
itself is a stub that renders an AdwStatusPage placeholder; the
full ViewSwitcher UI lands in the next commit. What this commit
delivers is the entire routing + lifecycle plumbing so the next
commit only has to fill in the widget tree.
- crates/app/src/ui/structure_tab.rs (NEW): minimal SimpleComponent.
ToolbarView root + inner Stack ("loading" / "ready" / "error") +
bottom action bar with Discard / Save / Drop Table buttons. The
tracker subscription, dirty-state emission, and Save / Discard
paths are fully wired against StructureChangeTracker.materialize
so the next commit's UI inherits a working transaction flow.
- crates/app/src/ui/app/mod.rs: add StructureTabSlot, the
WorkspaceTab::Structure variant, and 18 new AppMsg variants
covering NewTableTab / EditStructureTab / DropTablePrompt /
DropTableConfirmed / ExecuteStructureTransaction / Save{Completed,
Failed} / FetchStructureData / Structure{Columns,Indexes,FKs}Loaded
/ StructureLoadFailed / StructureTabDirtyChanged / SchemaChanged /
TablesReloaded / Undo + RedoActiveStructureTab.
- crates/app/src/ui/app/structure.rs (NEW, ~410 LOC): all the on_*
handlers. on_drop_table_prompt presents an AdwAlertDialog;
on_drop_table_confirmed builds DROP TABLE via sql_ddl, runs it
asynchronously, and triggers SchemaChanged for cross-tab cleanup.
on_execute_structure_transaction wraps Postgres in BEGIN/COMMIT
for atomicity (PG DDL is transactional) and runs MySQL/SQLite
per-statement (their DDL implicitly commits). on_schema_changed
refetches list_tables + repopulates the sidebar via
TablesReloaded → repopulate_sidebar. close_tabs_for_table walks
the tab map and force-closes any Browse/Structure tabs pointing
at the dropped table without going through the unsaved-changes
dialog (the underlying table is gone).
- crates/app/src/ui/app/workspace_tabs.rs: append_structure_tab
mirrors append_browse_tab_inner. Match arms extended in:
close_workspace_tab_by_id (tracker pending check),
finish_close_workspace_tab (tracker close + page lookup),
do_persist_workspace_state (Edit-mode tabs persist as
WorkspaceTabRecord::Structure; New mode skipped via continue),
restore_workspace_tabs (Structure record restored as Edit mode),
teardown_workspace_tabs (close all structure trackers on
disconnect).
- crates/app/src/services/workspace_state.rs: WorkspaceTabRecord
gains Structure { schema, table } and #[serde(other)] Unknown
variants so old binaries reading newer state don't fail.
What still needs wiring up in subsequent commits:
- Sidebar context menu entries ("Edit Structure" / "Drop Table…")
and the schema-header "+" button for "New Table…" — they emit
AppMsg::NewTableTab / EditStructureTab / DropTablePrompt which
this commit already handles.
- The full Columns / Indexes / Foreign Keys / SQL Preview
ViewSwitcher inside structure_tab.rs — replaces the placeholder
StatusPage.
Wire the sidebar entry points for the DDL UI. The App layer plumbing
landed in the previous commit; this commit fires it from the user's
two natural sidebar interactions.
- Schema header "+" button: the existing set_header_func was a plain
GtkLabel for each schema heading; replace with a horizontal Box
containing the same label plus a flat list-add-symbolic GtkButton
on the right. Click emits AppMsg::NewTableTab carrying the schema
for that header. Tooltip reads "New Table in {schema}…" when the
schema is non-empty, otherwise "New Table…". Single-schema
connections (MySQL / SQLite) get one header anchored to the top
of the sidebar so the affordance is always discoverable.
- Per-table context menu (right-click on a sidebar row): extend
the existing PopoverMenu from one section to three —
open → "Open in new tab"
structure → "Edit Structure" (NEW)
mutate → "Drop Table…" (NEW)
Two new SidebarRowMsg variants (EditStructure, DropTable) and
matching SidebarRowOutput entries; App's sidebar forwarder maps
them to AppMsg::EditStructureTab / AppMsg::DropTablePrompt.
EditStructure activates an existing Edit-mode Structure tab if
one matches (schema, table); otherwise appends a new one.
DropTablePrompt presents the AdwAlertDialog confirmation already
wired in commit #4.
Net effect: end-to-end DDL flow is now reachable from the UI even
though the actual editor inside the Structure tab is still the
placeholder StatusPage. The next commit replaces that with the
full Columns / Indexes / Foreign Keys / SQL Preview ViewSwitcher.
Replace the placeholder StatusPage with the working ViewSwitcher
editor. The Structure tab now provides the full DDL editing surface
the audit asked for.
UI layout (adw::ToolbarView):
- Top: AdwHeaderBar with table-name Entry + driver badge.
- Content: AdwViewSwitcher + AdwViewStack with four pages:
Columns — one editable row per column: Name (Entry),
Type (ComboBoxText with curated per-driver
suggestions + free-text), Nullable / PK / Auto
(CheckButtons), Default (Entry), Remove (trash
icon). Add Column button trailing the rows.
Indexes — read-only ColumnView-like rows with Name /
Cols / UNIQUE/PRIMARY badge / Remove. Primary
index is non-removable (it's owned by the PK
column constraint).
Foreign Keys — read-only rows with Name / Cols / "→ ref"
pointer / Remove. Add Foreign Key button opens
an AdwAlertDialog form for name + col multiselect
+ ref table + ref cols + ON DELETE / UPDATE
drop-downs.
SQL Preview — sourceview5::View read-only with sql syntax
highlighting. Regenerates on every tracker
OpsChanged event by calling materialize().
- Bottom: existing ActionBar with Discard / Save / Drop Table.
Editing flow:
- ColumnEdited / TableNameEdited / Add* / Remove* events mutate
the in-memory model (Rc<RefCell<Vec<…>>>) and then push the
matching StructureOp + inverse onto the per-tab tracker.
- New mode collapses everything into a single CreateTable op which
is rebuilt on every mutation; Edit mode pushes per-attribute
AlterColumn / AddColumn / DropColumn / AddIndex / DropIndex /
AddForeignKey / DropForeignKey / RenameTable / RenameColumn ops.
materialize() in StructureChangeTracker handles dialect quirks
(MySQL alter-grouping, statement ordering).
- StructureTabInput::Refresh tears down the three list views and
rebuilds them after every mutation. The lists are short enough
(a handful of columns / indexes / FKs) that full rebuild is
cheaper than per-row diffing.
- Save runs validate_save() first: rejects empty table name on New
mode, missing column types, duplicate column names, nullable PK
columns. Validation errors surface as toasts; structural errors
from materialize() (e.g. SqliteNotSupported) surface as alerts.
- Discard restores the original DraftColumns from
ColumnInfo.original (Edit mode) or empties the model (New mode).
SQLite limit handling: rows with original.is_some() get
sensitive=false + tooltip on the Type / Nullable / Default cells
when the driver is sqlite. The trash icon stays enabled (DROP COLUMN
landed in 3.35; older SQLite errors at execute time). Drop Foreign
Key buttons are disabled across the board for SQLite.
What's still deferred to follow-ups:
- MySQL drag-reorder of columns (driver_supports_reorder helper is
in place; the gtk::DragSource + DropTarget wiring is non-trivial
and benefits from manual GUI verification).
- update_added_columns_ops surgical replace — for now multiple
AddColumn ops on the same column may stack; materialize-time
ordering keeps the final SQL correct, but dirty-count is slightly
inflated.
- Live drop-down of available tables in the Add Foreign Key dialog
(today the user types "schema.table" free-text into the entry).
136 tests still green, clippy clean, build green.
Six critical correctness issues from the post-DDL deep audit (docs/audit-2026-04-29-app-deep.md): C-1: DROP TABLE no longer closes tabs synchronously before the async DROP completes. on_drop_table_confirmed dispatches an Ok arm to a new DropTableSucceeded variant that closes matching tabs + refreshes sidebar atomically. If DROP fails, the user's tab state is preserved. C-2: update_added_columns_ops is now a working surgical replace of pending AddColumn ops. Added retain_ops on StructureChangeTracker so the model can drop stale ops without losing alignment between the ops vec and the undo / redo deques. New columns added in Edit mode now Save with the user's actual edits, not the placeholder column_N name. C-3: window-close request now ORs change_tracker pending against structure_tracker pending. A user closing the window with dirty DDL no longer silently loses work. C-4: close-with-pending dialog "Save" now dispatches SaveActiveStructureTabById for Structure tabs (was always firing SaveActiveBrowseTabById regardless of tab kind, so Structure tabs silently did nothing on close-Save). Added the new AppMsg variant + matching App handler that materialises the tracker and routes to ExecuteStructureTransaction. C-5: undo / redo properly track inverse ops. StructureChangeTracker now stores (forward, inverse) pairs in the undo / redo deques so redo can re-establish the undo entry. Previously redo pushed only the original op, so the next undo would pop a stale inverse from an earlier step and apply the wrong reversal. C-6: StructureLoaded now arrives as a single coalesced message (StructureDataLoaded) instead of three fan-out messages each with two empty Vecs. The Structure tab rebuilds its column / index / FK list views once, not three times. Removes a visible flicker on Edit-mode tab open and simplifies the merge-vs-overwrite logic. Other small adjacent fixes: - StructureChangeTracker::clear no longer fires both Cleared and OpsChanged(0) — Cleared is sufficient and the duplicate event was triggering double tab-title updates on Discard. Tests: 136 still green. Clippy clean. Build green.
Items from docs/audit-2026-04-29-app-deep.md: H-4: Drop Table button moved from pack_end (next to Save) to pack_start. Spatial separation between destructive and primary actions matches HIG and reduces misclick risk. H-5: destructive-action CSS class added to all three trash-icon remove buttons in structure_tab.rs (column / index / FK rows), the connection-row delete button, and the history-dialog inline delete. Carries the standard red tint and matches user expectation. H-8: install_popover_close_cleanup and the context-menu close handler in grid.rs no longer capture clones of the popover into their own connect_closed callbacks. The closure now uses the callback's first parameter (`|p| p.unparent()`), eliminating the self-reference cycle that delayed popover finalisation past popdown. H-9: Narrowed the file-scoped `#![allow(deprecated)]` in structure_tab.rs to two scoped helpers (build_type_combo, combo_entry) that wrap the only ComboBoxText API surface. Future deprecations elsewhere in the file now surface their own warnings again rather than being silently absorbed by the blanket allow. H-10: Removed `let _ = glib::Type::INVALID;` debug-leftover from StructureTab::update; freed the `glib` import that was only used by the dead line. H-11: build_rename_table no longer has a dead MySQL early return producing identical SQL to the fallthrough. Single emit path. H-12: build_alter_column on Postgres now returns BuildDdlError:: NoChange when the post-edit DraftColumn matches its original on all three attributes, rather than emitting a no-op ALTER COLUMN ... TYPE that risked an unnecessary table rewrite. StructureChangeTracker::materialize handles NoChange specially (skip-and-continue) so a single no-op alter doesn't fail the entire Save transaction. H-13: FK dialog now validates ref_columns is non-empty (parallel to the existing cols.is_empty() check), rejecting the broken state at dialog confirm rather than at Save time.
Smaller polish items from docs/audit-2026-04-29-app-deep.md: M-3: SQL Preview ViewSwitcher page icon swapped from accessories-text-editor-symbolic (an application icon) to text-x-generic-symbolic (a content-category icon). Matches the HIG convention that ViewSwitcher icons represent the page's content type, not an unrelated app. M-10: validate_save no longer takes a (then-unused) driver_id parameter. Switched the duplicate-name detector from BTreeSet to HashSet — uniqueness check doesn't need ordered traversal. M-11: qualified_table now trims leading / trailing whitespace from schema and table names before quoting them. A user who typed `" users "` no longer ends up with a literally space- padded identifier surviving into the generated DDL. The validators still reject after-trim-empty separately. M-14: "Add Column" placeholder name now uses a monotonic counter instead of `vec.len() + 1`. Adding then removing then re-adding columns no longer produces duplicate `column_1` entries (which validate_save would reject at Save time, but the confusing UX is worse than a fresh `column_2` slot).
… UI batch 4) H-7: SidebarRow popover is now built lazily inside the right-click / Menu-key gesture handlers. Previously set_parent ran once at row construction and the popover lived for the row's whole lifetime — factory.guard().clear() during reload could finalise the row while the menu was visible, producing "GtkPopoverMenu was destroyed while visible" warnings. The new present_row_menu helper builds, parents, positions, popups, and registers a connect_closed→unparent in one shot per gesture so the popover always cleans up after itself. H-2: Structure tab columns / indexes / FKs sections now wrap their rows in a gtk::ListBox with the `.boxed-list` HIG class. Each row is a non-activatable, non-selectable gtk::ListBoxRow containing the existing horizontal content box. Visually this picks up the standard Adwaita rounded-corner + per-row separator + hover treatment used in GNOME Settings / Files. Helpers: boxed_list(), wrap_in_list_row(), wrap_button_in_row(). H-6: AdwShortcutsWindow gained a "Table structure" group with Ctrl+S / Ctrl+Z / Ctrl+Shift+Z entries so the Structure tab's keyboard surface is documented. (The window itself was already registered — the audit's "never registered" finding was wrong; gtk::ShortcutsWindow has been built and presented since the M2 keyboard work.) H-1 closed as N/A: libadwaita 0.9.1 doesn't provide adw::SearchBar. GNOME Files / Builder / Text Editor all use gtk::SearchBar inside their AdwToolbarView, which is exactly what this codebase does.
Eight smaller items from docs/audit-2026-04-29-app-deep.md:
M-1: Off-grid margins snapped to 6/12/18 on the 6px GNOME grid:
- structure_tab column / index / FK row vertical padding 2 → 6
(three places, matches the rest of the boxed-list rows)
- history_dialog pinned + all-queries heading bottom 4 → 6
- sidebar schema-header top 8 / bottom 4 → 12 / 6 (anchors the
schema label and the "+" button on grid).
M-5: WelcomeView's hard `set_size_request(560, -1)` replaced with
ScrolledWindow `max_content_width(560) + propagate_natural_width`.
Narrow viewports now collapse the welcome content gracefully
instead of pushing past the window frame.
M-12: on_schema_changed actually uses the (schema, table) params.
Iterates open Browse tabs and dispatches FetchBrowseColumns +
FetchBrowsePage + FetchBrowseRowCount for every match. Previously
the params were silently dropped — Browse tabs pointing at a
table that just got altered showed stale columns until a manual
F5. Now they refresh automatically alongside the sidebar.
L-1: editor_tab_tooltip switched from `q.chars().count()` (O(n) on
the full query) to a `char_indices().take(200)` byte-prefix walk.
For multi-megabyte SQL dumps we no longer scan past the 200-char
preview boundary just to compute the truncate-vs-don't decision.
L-5: SQL Preview SourceView in the Structure tab now connects to
AdwStyleManager::dark-notify and applies "Adwaita" / "Adwaita-dark"
scheme dynamically — mirroring editor.rs. Previously the scheme
was frozen at the boot theme even after the user toggled system
dark mode.
Skipped as N/A:
- M-2 (sidebar row double-padding) — `.navigation-sidebar` row
styling is theme-dependent; the inner 6px isn't necessarily
doubled.
- M-4 (status page icon convention) — Browse loading uses spinner
child, Structure loading uses synchronizing icon. Different
affordances for different states; intentional.
- M-15 (Test button placement) — current placement matches GNOME
Connections per existing inline comment.
- L-6 (keyboard menu set_pointing_to) — already correct via the
present_row_menu(None) path landed in the lazy-popover commit.
Root cause: GTK4's PopoverMenu resolves action names against its own attached action group + walks up via gtk_widget_get_native(), which short-circuits at the popover's surface. It does NOT continue up into widgets the popover was set_parent'd onto. The lazy-popover refactor (commit daa43bf, "H-7") moved the popover construction into present_row_menu but kept the action group on the ListBoxRow at init time. Action lookup chain: popover_menu_item → popover surface → STOP (no group found) The action was never found, so every menu click silently no-op'd. The menu opened correctly because that's done via menu_model on the popover itself; only action invocation broke. Fix: build + insert the action group on the popover directly inside present_row_menu. Each lazy popover owns its own actions, captured from the row's FactorySender. Matches the working pattern for the in-grid cell context menu in grid.rs (which inserts its action group on the cell widget that's also the popover anchor — there the surface short-circuit lands on the same widget that has the group, so it works coincidentally). Added tracing::trace! at three points so future regressions surface under RUST_LOG=tablepro_app::sidebar_row=trace: - "init_widgets: wired right-click context menu" - "right-click pressed; opening menu" - "action: open-in-new-tab" / "edit-structure" / "drop-table"
The lazy-popover fix (151164b) didn't actually resolve the broken right-click action invocation. Add info!-level logs at every hop in the chain so the user can run the app, reproduce, and pin down exactly which step drops: [1] right-click pressed → about to call present_row_menu [1.5] popover constructed; group.list_actions() lists the entries (sanity check that the group itself is fully populated) [1.6] popover.popup() called [2] action callback fired (open-in-new-tab / edit-structure / drop-table) [3] SidebarRow::update received message [4] SidebarRow output dispatched (sent_ok flag) [5] App forwarder mapped output to AppMsg [6] App::update entered handler (SelectTable / EditStructureTab / DropTablePrompt) Logs are info!-level (visible without RUST_LOG override) and tagged target = "tp_sidebar_menu" so they're trivial to filter: RUST_LOG=tp_sidebar_menu=info ./tablepro-app Reading the log: - If [1] fires but [1.5] doesn't list any actions → the group is empty (e.g. a typo in action names), GTK can't find them. - If [1.6] fires but [2] never does → menu shows but click doesn't reach the action. Action lookup failure (popover surface short-circuit, namespace mismatch, etc.). - If [2] fires but [3] doesn't → FactorySender disconnected, input channel dead. - If [3]/[4] fire but [5] doesn't → forwarder closure not called (dropped by relm4). - If [5] fires but [6] doesn't → AppMsg lost between forward and dispatch.
Logging confirmed the lazy-popover path (commits daa43bf / 151164b) broke action invocation despite the popover constructing correctly: [1] right-click pressed [1.5] popover constructed; group.list_actions() = ["edit-structure", "open-in-new-tab", "drop-table"] [1.6] popover.popup() called popover closed; unparenting -- [2] action callback NEVER fires -- Three lookup permutations all failed under libadwaita 1.5 + gtk-rs 0.11: - insert_action_group on the row, set_parent on the row at popup - insert_action_group on the popover, set_parent on the row at popup - insert_action_group on both row and popover, set_parent at popup GtkPopoverMenu's internal action muxer is wired at parent-attachment time. Calling set_parent dynamically inside a gesture handler — even with the action group already registered — does not cause the muxer to register either group. The menu opens (menu_model is read directly) but action lookup short-circuits. The H-7 audit finding that triggered the lazy refactor was concerned about a "PopoverMenu destroyed while visible" warning when the factory clears rows. That's a narrow race, defended against here by connect_unmap on the row → popover.popdown() before the row goes away. A rare GTK warning is far cheaper than a fully broken context menu. Reverted to the original eager pattern: build menu + popover + action group at init_widgets, set_parent(&root) once, popup() in the gesture handler. This is what worked in the M2 timeframe and matches how GNOME Files / Builder wire their sidebar context menus. Tracing logs kept at info! level — under target "tp_sidebar_menu" — so any future regression surfaces immediately under default logging. Will demote to trace! once the GUI verification passes.
Action invocation works after the eager-set_parent revert (51513ed). Demote the [1]…[6] info!-level breadcrumbs to a single trace! per arm under target "tablepro_app::sidebar_row" so default-level output stays clean. Symmetric cleanup on the App-side forwarder and AppMsg dispatch arms. Audit doc updated: H-7 marked as reverted with the lesson — GtkPopoverMenu's internal GtkActionMuxer snapshots the parent's action-group observation chain at set_parent() time. The lazy pattern works for gtk::Popover (custom child) but NOT for gtk::PopoverMenu (model-driven). For the model-driven flavour the popover must be parented eagerly so the muxer's snapshot includes the row's action group.
#3470) Signed-off-by: Dat Ngo Quoc <datlechin@gmail.com>
Open
Open
Open
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
The Linux app, GTK 4 and libadwaita in Rust, was built on the
linuxbranch. This brings it into main, which becomes its home: CI, packages andlinux-v*releases run from here.Change
linux/: the app, a Meson and Cargo workspace with the PostgreSQL, MySQL and MariaDB, SQL Server, ClickHouse and SQLite drivers, Flatpak, Debian and RPM packaging, its guide, docs and towncrier fragments.build-linux.ymlgates on oneLinuxjob;linux-packages.ymlandrelease-linux.ymlbuild packages and publishlinux-v*tags from main.linux/: the fix-issue Linux platform, alinuxsuite indetect-changed-paths.sh,check-changelog.pyfor both towncrier projects, banned words skipping generated and vendored files, and the root guides pointing atlinux/.Verified
A local merge into main has no conflicts and passes banned words,
check-changelog.py --prand the title check. This PR's CI runs the full Linux suite.Not in this PR
linux/:git diff origin/main...origin/linux -- . ':!linux'.Linuxon main, addrefs/tags/linux-v*to both tag rulesets, delete thelinuxbranch.Refs #311