From 44e9638b08a0a9e6ba416bc824153ef866c0dad5 Mon Sep 17 00:00:00 2001 From: NicolaasGrobler Date: Tue, 9 Jun 2026 12:22:01 +0800 Subject: [PATCH] fix(editor): run-status checkmarks land on the right block, accumulate, and follow edits MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The per-statement gutter glyphs (green check / red cross) were placed from line numbers frozen at run time and rebuilt on every change. That broke three ways: - A multi-statement *selection* was re-split and its statements numbered relative to the selection (1, 2, 3…) instead of the document, so glyphs landed on the wrong block. - statementResults was reset to [] every run, so running one block wiped the others — never one mark per block. - Glyphs were re-pinned to the frozen line on every rebuild, fighting Monaco's sticky tracking, so they didn't follow edits and didn't clear on cut. Fix: - New pure helpers mapSelectionStatementsToDocumentLines + mergeGlyphResults (src/utils/statementGlyphs.ts, unit-tested). - MainContent: selection statements map back to document-absolute lines; glyphs accumulate (merge + dedup by line) instead of being wiped each run. - QueryEditor: glyphs are id-keyed sticky Monaco decorations — reconcileGlyphs only creates new / removes gone glyphs (survivors keep their live positions), and a throttled pruneGlyphs on content change drops glyphs whose anchor line was edited/cut and keeps survivors' line numbers fresh in tab state. Verified with tsc --noEmit (clean) and npm test (181 passed, incl. 7 new). Fixes #223 Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 1 + src/components/editor/QueryEditor.tsx | 279 +++++++++++++++++++------- src/components/layout/MainContent.tsx | 99 ++++++--- src/utils/statementGlyphs.test.ts | 65 ++++++ src/utils/statementGlyphs.ts | 60 ++++++ 5 files changed, 400 insertions(+), 104 deletions(-) create mode 100644 src/utils/statementGlyphs.test.ts create mode 100644 src/utils/statementGlyphs.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index d000d18..8c0f46b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ All notable changes to QueryDen are documented here. This project adheres to [Se - **[#15](https://github.com/openidle-dev/queryden/issues/15) — Argon2id parameters are now explicitly locked.** Replaced `Argon2::default()` with an explicit `Params::new(19456, 2, 1, None)` — 19 MiB memory cost, 2 iterations, 1 parallel thread, using Argon2id v1.3. These match the actual defaults of the argon2 0.5 crate and prevent silent parameter drift across crate version bumps. ### Fixed +- **[#223](https://github.com/openidle-dev/queryden/issues/223) — Editor run-status checkmarks now land on the right block, accumulate, and follow edits.** The per-statement gutter glyphs (green ✓ / red ✗) were placed from line numbers frozen at run time and rebuilt on every change, so they landed on the wrong block when a multi-statement selection was run (line numbers were relative to the selection, not the document), were wiped on every run instead of keeping one mark per executed block, and went stale the moment the text was edited. Glyphs are now sticky Monaco decorations keyed by a stable id: they accumulate (re-running a block updates just that block's mark), sit on the correct document line, follow in-place edits, and clear when their block is edited or cut away. - **[#217](https://github.com/openidle-dev/queryden/issues/217) — Running a query now gives instant feedback and can't be triggered twice.** Pressing `Ctrl+Enter` previously showed no "running" state until *after* the database connection handshake (`Database.load`), which can take seconds on a cold connection — so the editor looked idle and a second press would run the query again. `setIsExecuting(true)` now fires synchronously at the start of execution (the spinner and tab status glyph appear the instant you press the shortcut), and a re-entrancy guard drops a duplicate trigger while a run is already in flight. - **You can now close the last remaining query tab.** The tab close button was hidden whenever only one tab was open, leaving no way to close the final editor. The button is always available now; closing the last tab falls back to the empty-state launcher. - **PSQL Console error messages no longer flash and disappear.** Errors in the CLI execution path (PostgreSQL version unknown, download cancelled, download failed, psql not found, `\watch` errors) are now committed as persistent `psqlConsoleEntry` entries before the live-output grace period expires, preventing the output from going blank after 300ms. The `\watch` loop-end path also commits accumulated output. Fixes Windows WebView2 race where `showLiveGrace` expired without entries. diff --git a/src/components/editor/QueryEditor.tsx b/src/components/editor/QueryEditor.tsx index f4f965d..22dc6fd 100644 --- a/src/components/editor/QueryEditor.tsx +++ b/src/components/editor/QueryEditor.tsx @@ -110,9 +110,19 @@ interface QueryEditorProps { status: 'running' | 'success' | 'error'; }; statementResults?: StatementResult[]; + /** + * Writeback for the run-status glyphs. The editor owns the *positions* of the + * glyphs once they exist (they're sticky Monaco decorations that follow + * edits), so when it prunes a glyph whose block was destroyed or refreshes a + * glyph's line number after an edit, it reports the new accumulated set back + * up so the tab state stays in sync (and survives a tab switch / remount). + */ + onStatementResultsChange?: (results: StatementResult[]) => void; } export interface StatementResult { + /** Stable id used to key the sticky gutter decoration to this result. */ + id?: string; lineNumber: number; status: 'running' | 'success' | 'error'; rowsAffected?: number; @@ -121,6 +131,44 @@ export interface StatementResult { executionTime?: number; } +// Build the Monaco glyph-margin decoration options for a run result. +// `NeverGrowsWhenTypingAtEdges` keeps the zero-width glyph range from absorbing +// text typed right at the line start, so the glyph tracks the line cleanly. +function buildGlyphDecoration(monaco: any, result: StatementResult, lineNumber: number) { + const { status, rowCount, rowsAffected, error, executionTime } = result; + + let glyphClassName = 'statement-glyph-running'; + let hoverMessage = 'Query running...'; + let tooltip = ''; + + if (status === 'success') { + glyphClassName = 'statement-glyph-success'; + hoverMessage = 'Query succeeded'; + if (rowCount !== undefined) { + tooltip = `${rowCount} row${rowCount !== 1 ? 's' : ''} retrieved`; + } else if (rowsAffected !== undefined) { + tooltip = `${rowsAffected} row${rowsAffected !== 1 ? 's' : ''} affected`; + } + if (executionTime !== undefined && executionTime > 0) { + tooltip += tooltip ? ` in ${executionTime}ms` : `${executionTime}ms`; + } + } else if (status === 'error') { + glyphClassName = 'statement-glyph-error'; + hoverMessage = 'Query failed'; + tooltip = error || 'Error executing query'; + } + + return { + range: new monaco.Range(lineNumber, 1, lineNumber, 1), + options: { + isWholeLine: false, + glyphMarginClassName: glyphClassName, + glyphMarginHoverMessage: { value: tooltip || hoverMessage }, + stickiness: monaco.editor.TrackedRangeStickiness.NeverGrowsWhenTypingAtEdges, + }, + }; +} + // Show intention actions popup (Alt+Enter) const showIntentionActions = (editor: any, monaco: any, onRunRef: React.MutableRefObject) => { const model = editor.getModel(); @@ -245,18 +293,33 @@ export const QueryEditor = memo(function QueryEditor({ hasError, hasSuccess, lastExecutedStatement: _lastExecutedStatement, - statementResults + statementResults, + onStatementResultsChange }: QueryEditorProps) { const { theme } = useTheme(); const settings = useSettings(); const editorRef = useRef(null); const monacoRef = useRef(null); - const decorationsRef = useRef([]); const onRunRef = useRef(null); const lastSnapshotRef = useRef(""); const snapshotTimerRef = useRef(null); const { schemaItems } = useConnections(); - + + // ─── Run-status gutter glyphs (green/red checkmarks per executed block) ───── + // These are sticky Monaco decorations: once created they follow edits (insert + // a line above a block and its glyph moves down with it) and are pruned when + // their block is destroyed. We key them by the StatementResult id so a glyph + // is never re-pinned to a frozen line number on re-render — the only source + // of truth for a glyph's *position* is Monaco itself. + const glyphDecoRef = useRef>(new Map()); // result id -> monaco decoration id + const glyphAnchorRef = useRef>(new Map()); // result id -> trimmed text of its anchor line + // Latest props captured into refs so the mount-time content-change handler + // (a stable closure) can read them without stale values. + const statementResultsRef = useRef(statementResults); + const onStatementResultsChangeRef = useRef(onStatementResultsChange); + statementResultsRef.current = statementResults; + onStatementResultsChangeRef.current = onStatementResultsChange; + onRunRef.current = onRun; // Auto-snapshot: debounce editor changes and save to local history @@ -282,97 +345,163 @@ export const QueryEditor = memo(function QueryEditor({ lastSchemaHash = ""; // Force cache miss }, [schemaItems]); - // Create a stable fingerprint of statementResults so the effect fires reliably - // even when React batches state updates and the array reference doesn't change. + // Keep isExecuting in a ref so reconcileGlyphs (called from the mount-time + // closure) reads the live value. + const isExecutingRef = useRef(isExecuting); + isExecutingRef.current = isExecuting; + + // Fingerprint that re-triggers reconcile only on meaningful changes. Keyed on + // id so a re-run (new id at the same line) re-fires, and on line number so a + // writeback that refreshes positions re-fires; executionTime is excluded so + // identical reruns don't thrash. const statementResultsFingerprint = useMemo(() => { if (!statementResults || statementResults.length === 0) return ''; - return statementResults.map(r => `${r.lineNumber}:${r.status}:${r.executionTime || 0}`).join(','); + return statementResults.map(r => `${r.id ?? '?'}:${r.status}:${r.lineNumber}`).join(','); }, [statementResults]); - // Effect to update Monaco decorations when statementResults changes (DataGrip-style gutter glyphs) - useEffect(() => { - if (!editorRef.current || !monacoRef.current) return; - + // Reconcile the sticky glyph decorations to match statementResults, keyed by + // id. Surviving glyphs are left exactly where Monaco has tracked them to (so + // they keep following edits); only newly-added ids get a decoration created, + // and removed ids get theirs cleared. This never re-pins a surviving glyph to + // a stale line number — that was the core "checkmark on the wrong block" bug. + const reconcileGlyphs = useCallback(() => { const editor = editorRef.current; const monaco = monacoRef.current; - - // Always clear existing decorations first - if (decorationsRef.current.length > 0) { - decorationsRef.current = editor.deltaDecorations(decorationsRef.current, []); + if (!editor || !monaco) return; + const model = editor.getModel(); + if (!model) return; + // Mid-execution we leave whatever is on screen; results land after it ends. + if (isExecutingRef.current) return; + + const results = statementResultsRef.current ?? []; + const known = glyphDecoRef.current; + const presentIds = new Set(results.map(r => r.id).filter(Boolean) as string[]); + + // Remove decorations whose result is gone (e.g. a re-run replaced the entry). + for (const [id, decoId] of [...known]) { + if (!presentIds.has(id)) { + model.deltaDecorations([decoId], []); + known.delete(id); + glyphAnchorRef.current.delete(id); + } } - - // During execution, show no glyph — wait for results - if (isExecuting) return; - - // Only create gutter glyphs when statementResults has data (after execution completes) - if (!statementResults || statementResults.length === 0) { - return; + + // Create decorations for results that don't have one yet (fresh runs + + // remount seeding). Surviving ids are skipped so Monaco keeps their live + // positions instead of being yanked back to a stored line number. + const lineCount = model.getLineCount(); + const newErrorLines: number[] = []; + for (const res of results) { + if (!res.id || known.has(res.id)) continue; + const line = Math.min(Math.max(res.lineNumber, 1), lineCount); + const [decoId] = model.deltaDecorations([], [buildGlyphDecoration(monaco, res, line)]); + known.set(res.id, decoId); + glyphAnchorRef.current.set(res.id, (model.getLineContent(line) || '').trim()); + if (res.status === 'error') newErrorLines.push(line); } - - // Create decorations for all statement results (DataGrip-style) - const decorations: any[] = []; - - statementResults.forEach((result) => { - const { lineNumber, status, rowCount, rowsAffected, error, executionTime } = result; - - let glyphClassName = 'statement-glyph-running'; - let hoverMessage = 'Query running...'; - let tooltip = ''; - - if (status === 'success') { - glyphClassName = 'statement-glyph-success'; - hoverMessage = 'Query succeeded'; - if (rowCount !== undefined) { - tooltip = `${rowCount} row${rowCount !== 1 ? 's' : ''} retrieved`; - } else if (rowsAffected !== undefined) { - tooltip = `${rowsAffected} row${rowsAffected !== 1 ? 's' : ''} affected`; - } - if (executionTime !== undefined && executionTime > 0) { - tooltip += tooltip ? ` in ${executionTime}ms` : `${executionTime}ms`; - } - } else if (status === 'error') { - glyphClassName = 'statement-glyph-error'; - hoverMessage = 'Query failed'; - tooltip = error || 'Error executing query'; - } - - // Add gutter glyph decoration - decorations.push({ - range: new monaco.Range(lineNumber, 1, lineNumber, 1), - options: { - isWholeLine: false, - glyphMarginClassName: glyphClassName, - glyphMarginHoverMessage: { value: tooltip || hoverMessage }, - } - }); - }); - - // Apply all decorations - if (decorations.length > 0) { - decorationsRef.current = editor.deltaDecorations([], decorations); + + // Reveal a freshly-failed statement — but only for newly-created error + // glyphs, so an edit-driven writeback never yanks the viewport around. + if (newErrorLines.length > 0) { + editor.revealLineInCenter(newErrorLines[newErrorLines.length - 1]); } - - // Scroll the last statement into view if there are errors - const hasErrors = statementResults.some(r => r.status === 'error'); - if (hasErrors) { - const lastError = [...statementResults].reverse().find(r => r.status === 'error'); - if (lastError) { - editor.revealLineInCenter(lastError.lineNumber); + }, []); + + // Prune/refresh glyphs as the text changes: drop a glyph whose anchor line no + // longer reads as the block that produced it (edited or cut away) and keep the + // survivors' line numbers fresh. The new set is reported up so tab state — + // which must survive a tab switch / remount — stays in sync. + const pruneGlyphs = useCallback(() => { + const editor = editorRef.current; + if (!editor) return; + const model = editor.getModel(); + if (!model) return; + + const known = glyphDecoRef.current; + if (known.size === 0) return; + + const removed = new Set(); + const lineById = new Map(); + + for (const [id, decoId] of [...known]) { + const range = model.getDecorationRange(decoId); + if (!range) { + known.delete(id); + glyphAnchorRef.current.delete(id); + removed.add(id); + continue; + } + const line = range.startLineNumber; + const lineText = (model.getLineContent(line) || '').trim(); + const anchor = glyphAnchorRef.current.get(id); + // Destroyed if the anchor line is now blank or its text changed (the block + // was edited or cut). Following in place — inserting/deleting *other* lines + // — leaves this line's text identical, so the glyph survives and shifts. + if (!lineText || (anchor !== undefined && lineText !== anchor)) { + model.deltaDecorations([decoId], []); + known.delete(id); + glyphAnchorRef.current.delete(id); + removed.add(id); + } else { + lineById.set(id, line); } } - - }, [statementResultsFingerprint, isExecuting]); + + if (removed.size === 0 && lineById.size === 0) return; + + const current = statementResultsRef.current ?? []; + let changed = removed.size > 0; + const next = current + .filter(r => !(r.id && removed.has(r.id))) + .map(r => { + const newLine = r.id ? lineById.get(r.id) : undefined; + if (newLine !== undefined && newLine !== r.lineNumber) { + changed = true; + return { ...r, lineNumber: newLine }; + } + return r; + }); + + if (changed) onStatementResultsChangeRef.current?.(next); + }, []); + + // Drive reconcile from React state changes (new runs, writebacks). Mount-time + // seeding is also kicked from handleEditorMount in case onMount fires after + // this effect's first pass. + useEffect(() => { + reconcileGlyphs(); + }, [statementResultsFingerprint, isExecuting, reconcileGlyphs]); const handleEditorMount: OnMount = (editor, monaco) => { monacoRef.current = monaco; editorRef.current = editor; - + + // Seed any persisted run-status glyphs now that the editor exists. The + // statementResults effect may have run its first pass before onMount fired + // (editorRef was still null then), so a tab reopened with prior glyphs would + // otherwise show none until the next run. + reconcileGlyphs(); + + // Prune/refresh the sticky glyphs as the text changes (throttled so large + // files stay responsive). Glyphs follow edits via Monaco's own tracking; + // this only removes ones whose block was destroyed and keeps line numbers + // fresh in tab state. + let glyphPruneThrottle: ReturnType | null = null; + const throttledGlyphPrune = () => { + if (glyphPruneThrottle !== null) return; + glyphPruneThrottle = setTimeout(() => { + glyphPruneThrottle = null; + pruneGlyphs(); + }, 200); + }; + const glyphPruneDisposable = editor.onDidChangeModelContent(() => throttledGlyphPrune()); + // Focus after a short delay to ensure UI is ready setTimeout(() => editor.focus(), 100); const focusHandler = () => editor.focus(); const formatHandler = () => editor.getAction('editor.action.formatDocument')?.run(); - + window.addEventListener("focus-editor", focusHandler); window.addEventListener("format-sql", formatHandler); @@ -667,12 +796,14 @@ export const QueryEditor = memo(function QueryEditor({ contentChangeDisposable?.dispose(); cursorMoveDisposable?.dispose(); cursorContentDisposable?.dispose(); + glyphPruneDisposable?.dispose(); window.removeEventListener("focus-editor", focusHandler); window.removeEventListener("format-sql", formatHandler); window.removeEventListener("run-query-smart", handleRunSmart); window.removeEventListener("run-query-all", handleRunAll); if (domNode) domNode.removeEventListener("contextmenu", handleContextMenu); if (varDecoThrottle !== null) clearTimeout(varDecoThrottle); + if (glyphPruneThrottle !== null) clearTimeout(glyphPruneThrottle); }); // NOTE: Ctrl+Shift+F is intentionally NOT bound to formatDocument here. diff --git a/src/components/layout/MainContent.tsx b/src/components/layout/MainContent.tsx index e7d82dd..3ac96a5 100644 --- a/src/components/layout/MainContent.tsx +++ b/src/components/layout/MainContent.tsx @@ -12,6 +12,7 @@ import { EmptyStateLauncher } from "./EmptyStateLauncher"; import { logger } from "../../utils/logger"; import { getDefaultDatabaseName } from "../../config/app"; import { splitStatements } from "../../utils/splitStatements"; +import { mapSelectionStatementsToDocumentLines, mergeGlyphResults } from "../../utils/statementGlyphs"; import { applyQueryLimit } from "../../utils/applyQueryLimit"; import { VariableSubstitutionDialog, extractVariables, substituteVariables, VariableValues } from "../ui/VariableSubstitutionDialog"; import { useLocalHistory } from "../../store/localHistoryStore"; @@ -92,6 +93,15 @@ export interface PsqlConsoleEntry { } export interface StatementResult { + /** + * Stable id assigned when the glyph is accumulated into a tab's results. + * The editor keys its sticky Monaco decorations off this so a glyph can + * follow edits (and be pruned when its block is destroyed) without being + * re-pinned to a frozen line number on every re-render. Optional because + * the per-run results built during execution don't have one yet — it's + * assigned by `appendGlyphResults`. + */ + id?: string; lineNumber: number; status: 'running' | 'success' | 'error'; rowsAffected?: number; @@ -714,6 +724,28 @@ export function MainContent() { setQueryTabs(prev => prev.map(t => t.id === tabId ? { ...t, ...updates } : t)); }, []); + // Accumulate run-status gutter glyphs for a tab. Each run's results are + // merged into whatever the tab already has (one glyph per block that's been + // run; re-running a block replaces its glyph — see mergeGlyphResults), rather + // than wiping the lot every execution. Runs through the functional setState so + // it always merges into the freshest results, including the editor's own + // sticky-position writebacks. Ids are assigned here so the editor can key its + // decorations stably. + const appendGlyphResults = useCallback((tabId: string, newResults: StatementResult[]) => { + if (newResults.length === 0) return; + const withIds = newResults.map(r => ({ ...r, id: crypto.randomUUID() })); + setQueryTabs(prev => prev.map(t => t.id === tabId + ? { ...t, statementResults: mergeGlyphResults(t.statementResults ?? [], withIds) } + : t)); + }, []); + + // Replace the accumulated glyph set wholesale — used by the editor's writeback + // when it prunes destroyed glyphs or refreshes their line numbers as the text + // is edited. + const setGlyphResults = useCallback((tabId: string, results: StatementResult[]) => { + setQueryTabs(prev => prev.map(t => t.id === tabId ? { ...t, statementResults: results } : t)); + }, []); + const addNewTab = useCallback(( query = "", name = "", @@ -879,11 +911,19 @@ const executeQuery = useCallback(async (specificQuery?: any, statementInfo?: { l // commands into a prepared statement", and the libpq path further down // sends one execute() per statement only when isRunAll is set. if (!isRunAll && typeof specificQuery === "string") { - const parts = splitStatements(queryToRun); - if (parts.length > 1) { + // Split the *original* selection (not the trimmed queryToRun) and map each + // statement back to its document-absolute line. `statementInfo.lineNumber` + // is the document line where the selection (char 0 of specificQuery) began, + // so a statement reported at relative line N sits on document line + // baseLine + (N - 1). Using splitStatements(queryToRun) here was the bug + // that put glyphs on the wrong block: those line numbers are relative to + // the selection, not the document. + const baseLine = statementInfo?.lineNumber ?? 1; + const mapped = mapSelectionStatementsToDocumentLines(specificQuery, baseLine); + if (mapped.length > 1) { isRunAll = true; - statementsToRun = parts.map(p => p.text); - statementInfos = parts.map(p => ({ lineNumber: p.lineNumber, statementText: p.text })); + statementsToRun = mapped.map(p => p.text); + statementInfos = mapped.map(p => ({ lineNumber: p.lineNumber, statementText: p.text })); } } runningCmdRef.current = queryToRun; @@ -959,11 +999,12 @@ const executeQuery = useCallback(async (specificQuery?: any, statementInfo?: { l setMultiResults([]); setRunningTimeMs(0); cancelFlagRef.current = false; - - // Clear statement results when starting new execution (glyphs will appear after execution completes) - if (currentTabId) { - updateTabState(currentTabId, { statementResults: [] }); - } + + // Note: we intentionally do NOT clear statementResults here. Glyphs now + // accumulate (one per block that's been run) and follow the text as it's + // edited, so wiping them on every run would defeat the feature. Each run + // merges its result into the existing set via appendGlyphResults; re-running + // a block replaces just that block's glyph. const startTime = Date.now(); let intervalId: any = null; @@ -1392,7 +1433,7 @@ const executeQuery = useCallback(async (specificQuery?: any, statementInfo?: { l error: mr.error, executionTime: 0 }))); - if (currentTabId) updateTabState(currentTabId, { statementResults }); + if (currentTabId) appendGlyphResults(currentTabId, statementResults); const selectResults = multiResults.filter(r => r.rows && r.rows.length > 0); if (selectResults.length > 0) { rows = selectResults[0].rows || []; @@ -1509,8 +1550,8 @@ const executeQuery = useCallback(async (specificQuery?: any, statementInfo?: { l rows = cliRows ?? []; rowsAffected = rows.length; statementResults.push({ lineNumber: stmtInfo.lineNumber, status: 'success', rowCount: rowsAffected, executionTime: Date.now() - stmtStartTime }); - // Store columns for the ResultPanel - if (currentTabId) updateTabState(currentTabId, { statementResults, columns: cliCols ?? [] }); + // Store columns for the ResultPanel (glyphs are appended below). + if (currentTabId) updateTabState(currentTabId, { columns: cliCols ?? [] }); setLastColumns(cliCols ?? []); } else { const { rowsAffected: affected } = await cliExecStmt(queryToRun, false, currentPsqlExpanded, currentCliDatabase || ""); @@ -1518,7 +1559,6 @@ const executeQuery = useCallback(async (specificQuery?: any, statementInfo?: { l setSuccess(`Query executed successfully. ${rowsAffected} rows affected.`); rows = []; statementResults.push({ lineNumber: stmtInfo.lineNumber, status: 'success', rowsAffected, executionTime: Date.now() - stmtStartTime }); - if (currentTabId) updateTabState(currentTabId, { statementResults }); } } catch (stmtErr: any) { // Show the error in the psql terminal @@ -1546,7 +1586,7 @@ const executeQuery = useCallback(async (specificQuery?: any, statementInfo?: { l isExecutingRef.current = false; return; } - if (currentTabId) updateTabState(currentTabId, { statementResults }); + if (currentTabId) appendGlyphResults(currentTabId, statementResults); } // Skip the libpq block entirely @@ -1733,9 +1773,9 @@ const executeQuery = useCallback(async (specificQuery?: any, statementInfo?: { l // Store statement results for gutter glyphs if (currentTabId) { - updateTabState(currentTabId, { statementResults }); + appendGlyphResults(currentTabId, statementResults); } - + // Combine results - use first SELECT result, or show counts for all const selectResults = multiResults.filter(r => r.rows && r.rows.length > 0); if (selectResults.length > 0) { @@ -1775,7 +1815,7 @@ const executeQuery = useCallback(async (specificQuery?: any, statementInfo?: { l // Store statement results for gutter glyphs if (currentTabId) { - updateTabState(currentTabId, { statementResults }); + appendGlyphResults(currentTabId, statementResults); } } @@ -2030,20 +2070,18 @@ const executeQuery = useCallback(async (specificQuery?: any, statementInfo?: { l error: errorMsg, executionTime: duration }; - - // If we already have partial results from multi-statement execution, update them - // Note: statementResults is inside the inner try block, not accessible here. - // Start with the error result. - const updatedStatementResults: StatementResult[] = [errorStatementResult]; - - updateTabState(currentTabId, { - error: errorMsg, - success: null, + + // Accumulate the error glyph onto whatever the tab already has (it + // replaces any prior glyph on the same line — see mergeGlyphResults). + appendGlyphResults(currentTabId, [errorStatementResult]); + + updateTabState(currentTabId, { + error: errorMsg, + success: null, executionTime: duration, - statementResults: updatedStatementResults, - lastExecutedStatement: { - lineNumber: errorLineNumber, - status: 'error' + lastExecutedStatement: { + lineNumber: errorLineNumber, + status: 'error' } }); } @@ -3523,6 +3561,7 @@ const executeQuery = useCallback(async (specificQuery?: any, statementInfo?: { l hasError={!!error} hasSuccess={!!success} statementResults={activeTab?.statementResults} + onStatementResultsChange={(rs) => activeTabId && setGlyphResults(activeTabId, rs)} /> ) diff --git a/src/utils/statementGlyphs.test.ts b/src/utils/statementGlyphs.test.ts new file mode 100644 index 0000000..abb869e --- /dev/null +++ b/src/utils/statementGlyphs.test.ts @@ -0,0 +1,65 @@ +import { describe, it, expect } from "vitest"; +import { + mapSelectionStatementsToDocumentLines, + mergeGlyphResults, +} from "./statementGlyphs"; + +describe("mapSelectionStatementsToDocumentLines", () => { + it("maps a single-statement selection to its document line", () => { + // Selection that starts on document line 6. + const out = mapSelectionStatementsToDocumentLines("SELECT 1", 6); + expect(out).toEqual([{ text: "SELECT 1", lineNumber: 6 }]); + }); + + it("offsets every statement's line by the selection's start line", () => { + // Two statements; relative lines 1 and 3 inside the selection. + const selection = "SELECT 1;\n\nSELECT 2"; + const out = mapSelectionStatementsToDocumentLines(selection, 6); + // The regression: previously these came back as 1 and 3 (relative to the + // selection) so the glyphs landed on document lines 1 and 3 instead of 6/8. + expect(out).toEqual([ + { text: "SELECT 1", lineNumber: 6 }, + { text: "SELECT 2", lineNumber: 8 }, + ]); + }); + + it("treats a selection starting on line 1 as document-absolute already", () => { + const out = mapSelectionStatementsToDocumentLines("SELECT 1;\nSELECT 2", 1); + expect(out).toEqual([ + { text: "SELECT 1", lineNumber: 1 }, + { text: "SELECT 2", lineNumber: 2 }, + ]); + }); + + it("returns an empty list for whitespace-only selections", () => { + expect(mapSelectionStatementsToDocumentLines(" \n ", 4)).toEqual([]); + }); +}); + +describe("mergeGlyphResults", () => { + it("appends a glyph for a newly-run block, keeping the others", () => { + const existing = [{ lineNumber: 1, status: "success" }]; + const incoming = [{ lineNumber: 6, status: "success" }]; + expect(mergeGlyphResults(existing, incoming)).toEqual([ + { lineNumber: 1, status: "success" }, + { lineNumber: 6, status: "success" }, + ]); + }); + + it("replaces the glyph on a re-run of the same line", () => { + const existing = [ + { lineNumber: 1, status: "success", executionTime: 5 }, + { lineNumber: 6, status: "error", executionTime: 9 }, + ]; + const incoming = [{ lineNumber: 6, status: "success", executionTime: 2 }]; + expect(mergeGlyphResults(existing, incoming)).toEqual([ + { lineNumber: 1, status: "success", executionTime: 5 }, + { lineNumber: 6, status: "success", executionTime: 2 }, + ]); + }); + + it("returns existing unchanged when there's nothing incoming", () => { + const existing = [{ lineNumber: 1, status: "success" }]; + expect(mergeGlyphResults(existing, [])).toBe(existing); + }); +}); diff --git a/src/utils/statementGlyphs.ts b/src/utils/statementGlyphs.ts new file mode 100644 index 0000000..4fed613 --- /dev/null +++ b/src/utils/statementGlyphs.ts @@ -0,0 +1,60 @@ +/** + * Pure helpers for the "run-status gutter glyphs" feature (the green/red + * checkmarks shown next to each SQL block that has been run). + * + * The hard parts of that feature — placing a glyph on the right line and + * accumulating one glyph per block across runs — used to be done with + * frozen, run-time line numbers and got out of sync the moment the editor + * text changed or a multi-statement *selection* was run (the substring's + * line numbers were 1-based from the selection, not the document). These + * helpers isolate the two bits of logic that are easy to get wrong so they + * can be unit-tested without a live Monaco editor. + */ +import { splitStatements } from "./splitStatements"; + +export interface DocumentStatement { + /** Trimmed statement text, without the terminating semicolon. */ + text: string; + /** 1-based line number in the *document* where the statement begins. */ + lineNumber: number; +} + +/** + * Split a selection's text into top-level statements and map each one back to + * its document-absolute 1-based line number. + * + * Why this exists: when the user runs a selection (or a multi-statement block), + * the only text available downstream is the selected substring. Splitting that + * substring gives line numbers relative to the *selection* (statement 2 might + * report line 4 even though it sits on document line 14). The glyph then lands + * on the wrong block. `selectionStartLine` is the document line where char 0 of + * `selectionText` lives, so the document line is simply + * `selectionStartLine + (relativeLine - 1)`. + */ +export function mapSelectionStatementsToDocumentLines( + selectionText: string, + selectionStartLine: number, +): DocumentStatement[] { + return splitStatements(selectionText).map((s) => ({ + text: s.text, + lineNumber: selectionStartLine + (s.lineNumber - 1), + })); +} + +/** + * Accumulate run-status glyphs across runs. + * + * Each new run replaces any existing glyph that sits on the same line (it's a + * re-run of that block) and appends the rest, so a tab keeps exactly one glyph + * per distinct block that has been run rather than wiping the others every time + * a single block is executed. + */ +export function mergeGlyphResults( + existing: T[], + incoming: T[], +): T[] { + if (incoming.length === 0) return existing; + const incomingLines = new Set(incoming.map((r) => r.lineNumber)); + const kept = existing.filter((r) => !incomingLines.has(r.lineNumber)); + return [...kept, ...incoming]; +}