From 2e85c8b55d62ddc5cdcedd56e0a1a0244a859917 Mon Sep 17 00:00:00 2001 From: zipg Date: Fri, 18 Sep 2026 01:37:02 +0800 Subject: [PATCH] fix(query): use adjacent comments for result labels --- .../__tests__/sql/queryResultSource.spec.ts | 24 ++++++++++------- apps/desktop/src/lib/sql/queryResultSource.ts | 26 +++++++++---------- packages/app-tests/queryStore.test.ts | 6 ++++- 3 files changed, 31 insertions(+), 25 deletions(-) diff --git a/apps/desktop/src/lib/__tests__/sql/queryResultSource.spec.ts b/apps/desktop/src/lib/__tests__/sql/queryResultSource.spec.ts index 73e828cae6..3c8dc62be6 100644 --- a/apps/desktop/src/lib/__tests__/sql/queryResultSource.spec.ts +++ b/apps/desktop/src/lib/__tests__/sql/queryResultSource.spec.ts @@ -2,21 +2,25 @@ import { describe, expect, it } from "vitest"; import { queryResultNameFromPreamble, queryResultSourceLabel } from "@/lib/sql/queryResultSource"; describe("queryResultNameFromPreamble", () => { - it("uses the nearest non-empty Name line comment", () => { - expect(queryResultNameFromPreamble("-- Name: Old name\n-- unrelated\r\n -- NAME : Latest name \r\n")).toBe("Latest name"); - expect(queryResultNameFromPreamble("-- Name: kept\n-- Name: \n")).toBe("kept"); - expect(queryResultNameFromPreamble("-- Name: explicit\n-- nearest ordinary comment\n")).toBe("explicit"); + it("uses only the immediately preceding line comment", () => { + expect(queryResultNameFromPreamble("-- Name: Latest name\r\n")).toBe("Latest name"); + expect(queryResultNameFromPreamble("-- unrelated\n")).toBe("unrelated"); + expect(queryResultNameFromPreamble("-- indented\n ")).toBe("indented"); + expect(queryResultNameFromPreamble("-- Name: Old name\n-- unrelated\n")).toBe("unrelated"); + expect(queryResultNameFromPreamble("-- Name: explicit\n-- \n")).toBeUndefined(); }); - it("falls back to the nearest non-empty line comment", () => { - expect(queryResultNameFromPreamble("-- older comment\n/*\n-- Name: block\n*/\n-- 当前时间\n")).toBe("当前时间"); - expect(queryResultNameFromPreamble("/* -- block only */\n-- \n")).toBeUndefined(); + it("stops at a blank line and ignores non-line comments", () => { + expect(queryResultNameFromPreamble("-- older comment\n\n")).toBeUndefined(); + expect(queryResultNameFromPreamble("/* Name: block */\n")).toBeUndefined(); + expect(queryResultNameFromPreamble("-- earlier comment\n/* block */\n")).toBeUndefined(); }); it("accepts hash comments for MySQL without treating them as portable SQL comments", () => { - expect(queryResultNameFromPreamble("# MySQL report\nSELECT 1", { databaseType: "mysql" })).toBe("MySQL report"); - expect(queryResultNameFromPreamble("# temporary table\nSELECT 1", { databaseType: "sqlserver" })).toBeUndefined(); - expect(queryResultNameFromPreamble("# portable comment\nSELECT 1")).toBeUndefined(); + expect(queryResultNameFromPreamble("# MySQL report\n", { databaseType: "mysql" })).toBe("MySQL report"); + expect(queryResultNameFromPreamble("# temporary table\n", { databaseType: "sqlserver" })).toBeUndefined(); + expect(queryResultNameFromPreamble("# portable comment\n")).toBeUndefined(); + expect(queryResultNameFromPreamble("# MySQL report\n\n", { databaseType: "mysql" })).toBeUndefined(); }); }); diff --git a/apps/desktop/src/lib/sql/queryResultSource.ts b/apps/desktop/src/lib/sql/queryResultSource.ts index 2e4682de7c..5d9ab70e8c 100644 --- a/apps/desktop/src/lib/sql/queryResultSource.ts +++ b/apps/desktop/src/lib/sql/queryResultSource.ts @@ -10,21 +10,19 @@ export interface QueryResultSourceLabelOptions { const HASH_COMMENT_DATABASE_TYPES = new Set(["mysql"]); export function queryResultNameFromPreamble(preamble: string, options: Pick = {}): string | undefined { - let name: string | undefined; - let fallback: string | undefined; - const withoutBlockComments = preamble.replace(/\/\*[\s\S]*?\*\//g, ""); - for (const line of withoutBlockComments.split(/\r?\n/)) { - const commentMatch = line.match(/^\s*(--|#)\s*(.*)$/); - if (commentMatch?.[1] === "#" && !HASH_COMMENT_DATABASE_TYPES.has(options.databaseType!)) continue; - const comment = commentMatch?.[2]?.trim(); - if (!comment) continue; + const beforeStatementLine = preamble.replace(/[ \t]*$/, ""); + const previousLine = + beforeStatementLine + .replace(/\r?\n$/, "") + .split(/\r?\n/) + .pop() ?? ""; + const commentMatch = previousLine.match(/^\s*(--|#)\s*(.*)$/); + if (commentMatch?.[1] === "#" && !HASH_COMMENT_DATABASE_TYPES.has(options.databaseType!)) return undefined; + const comment = commentMatch?.[2]?.trim(); + if (!comment) return undefined; - const nameMatch = comment.match(/^name\s*:\s*(.*)$/i); - const candidate = nameMatch?.[1]?.trim(); - if (candidate) name = candidate; - else if (!nameMatch) fallback = comment; - } - return name ?? fallback; + const nameMatch = comment.match(/^name\s*:\s*(.*)$/i); + return nameMatch ? nameMatch[1]?.trim() || undefined : comment; } function firstSourceOfKind(sources: SqlSemanticRowSource[], kind: SqlSemanticRowSource["kind"]): SqlSemanticRowSource | undefined { diff --git a/packages/app-tests/queryStore.test.ts b/packages/app-tests/queryStore.test.ts index 3ce0f99eb2..36ff96e19e 100644 --- a/packages/app-tests/queryStore.test.ts +++ b/packages/app-tests/queryStore.test.ts @@ -9088,7 +9088,7 @@ test("query results keep readable table source labels with active database conte } }); -test("cursor execution uses the preceding comment from the full query draft", async () => { +test("cursor execution uses only the immediately preceding comment from the full query draft", async () => { const restoreStorage = installMemoryStorage(); setActivePinia(createPinia()); const connectionStore = useConnectionStore(); @@ -9123,6 +9123,10 @@ test("cursor execution uses the preceding comment from the full query draft", as await store.executeTabSql(tabId, selectedSql, { sourceOffset: tab.sql.indexOf(selectedSql) }); assert.equal(currentSql, selectedSql); assert.equal(tab.result?.sourceLabel, "Orders report"); + + tab.sql = "-- Users report\nSELECT 1;\n\n-- Orders report\n\nSELECT 2;"; + await store.executeTabSql(tabId, selectedSql, { sourceOffset: tab.sql.indexOf(selectedSql) }); + assert.equal(tab.result?.sourceLabel, undefined); } finally { globalThis.fetch = originalFetch; restoreStorage();