mirror of
https://github.com/t8y2/dbx.git
synced 2026-10-19 16:16:45 +00:00
fix(editor): suppress repeated execution shortcuts while running
This commit is contained in:
@@ -4747,19 +4747,19 @@ async function handleKeydown(e: KeyboardEvent) {
|
||||
if (isExecuteSqlShortcut(e, shortcuts) && defaultQueryShortcutTarget) {
|
||||
e.preventDefault();
|
||||
e.stopPropagation();
|
||||
void newQuery({ preferredSource: "sidebar", openDefault: true });
|
||||
if (!e.repeat) void newQuery({ preferredSource: "sidebar", openDefault: true });
|
||||
return;
|
||||
}
|
||||
if (activeTab.value?.mode === "query" && isExecuteSqlInNewResultTabShortcut(e, shortcuts) && e.target instanceof Element && e.target.closest("[data-query-editor-root]")) {
|
||||
e.preventDefault();
|
||||
e.stopPropagation();
|
||||
if (!contentAreaRef.value?.shouldBlockQueryEditorExecutionShortcut?.(e)) requestActiveEditorExecuteInNewResultTab();
|
||||
if (!e.repeat && !activeTab.value.isExecuting && !contentAreaRef.value?.shouldBlockQueryEditorExecutionShortcut?.(e)) requestActiveEditorExecuteInNewResultTab();
|
||||
return;
|
||||
}
|
||||
if (activeTab.value?.mode === "query" && isExecuteSqlShortcut(e, shortcuts) && e.target instanceof Element && e.target.closest("[data-query-editor-root]")) {
|
||||
e.preventDefault();
|
||||
e.stopPropagation();
|
||||
if (!contentAreaRef.value?.shouldBlockQueryEditorExecutionShortcut?.(e)) requestActiveEditorExecute();
|
||||
if (!e.repeat && !activeTab.value.isExecuting && !contentAreaRef.value?.shouldBlockQueryEditorExecutionShortcut?.(e)) requestActiveEditorExecute();
|
||||
return;
|
||||
}
|
||||
if (activeTab.value?.mode === "query" && isSendSelectionToAiShortcut(e, shortcuts) && e.target instanceof Element && e.target.closest("[data-query-editor-root]")) {
|
||||
|
||||
@@ -138,7 +138,7 @@ import { scrollEditorLine } from "@/lib/editor/scrollEditorLine";
|
||||
import { createQueryEditorStringMouseSelection } from "@/lib/editor/queryEditorStringMouseSelection";
|
||||
import { createQueryEditorCompletionShortcutBindings } from "@/lib/editor/queryEditorCompletionShortcut";
|
||||
import { createQueryEditorSelectionCaseShortcutBindings } from "@/lib/editor/queryEditorSelectionCaseShortcut";
|
||||
import { createQueryEditorExecutionShortcutBindings, createQueryEditorPostCompositionKeyGuard } from "@/lib/editor/queryEditorExecutionShortcut";
|
||||
import { createQueryEditorExecutionShortcutBindings, createQueryEditorExecutionShortcutHandler, createQueryEditorPostCompositionKeyGuard } from "@/lib/editor/queryEditorExecutionShortcut";
|
||||
import { supportsQueryEditorBlockComments, supportsSqlInListPaste } from "@/lib/database/databaseFeatureSupport";
|
||||
import { queryContextObjectRoute, queryTableCandidateAtSqlPosition, resolveQueryContextCandidateDatabase, resolveQueryContextObjectTarget, type QueryContextObjectAction } from "@/lib/sql/queryCursorTableTarget";
|
||||
import * as api from "@/lib/backend/api";
|
||||
@@ -1311,8 +1311,16 @@ function runKeymapExtension(codeMirrorKeymap: (typeof import("@codemirror/view")
|
||||
shortcuts.executeSql,
|
||||
() => requestExecute({ bypassPicker: true }),
|
||||
(currentView) => shouldBlockExecutionShortcut(undefined, currentView),
|
||||
() => !props.isExecuting,
|
||||
);
|
||||
const executeInNewResultTabBindings = props.hideExecutionControls
|
||||
? []
|
||||
: createQueryEditorExecutionShortcutBindings(
|
||||
shortcuts.executeSqlInNewResultTab,
|
||||
requestExecuteInNewResultTab,
|
||||
(currentView) => shouldBlockExecutionShortcut(undefined, currentView),
|
||||
() => !props.isExecuting,
|
||||
);
|
||||
const executeInNewResultTabBindings = props.hideExecutionControls ? [] : createQueryEditorExecutionShortcutBindings(shortcuts.executeSqlInNewResultTab, requestExecuteInNewResultTab, (currentView) => shouldBlockExecutionShortcut(undefined, currentView));
|
||||
const explainBindings = props.enableExplainShortcut
|
||||
? createQueryEditorExecutionShortcutBindings(
|
||||
shortcuts.explainSql,
|
||||
@@ -1330,6 +1338,11 @@ function runKeymapExtension(codeMirrorKeymap: (typeof import("@codemirror/view")
|
||||
openReplace,
|
||||
isReadOnly: () => !!props.readOnly,
|
||||
});
|
||||
const executionShortcutHandler = createQueryEditorExecutionShortcutHandler({
|
||||
getShortcuts: () => shortcuts,
|
||||
isExecuting: () => !!props.isExecuting,
|
||||
enableExplainShortcut: () => !!props.enableExplainShortcut,
|
||||
});
|
||||
const sqlShortcutActions = enabledSqlShortcutActions(settingsStore.editorSettings.sqlShortcuts);
|
||||
const sqlShortcutKeymapBindings = uniqueSqlShortcutBindings(sqlShortcutActions).filter((shortcut) => !isCharacterProducingShortcut(shortcut));
|
||||
// Do not set preventDefault: true — when run returns false (wrong DB scope / no table token),
|
||||
@@ -1354,6 +1367,7 @@ function runKeymapExtension(codeMirrorKeymap: (typeof import("@codemirror/view")
|
||||
() => props.databaseType,
|
||||
);
|
||||
const combinedDomKeydownHandler = (event: KeyboardEvent, view: EditorViewType) => {
|
||||
if (executionShortcutHandler(event)) return true;
|
||||
if (replaceShortcutHandler(event)) return true;
|
||||
return sqlShortcutDomHandler(event, view);
|
||||
};
|
||||
@@ -2896,6 +2910,8 @@ function unfoldAll(): boolean {
|
||||
}
|
||||
|
||||
function shouldBlockExecutionShortcut(event?: KeyboardEvent, currentView: EditorViewType | null = view.value): boolean {
|
||||
if (props.isExecuting) return true;
|
||||
if (event?.repeat) return true;
|
||||
return (currentView ? isEditorComposing(currentView) : false) || (event ? postCompositionKeyGuard.blocks(event) : false);
|
||||
}
|
||||
|
||||
|
||||
@@ -36,6 +36,7 @@ export interface QueryEditorProps {
|
||||
hideExecutionControls?: boolean;
|
||||
enableExplainShortcut?: boolean;
|
||||
canExplain?: boolean;
|
||||
isExecuting?: boolean;
|
||||
initialViewport?: { scrollTop: number; scrollLeft: number };
|
||||
initialSelection?: { anchor: number; head: number };
|
||||
revealRequest?: { id: number; line: number; column?: number };
|
||||
|
||||
@@ -53,6 +53,7 @@ export function useQueryEditorExecution(options: QueryEditorExecutionOptions) {
|
||||
}
|
||||
|
||||
function requestExecute(options: RequestExecuteOptions = {}) {
|
||||
if (props.isExecuting) return false;
|
||||
executionViewportOwnership.cancelPendingRequest();
|
||||
const currentView = view.value;
|
||||
if (!currentView) return false;
|
||||
@@ -61,6 +62,7 @@ export function useQueryEditorExecution(options: QueryEditorExecutionOptions) {
|
||||
}
|
||||
|
||||
function requestExecuteInNewResultTab() {
|
||||
if (props.isExecuting) return false;
|
||||
return requestExecute({ bypassPicker: true, openInNewResultTab: true });
|
||||
}
|
||||
|
||||
|
||||
@@ -2049,6 +2049,7 @@ defineExpose({
|
||||
:hide-execution-controls="!!activeTab.ddlViewer"
|
||||
enable-explain-shortcut
|
||||
:can-explain="!activeTab.isExecuting && !activeTab.isExplaining && !!executableSql.trim()"
|
||||
:is-executing="activeTab.isExecuting"
|
||||
@update:model-value="emit('editorUpdate', activeTab.id, $event)"
|
||||
@selection-change="emit('editorSelectionChange', activeTab.id, $event)"
|
||||
@send-selection-to-ai="emit('sendSelectionToAi', activeTab.id, $event)"
|
||||
|
||||
@@ -221,6 +221,30 @@ describe("useSqlExecution", () => {
|
||||
expect(refreshObjects).toHaveBeenCalledWith("conn-1", "app", "public");
|
||||
});
|
||||
|
||||
it("rejects execution when the target tab is already executing", async () => {
|
||||
const sql = "SELECT 1;";
|
||||
const activeTab = ref<QueryTab | undefined>({ ...queryTab("app"), sql, isExecuting: true });
|
||||
const activeConnection = ref<ConnectionConfig | undefined>(connection("mysql"));
|
||||
const activeOutputView = ref<"result" | "summary" | "explain" | "chart">("result");
|
||||
const queryStore = useQueryStore();
|
||||
const executeCurrentSql = vi.spyOn(queryStore, "executeCurrentSql");
|
||||
const onExecutionCancelled = vi.fn();
|
||||
|
||||
const execution = useSqlExecution({
|
||||
activeTab: computed(() => activeTab.value),
|
||||
activeConnection: computed(() => activeConnection.value),
|
||||
executableSql: computed(() => sql),
|
||||
activeOutputView,
|
||||
onExecutionCancelled,
|
||||
});
|
||||
|
||||
const outcome = await execution.tryExecute(undefined, { editorViewportRequestId: 123 });
|
||||
|
||||
expect(outcome).toEqual({ status: "not-started", reason: "busy", tabId: "tab-1" });
|
||||
expect(executeCurrentSql).not.toHaveBeenCalled();
|
||||
expect(onExecutionCancelled).toHaveBeenCalledWith(123);
|
||||
});
|
||||
|
||||
it("sends every placeholder syntax and @set unchanged when substitution is disabled", async () => {
|
||||
const sql = ["@set tenant = 42;", "SELECT ?, :named, ${shell}, #{mybatis}, @sqlserver, @tenant;"].join("\n");
|
||||
const activeTab = ref<QueryTab | undefined>({ ...queryTab("app"), sql });
|
||||
|
||||
@@ -295,7 +295,8 @@ export function useSqlExecution(deps: {
|
||||
cancelEditorViewportRequest(options.editorViewportRequestId);
|
||||
return notStarted("no-tab", options.tabId);
|
||||
}
|
||||
const { sql, sourceOffset, editorViewportRequestId } = await resolvedExecutableSql(sqlOverride, context);
|
||||
const { sql, sourceOffset, editorViewportRequestId: snapshotRequestId } = await resolvedExecutableSql(sqlOverride, context);
|
||||
const editorViewportRequestId = snapshotRequestId ?? options.editorViewportRequestId;
|
||||
const executionOptions = { ...options, editorViewportRequestId, tabId: context.tabId };
|
||||
if (!sql.trim()) {
|
||||
cancelEditorViewportRequest(editorViewportRequestId);
|
||||
@@ -306,6 +307,10 @@ export function useSqlExecution(deps: {
|
||||
cancelEditorViewportRequest(editorViewportRequestId);
|
||||
return notStarted("no-tab", context.tabId);
|
||||
}
|
||||
if (tab.isExecuting) {
|
||||
cancelEditorViewportRequest(editorViewportRequestId);
|
||||
return notStarted("busy", context.tabId);
|
||||
}
|
||||
if (requiresDatabaseSelection(tab, context.connection, sql)) {
|
||||
deps.onMissingDatabase?.(context.tabId);
|
||||
cancelEditorViewportRequest(editorViewportRequestId);
|
||||
|
||||
+75
-1
@@ -3,7 +3,7 @@
|
||||
import { EditorState } from "@codemirror/state";
|
||||
import { EditorView, keymap, runScopeHandlers } from "@codemirror/view";
|
||||
import { describe, expect, it, vi } from "vitest";
|
||||
import { createQueryEditorExecutionShortcutBindings } from "../../editor/queryEditorExecutionShortcut";
|
||||
import { createQueryEditorExecutionShortcutBindings, createQueryEditorExecutionShortcutHandler } from "../../editor/queryEditorExecutionShortcut";
|
||||
|
||||
function createView(bindings: ReturnType<typeof createQueryEditorExecutionShortcutBindings>[]): EditorView {
|
||||
return new EditorView({
|
||||
@@ -105,4 +105,78 @@ describe("QueryEditor execution shortcuts", () => {
|
||||
expect(executeInNewResultTab).toHaveBeenCalledOnce();
|
||||
view.destroy();
|
||||
});
|
||||
|
||||
describe("createQueryEditorExecutionShortcutHandler", () => {
|
||||
const shortcuts = {
|
||||
executeSql: "Mod+Enter",
|
||||
executeSqlInNewResultTab: "Mod+\\",
|
||||
explainSql: "Mod+E",
|
||||
};
|
||||
|
||||
it("passes through non-repeated events when not executing", () => {
|
||||
const handler = createQueryEditorExecutionShortcutHandler({
|
||||
getShortcuts: () => shortcuts,
|
||||
isExecuting: () => false,
|
||||
});
|
||||
|
||||
const event = executeEvent("Enter", { repeat: false });
|
||||
const preventDefault = vi.spyOn(event, "preventDefault");
|
||||
const stopPropagation = vi.spyOn(event, "stopPropagation");
|
||||
|
||||
expect(handler(event)).toBe(false);
|
||||
expect(preventDefault).not.toHaveBeenCalled();
|
||||
expect(stopPropagation).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("suppresses execution shortcuts when event.repeat is true", () => {
|
||||
const handler = createQueryEditorExecutionShortcutHandler({
|
||||
getShortcuts: () => shortcuts,
|
||||
isExecuting: () => false,
|
||||
});
|
||||
|
||||
const event = executeEvent("Enter", { repeat: true });
|
||||
const preventDefault = vi.spyOn(event, "preventDefault");
|
||||
const stopPropagation = vi.spyOn(event, "stopPropagation");
|
||||
|
||||
expect(handler(event)).toBe(true);
|
||||
expect(preventDefault).toHaveBeenCalled();
|
||||
expect(stopPropagation).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("suppresses execution shortcuts when isExecuting is true", () => {
|
||||
const handler = createQueryEditorExecutionShortcutHandler({
|
||||
getShortcuts: () => shortcuts,
|
||||
isExecuting: () => true,
|
||||
});
|
||||
|
||||
const event = executeEvent("Enter", { repeat: false });
|
||||
const preventDefault = vi.spyOn(event, "preventDefault");
|
||||
const stopPropagation = vi.spyOn(event, "stopPropagation");
|
||||
|
||||
expect(handler(event)).toBe(true);
|
||||
expect(preventDefault).toHaveBeenCalled();
|
||||
expect(stopPropagation).toHaveBeenCalled();
|
||||
});
|
||||
|
||||
it("suppresses explain shortcut when enabled and isExecuting is true", () => {
|
||||
const handler = createQueryEditorExecutionShortcutHandler({
|
||||
getShortcuts: () => shortcuts,
|
||||
isExecuting: () => true,
|
||||
enableExplainShortcut: () => true,
|
||||
});
|
||||
|
||||
const event = executeEvent("e", { repeat: false });
|
||||
expect(handler(event)).toBe(true);
|
||||
});
|
||||
|
||||
it("does not suppress unrelated keys even if repeating", () => {
|
||||
const handler = createQueryEditorExecutionShortcutHandler({
|
||||
getShortcuts: () => shortcuts,
|
||||
isExecuting: () => true,
|
||||
});
|
||||
|
||||
const event = new KeyboardEvent("keydown", { key: "a", repeat: true });
|
||||
expect(handler(event)).toBe(false);
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -180,6 +180,10 @@ export function isExecuteSqlInNewResultTabShortcut(event: ShortcutLikeEvent, sho
|
||||
return matchesShortcut(event, actionShortcut("executeSqlInNewResultTab", shortcuts));
|
||||
}
|
||||
|
||||
export function isExplainSqlShortcut(event: ShortcutLikeEvent, shortcuts?: Partial<ShortcutSettings>): boolean {
|
||||
return matchesShortcut(event, actionShortcut("explainSql", shortcuts));
|
||||
}
|
||||
|
||||
export function isCloseTabShortcut(event: ShortcutLikeEvent, shortcuts?: Partial<ShortcutSettings>): boolean {
|
||||
return matchesShortcut(event, actionShortcut("closeTab", shortcuts));
|
||||
}
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import type { Command, EditorView, KeyBinding } from "@codemirror/view";
|
||||
import { shortcutToCodeMirrorKey } from "@/lib/editor/shortcutRegistry";
|
||||
import { shortcutToCodeMirrorKey, type ShortcutSettings } from "@/lib/editor/shortcutRegistry";
|
||||
import { isExecuteSqlShortcut, isExecuteSqlInNewResultTabShortcut, isExplainSqlShortcut } from "@/lib/editor/keyboardShortcuts";
|
||||
|
||||
const SAFARI_POST_COMPOSITION_KEY_WINDOW_MS = 100;
|
||||
|
||||
@@ -72,3 +73,19 @@ export function createQueryEditorExecutionShortcutBindings(shortcut: string, run
|
||||
},
|
||||
];
|
||||
}
|
||||
|
||||
/**
|
||||
* Intercept DOM keydown events for execution shortcuts to prevent duplicate
|
||||
* executions caused by key repeat or while a query is already running.
|
||||
*/
|
||||
export function createQueryEditorExecutionShortcutHandler(options: { getShortcuts: () => Partial<ShortcutSettings>; isExecuting?: () => boolean; enableExplainShortcut?: () => boolean }): (event: KeyboardEvent) => boolean {
|
||||
return (event: KeyboardEvent) => {
|
||||
if (!event.repeat && !options.isExecuting?.()) return false;
|
||||
const shortcuts = options.getShortcuts();
|
||||
const isExecution = isExecuteSqlShortcut(event, shortcuts) || isExecuteSqlInNewResultTabShortcut(event, shortcuts) || (options.enableExplainShortcut?.() && isExplainSqlShortcut(event, shortcuts));
|
||||
if (!isExecution) return false;
|
||||
event.preventDefault();
|
||||
event.stopPropagation();
|
||||
return true;
|
||||
};
|
||||
}
|
||||
|
||||
@@ -1496,6 +1496,31 @@ describe("queryStore multi-statement errors", () => {
|
||||
expect(tab.executionId).toBeUndefined();
|
||||
});
|
||||
|
||||
it("rejects executeCurrentSql when another query is currently executing", async () => {
|
||||
const { useQueryStore } = await import("@/stores/queryStore");
|
||||
const store = useQueryStore();
|
||||
const id = store.createTab("mysql-1", "app", "ConcurrentRun");
|
||||
const tab = store.tabs.find((item) => item.id === id)!;
|
||||
const gate = deferred<QueryResult[]>();
|
||||
mocks.executeMulti.mockReturnValueOnce(gate.promise);
|
||||
|
||||
const firstRun = store.executeCurrentSql("SELECT 1", { tabId: id });
|
||||
expect(tab.isExecuting).toBe(true);
|
||||
|
||||
const secondRun = await store.executeCurrentSql("SELECT 2", { tabId: id });
|
||||
expect(secondRun).toBe(false);
|
||||
|
||||
await vi.waitFor(() => expect(mocks.executeMulti).toHaveBeenCalledTimes(1));
|
||||
|
||||
const thirdRun = await store.executeCurrentSql("SELECT 3", { tabId: id });
|
||||
expect(thirdRun).toBe(false);
|
||||
expect(mocks.executeMulti).toHaveBeenCalledTimes(1);
|
||||
|
||||
gate.resolve([{ columns: ["value"], rows: [[1]], affected_rows: 0, execution_time_ms: 1 }]);
|
||||
await firstRun;
|
||||
expect(tab.isExecuting).toBe(false);
|
||||
});
|
||||
|
||||
it("cancels result restoration without dispatch and ignores its late payload", async () => {
|
||||
const { store, id, tab, read, snapshot } = await prepareDiskBackedAutoKeptResult();
|
||||
const execution = store.executeCurrentSql("SELECT 2");
|
||||
|
||||
@@ -6502,7 +6502,7 @@ export const useQueryStore = defineStore("query", () => {
|
||||
const executionTabId = options?.tabId ?? activeTabId.value;
|
||||
if (!executionTabId) return;
|
||||
const tab = tabs.value.find((item) => item.id === executionTabId);
|
||||
if (tab && pendingResultRunPreparations.has(tab)) return false;
|
||||
if (tab && (tab.isExecuting || pendingResultRunPreparations.has(tab))) return false;
|
||||
const previousGridKey = tab ? resultGridInstanceKey(tab) : undefined;
|
||||
if (tab?.mode === "query") {
|
||||
tab.resultSortColumn = undefined;
|
||||
|
||||
Reference in New Issue
Block a user