From 7340307a70f6393c7fd33cea752493605c1ff94c Mon Sep 17 00:00:00 2001 From: Wild Karl-Heinz Date: Wed, 2 Sep 2026 16:16:21 +0200 Subject: [PATCH] Add a sheet qualifier: "Blatt!Tabelle::Zelle", table names unique per sheet Table names were document-unique; now they're unique per sheet only -- two different sheets may each hold a table called "Umsatz". A plain, unqualified reference (Umsatz!B2 / Umsatz::B2, unchanged from before) resolves to the table on the SAME sheet as the formula referencing it when the name is otherwise ambiguous, falling back to the single document-wide match when it isn't ambiguous at all -- exactly as before for every existing formula. The new explicit 3-part form (Analyse!Umsatz::B2, quoted where a name has spaces) always targets the named sheet's table regardless of where the formula itself lives, for the case that's still genuinely ambiguous. Engine: parser.ts's afterFirstQualifierToken() disambiguates via lookahead -- "X!Y::" (a name, then "::") is the new 3-part sheet+table form; a bare "X!Y" keeps its original single-qualifier meaning, so every formula written before this exists still parses identically. CellNode/RangeNode gain an optional sheetQualifier field. Workbook.resolveTableName(name, sheetName?) implements the resolution policy; TableModel gains sheetName. precedentsOf() now also sets homeTableId (previously only evaluateCell did), so graph edges use the same same-sheet-preference resolution an evaluation would. Store: table name uniqueness checks (addTable, renameObject) are now scoped to the target sheet instead of the whole document. Point-mode's qualifyRef() inserts the full "Blatt!Tabelle::" form only when the clicked table's name is genuinely ambiguous and lives on a different sheet than the editor; otherwise the simple form, unchanged. 200 tests passing (19 new), vue-tsc and eslint clean, verified live in the running app: renamed a Blatt-2 table to the same name as Analyse's seed "Umsatz", confirmed an unqualified reference resolves per-sheet, an explicit sheet-qualified one always targets the named sheet, and the original sheet's own formulas are unaffected. Co-Authored-By: Claude Sonnet 5 --- README.md | 65 +++++++++++++--- src/engine/README.md | 52 ++++++++++++- src/engine/__tests__/parser.spec.ts | 39 +++++++++- src/engine/__tests__/userFunctions.spec.ts | 9 ++- src/engine/__tests__/workbook.spec.ts | 83 +++++++++++++++++++- src/engine/evaluator.ts | 8 +- src/engine/parser.ts | 88 ++++++++++++++++------ src/engine/types.ts | 28 +++++-- src/engine/workbook.ts | 35 +++++++-- src/store/__tests__/document.spec.ts | 82 ++++++++++++++++++++ src/store/document.ts | 61 +++++++++++---- 11 files changed, 483 insertions(+), 67 deletions(-) diff --git a/README.md b/README.md index ab488d5..4ac3dc2 100644 --- a/README.md +++ b/README.md @@ -13,7 +13,7 @@ Vorbild von **Apple Numbers**. Zwei Dinge stehen im Mittelpunkt: ```bash npm install npm run dev # Vite-Devserver -npm run test # Vitest-Suite (Engine + Store + IO), 181 Tests +npm run test # Vitest-Suite (Engine + Store + IO), 200 Tests npm run build # Typecheck + Produktionsbuild npm run lint # ESLint (flat config) ``` @@ -28,8 +28,10 @@ I/O über File API und Blob-URLs. Wie in Numbers ist ein *Blatt* kein einzelnes Endlos-Raster, sondern eine scroll- und zoombare Fläche, die **Objekte** trägt: -- **Tabellen-Objekt** — ein eigenes Gitter mit dokumentweit eindeutigem Namen. - Frei positionierbar (Ziehen an der Kopfleiste). Zwei Darstellungen: +- **Tabellen-Objekt** — ein eigenes Gitter mit einem Namen, der **pro Blatt** + eindeutig ist (siehe „Formelgrammatik" unten für Bezüge über mehrere Blätter + mit demselben Tabellennamen). Frei positionierbar (Ziehen an der + Kopfleiste). Zwei Darstellungen: - **Automatisch** — so groß wie die Zellen, alle Zeilen sichtbar; der SE-Griff skaliert die Spaltenbreiten proportional. - **Feste Größe** — fester Viewport, **intern scrollbar**, mit frei wählbarer @@ -268,8 +270,16 @@ primary = NUMBER | STRING | "(" expression ")" | funcall | reference ; funcall = IDENT "(" [ expression { ";" expression } ] ")" ; -reference = [ qualifier ] CELL [ ":" CELL ] ; +reference = [ sheetQualifier ] [ qualifier ] CELL [ ":" CELL ] ; qualifier = (IDENT | QUOTED) ("!" | "::") ; (* Tabellenname *) +sheetQualifier = (IDENT | QUOTED) "!" ; (* Blattname -- nur erkannt, + wenn direkt gefolgt von + einem vollen "Tabelle::" + (3-teilige Form); ein + einzelnes "X!Y" ohne + folgendes "::" bleibt + die alte 2-teilige Form + (X = Tabelle) *) IDENT = /[A-Za-z_][A-Za-z0-9_]*/ ; QUOTED = /' ( [^'] | '' )* '/ ; (* Name mit Leerzeichen *) @@ -278,11 +288,32 @@ CELL = /\$?[A-Za-z]{1,3}\$?[0-9]{1,7}/ ; Präzedenz schwach → stark: `+ -` < `* /` < unäres `+ -` < `^`. -Der Qualifier ist der **Tabellenname** (nicht der Blattname). `!` und der -Numbers-Stil `::` sind austauschbar. Tabellennamen werden dokumentweit eindeutig -gehalten und **case-/whitespace-tolerant** aufgelöst: +Der (innere) Qualifier ist der **Tabellenname**. `!` und der Numbers-Stil `::` +sind dabei austauschbar und **case-/whitespace-tolerant**: `Kosten!A2` = `'Kosten'!A2` = `kosten!a2` = `Kosten::A2`. +**Tabellennamen sind nur pro Blatt eindeutig**, nicht dokumentweit — zwei +verschiedene Blätter dürfen je eine Tabelle „Umsatz" haben (beim Anlegen/ +Umbenennen wird nur gegen das **eigene** Blatt geprüft). Ein unqualifizierter +Bezug wie `Umsatz!B2` (ohne Blattangabe) löst sich dann wie folgt auf: + +1. Gibt es die Tabelle nur auf **einem** Blatt (der Normalfall), wird sie + genommen — unverändert wie zuvor. +2. Gibt es sie auf **mehreren** Blättern, gewinnt die Tabelle auf **demselben + Blatt wie die Formel selbst** (nicht: dem gerade angezeigten Blatt in der + UI — das spielt für die Formel keine Rolle). +3. Bleibt es danach noch mehrdeutig, ergibt sich `#REF` — hier hilft die + explizite **3-teilige Form** `Blatt!Tabelle::Zelle`, z. B. + `=Analyse!Umsatz::B2` oder mit Leerzeichen `='Blatt 2'!Umsatz::B2`, die + **immer** genau die Tabelle auf dem genannten Blatt trifft, unabhängig + davon, wo die Formel selbst steht. Die 2-teilige Form (`Umsatz!B2` / + `Umsatz::B2`) bleibt für jede bestehende Formel exakt gültig — die + 3-teilige Form ist rein additiv. + +Die Formel-**Point-Mode**-Einfügung (s. o.) erkennt eine mehrdeutige +Zieltabelle selbst und fügt automatisch die volle 3-teilige Form ein, statt +einer Formel, die versehentlich die falsche Tabelle träfe. + **`$`-Bezüge sperren** (relativ/gemischt/absolut): `A1` (relativ) vs. `$A1` / `A$1` (Spalte bzw. Zeile gesperrt) vs. `$A$1` (beide gesperrt). Für die Auswertung selbst ist `$` irrelevant — sie ist rein positionsbasiert. Es zählt @@ -301,7 +332,10 @@ Bereiche (lokal oder `Name!…`), Zahl-Literale — z. B. `=SUM(Umsatz!B2:E2; 100; A1)`. **Fehlermodell** (propagiert an abhängige Zellen): -`#REF` (ungültiger/gelöschter Bezug oder unbekannter Tabellenname), `#DIV/0!`, +`#REF` (ungültiger/gelöschter Bezug, unbekannter Tabellen-/Blattname, oder ein +unqualifizierter Tabellenname, der auf mehreren Blättern existiert und auch +über die „gleiches Blatt wie die Formel"-Regel nicht eindeutig wird — siehe +oben), `#DIV/0!`, `#NAME?` (unbekannte Funktion, eingebaut oder benutzerdefiniert), `#ERR` (Parse-Fehler; auch eine falsche Argumentanzahl bei einer benutzerdefinierten Funktion), `#CIRC` (Zirkelbezug, auch tabellenübergreifend), `#VALUE!` @@ -411,7 +445,7 @@ Grundrechenarten: `Übersicht!B2 = =SUM(Umsatz!B2:E2)`, `D2 = =B2-C2`, ## Tests -`npm run test` — Vitest, 181 Tests: +`npm run test` — Vitest, 200 Tests: - **lexer / parser** — Präzedenz & Assoziativität (`^` vor `*/` vor `+-`, `^` rechts, `-2^2 = -(2^2)`), lokale/qualifizierte/quoted Bezüge, `!` **und** `::`, @@ -487,6 +521,19 @@ Grundrechenarten: `Übersicht!B2 = =SUM(Umsatz!B2:E2)`, `D2 = =B2-C2`, Löschen lässt bestehende Aufrufe zu `#NAME?` werden, Undo/Redo einer Funktionsänderung, voller JSON-Round-Trip, `customFunctions: []` bei v1→v2-Migration. +- **Blatt-Qualifier „Blatt!Tabelle::Zelle"** — Parser-Ebene: die explizite + 3-teilige Form (bar und quoted, in Bereichen/Funktionsargumenten), volle + Rückwärtskompatibilität der 2-teiligen Form (`X!Y`, `X::Y`, auch ein + zweites `!` statt `::`), Fehler bei fehlender Zelle nach `::`. Workbook- + Ebene: zwei gleichnamige Tabellen auf verschiedenen Blättern lösen ein + unqualifiziertes `Tabelle!Zelle` zum **gleichen Blatt wie die Formel** + auf, die explizite Form trifft immer das genannte Blatt unabhängig vom + Formel-Standort, ein unbekanntes Blatt/eine weiterhin mehrdeutige + Tabelle ergibt `#REF`, Neuberechnung folgt korrekt der Auflösung. Store- + Ebene: Tabellennamen sind nur noch pro Blatt eindeutig (Anlegen/ + Umbenennen prüft nur gegen das eigene Blatt), Point-Mode fügt die volle + 3-teilige Form ein, sobald ein Klick auf eine mehrdeutig benannte Tabelle + auf einem *anderen* Blatt trifft. - **IO** — JSON verlustfreier Round-Trip (identisch), defekte/leere Datei → Fehler ohne Nebenwirkung, unbekannter Tabellenname → `#REF`, v1→v2 beim Import; CSV Übersicht-Export→Re-Import gleiche Zahlen, RFC-4180-Quoting, diff --git a/src/engine/README.md b/src/engine/README.md index c83ccca..6aa5356 100644 --- a/src/engine/README.md +++ b/src/engine/README.md @@ -4,8 +4,11 @@ Selbst geschriebene, framework-agnostische Formel-Engine. Kein HyperFormula, kei `formula-parser`, keine Spreadsheet-Library. Reines ES-Modul, in Vitest getestet. Die adressierbare Einheit ist eine **Tabelle** – ein Tabellen-Objekt auf einer -Numbers-artigen Blattfläche. Tabellennamen sind dokumentweit eindeutig, deshalb -löst `Umsatz!B2` (bzw. `Umsatz::B2`) von jedem Blatt aus identisch auf. +Numbers-artigen Blattfläche. Tabellennamen sind nur **pro Blatt** eindeutig +(zwei Blätter dürfen je eine Tabelle "Umsatz" haben) – `Umsatz!B2` (bzw. +`Umsatz::B2`) löst dann zur Tabelle auf dem **gleichen Blatt wie die Formel** +auf, sofern eindeutig; die explizite Form `Blatt!Tabelle::B2` trifft immer +genau die genannte Tabelle. Siehe "Blatt-Qualifier" weiter unten. ## Module @@ -45,10 +48,18 @@ primary = NUMBER funcall = IDENT "(" [ expression { ";" expression } ] ")" ; -reference = [ qualifier ] CELL [ ":" CELL ] ; +reference = [ sheetQualifier ] [ qualifier ] CELL [ ":" CELL ] ; qualifier = (IDENT | QUOTED) ("!" | "::") ; (* Tabellenname; "!" und der Numbers-Stil "::" sind austauschbar *) +sheetQualifier = (IDENT | QUOTED) "!" ; (* Blattname -- nur erkannt, + wenn direkt gefolgt von + vollem "Tabelle::" (die + 3-teilige Form); ein + einzelnes "X!Y" bleibt + die alte, rückwärts- + kompatible 2-teilige + Form (X = Tabelle) *) NUMBER = /[0-9]+ ("." [0-9]+)? ([eE] [+-]? [0-9]+)?/ | /"." [0-9]+/ ; STRING = /" ... "/ (Backslash-Escapes) ; @@ -81,6 +92,39 @@ eine Zelle aussieht) zusätzlich geprüft wird — ein Treffer erzeugt statt ein verwendet (`userFunctions.ts`); ohne das Argument ist das Verhalten exakt wie zuvor, jeder bare Bezeichner ohne Zellform bleibt ein `ParseError`. +## Blatt-Qualifier ("Blatt!Tabelle::Zelle") + +`parser.ts#Parser.afterFirstQualifierToken` entscheidet per Lookahead, ob ein +`!` einen (alten) Tabellen-Qualifier einleitet oder einen **Blatt**-Qualifier: +nur wenn direkt danach `IDENT/QUOTED "::"` folgt, ist es ein Blattname und der +Parser konsumiert drei Segmente (`Blatt`, `Tabelle`, `Zelle`) statt zwei — +sonst bleibt die alte, einstufige Bedeutung (`X!Y` = Tabelle `X`, Zelle `Y`) +exakt erhalten. `CellNode`/`RangeNode` bekommen dafür ein optionales +`sheetQualifier?: string`-Feld neben dem bestehenden `qualifier`. + +Die Auflösung passiert in `workbook.ts#Workbook.resolveTableName(name, +sheetName?)` (Teil von `EngineHost`): + +- `sheetName` gegeben (3-teilige Form) → nur eine Tabelle mit diesem Namen + **auf diesem Blatt** trifft; sonst `null` (→ `#REF`). +- `sheetName` fehlt und der Tabellenname ist **eindeutig** im Dokument → wie + bisher, unverändert. +- `sheetName` fehlt und der Name kommt auf **mehreren** Blättern vor → + bevorzugt die Tabelle auf demselben Blatt wie `this.homeTableId` (dafür + setzen sowohl `evaluateCell` als auch `precedentsOf` `homeTableId`, bevor + sie referenzieren — Letzteres, damit auch die **Graph-Kanten** beim Parsen + schon dieselbe Auflösung wie später die Auswertung verwenden). Bleibt es + danach mehrdeutig: `null` (→ `#REF`). + +`TableModel` (workbook.ts) trägt dafür zusätzlich `sheetName: string`. Die +Store-Seite (`document.ts#toTableModels`) befüllt es aus dem Blatt, dem die +Tabelle gehört; `tableNamesOnSheet(sheetId)`/`objectNamesOnSheet(sheetId)` +ersetzen dort die früher dokumentweiten Eindeutigkeits-Prüfungen beim Anlegen/ +Umbenennen einer Tabelle. `qualifyRef()` (Point-Mode) fügt die volle +3-teilige Form nur dann ein, wenn der angeklickte Tabellenname wirklich +mehrdeutig **und** auf einem anderen Blatt als der Editor liegt — sonst reicht +weiterhin die einfache Form. + ## Benutzerdefinierte Funktionen Kein Laufzeit-Closure, keine Änderung an `evaluator.ts`/`graph.ts`: ein Aufruf @@ -132,7 +176,7 @@ rufen `rebuild()` für einen vollständigen – trotzdem billigen – Durchlauf. | Wert | Bedeutung | |------------|-----------| -| `#REF` | ungültiger/gelöschter Bezug **oder** unbekannter Tabellenname | +| `#REF` | ungültiger/gelöschter Bezug, unbekannter Tabellen-/Blattname, **oder** ein unqualifizierter Tabellenname, der auf mehreren Blättern existiert und auch über die "gleiches Blatt wie die Formel"-Regel nicht eindeutig wird | | `#DIV/0!` | Division durch 0 (auch `AVG` ohne Zahlen) | | `#NAME?` | unbekannte Funktion (eingebaut oder benutzerdefiniert) | | `#ERR` | Parse-Fehler (`=A1+`, `=SUM(`); auch Arity-Mismatch oder Expansionstiefen-Limit bei einer benutzerdefinierten Funktion | diff --git a/src/engine/__tests__/parser.spec.ts b/src/engine/__tests__/parser.spec.ts index 3a571be..651ba12 100644 --- a/src/engine/__tests__/parser.spec.ts +++ b/src/engine/__tests__/parser.spec.ts @@ -15,9 +15,9 @@ function sexpr(n: Node): string { case 'func': return `(${n.name} ${n.args.map(sexpr).join(' ')})`; case 'cell': - return `${n.qualifier ? n.qualifier + '!' : ''}cell(${n.col},${n.row})`; + return `${n.sheetQualifier ? n.sheetQualifier + '!' : ''}${n.qualifier ? n.qualifier + '!' : ''}cell(${n.col},${n.row})`; case 'range': - return `${n.qualifier ? n.qualifier + '!' : ''}range(${n.start.col},${n.start.row}:${n.end.col},${n.end.row})`; + return `${n.sheetQualifier ? n.sheetQualifier + '!' : ''}${n.qualifier ? n.qualifier + '!' : ''}range(${n.start.col},${n.start.row}:${n.end.col},${n.end.row})`; case 'param': return `param(${n.name})`; } @@ -89,6 +89,41 @@ describe('parser references', () => { }); }); +describe('parser sheet qualifier -- "Blatt!Tabelle::Zelle"', () => { + it('parses the explicit 3-part form, bare and quoted', () => { + expect(parse('=Analyse!Umsatz::B2')).toBe('Analyse!Umsatz!cell(1,1)'); + expect(parse("='Meine Analyse'!Umsatz::B2")).toBe('Meine Analyse!Umsatz!cell(1,1)'); + expect(parse("=Analyse!'Kosten Q3'::B2:E2")).toBe('Analyse!Kosten Q3!range(1,1:4,1)'); + expect(parse("='Meine Analyse'!'Kosten Q3'::B2")).toBe('Meine Analyse!Kosten Q3!cell(1,1)'); + }); + + it('is unaffected in a range end / function call', () => { + expect(parse('=SUM(Analyse!Umsatz::B2:E2; 1)')).toBe('(SUM Analyse!Umsatz!range(1,1:4,1) 1)'); + }); + + it('a bare "X!Y" (no further "::") keeps its original, table-only meaning -- fully backward-compatible', () => { + // no "::" ever follows -> "Analyse" is read as the TABLE, "A1" the cell, + // exactly as parseFormula() behaved before the sheet qualifier existed. + expect(parse('=Analyse!A1')).toBe('Analyse!cell(0,0)'); + expect(parse('=Umsatz!B2')).toBe(parse('=Umsatz!B2')); // unchanged from the existing suite + }); + + it('"X::Y" (no leading sheet) is still the plain table qualifier, never a sheet one', () => { + expect(parse('=Umsatz::B2')).toBe('Umsatz!cell(1,1)'); + }); + + it('"X!Y!Z" (a second "!" instead of "::") does not form a sheet qualifier -- "!" only introduces a sheet name when followed by NAME "::"', () => { + // no "::" anywhere ahead of "Y" -> "X!" is read as an ordinary (old-style) + // table qualifier, leaving "Y" to be parsed as the cell -- which it isn't + // shaped like (no digits) -> ParseError, same as e.g. "=X!Y" always was. + expect(() => parseFormula('=X!Y!Z1')).toThrow(ParseError); + }); + + it('rejects a malformed 3-part reference (missing cell after "::")', () => { + expect(() => parseFormula('=Analyse!Umsatz::')).toThrow(ParseError); + }); +}); + describe('parseFormula(src, paramNames) -- user-defined function bodies', () => { it('a name in paramNames parses as a param reference, case-insensitively', () => { expect(parse2('=revenue - cost', ['revenue', 'cost'])).toBe('(- param(revenue) param(cost))'); diff --git a/src/engine/__tests__/userFunctions.spec.ts b/src/engine/__tests__/userFunctions.spec.ts index f2f0ef9..a660c04 100644 --- a/src/engine/__tests__/userFunctions.spec.ts +++ b/src/engine/__tests__/userFunctions.spec.ts @@ -10,7 +10,12 @@ import { Workbook, type TableModel } from '../workbook'; import { FormulaError, isError, keyOf } from '../types'; // same small A1 helper used by workbook.spec.ts -function sheet(id: string, name: string, cells: Record): TableModel { +function sheet( + id: string, + name: string, + cells: Record, + sheetName = 'Blatt1', +): TableModel { const inputs = new Map(); for (const [a1, raw] of Object.entries(cells)) { const m = /^([A-Z]+)([0-9]+)$/.exec(a1)!; @@ -18,7 +23,7 @@ function sheet(id: string, name: string, cells: Record): TableMo const row = Number(m[2]) - 1; inputs.set(`${col},${row}`, raw); } - return { id, name, cols: 8, rows: 30, inputs }; + return { id, name, sheetName, cols: 8, rows: 30, inputs }; } const val = (wb: Workbook, sheetId: string, a1: string) => { const m = /^([A-Z]+)([0-9]+)$/.exec(a1)!; diff --git a/src/engine/__tests__/workbook.spec.ts b/src/engine/__tests__/workbook.spec.ts index 1566a92..5af0891 100644 --- a/src/engine/__tests__/workbook.spec.ts +++ b/src/engine/__tests__/workbook.spec.ts @@ -2,7 +2,12 @@ import { describe, it, expect } from 'vitest'; import { Workbook, type TableModel } from '../workbook'; import { FormulaError, isError, keyOf } from '../types'; -function sheet(id: string, name: string, cells: Record): TableModel { +function sheet( + id: string, + name: string, + cells: Record, + sheetName = 'Blatt1', +): TableModel { const inputs = new Map(); for (const [a1, raw] of Object.entries(cells)) { const m = /^([A-Z]+)([0-9]+)$/.exec(a1)!; @@ -10,7 +15,7 @@ function sheet(id: string, name: string, cells: Record): TableMo const row = Number(m[2]) - 1; inputs.set(`${col},${row}`, raw); } - return { id, name, cols: 8, rows: 30, inputs }; + return { id, name, sheetName, cols: 8, rows: 30, inputs }; } /** Rebuilds the small "Umsatz / Kosten / Übersicht" fixture from the brief. */ @@ -144,3 +149,77 @@ describe('Workbook recalculation order', () => { expect(pos(keyOf('s-ueber', 3, 1))).toBeLessThan(pos(keyOf('s-ueber', 4, 1))); }); }); + +describe('Workbook sheet qualifier ("Blatt!Tabelle::Zelle") -- table names unique per sheet only', () => { + /** Two sheets ("Analyse", "Zweitblatt") each hold their own table called + * "Umsatz" -- the same table NAME on two different sheets, only possible + * now that table-name uniqueness is per-sheet rather than document-wide. */ + function multiSheetFixture(): Workbook { + const analyseUmsatz = sheet('s-a-umsatz', 'Umsatz', { B2: '100' }, 'Analyse'); + const analyseOther = sheet( + 's-a-other', + 'Auswertung', + { + B2: '=Umsatz!B2', // ambiguous name, home sheet is Analyse -> Analyse's own Umsatz + B3: '=Analyse!Umsatz::B2', // explicit sheet qualifier + B4: '=Zweitblatt!Umsatz::B2', // explicit sheet qualifier, the OTHER sheet's Umsatz + }, + 'Analyse', + ); + const zweitUmsatz = sheet('s-b-umsatz', 'Umsatz', { B2: '999' }, 'Zweitblatt'); + const zweitOther = sheet( + 's-b-other', + 'Check', + { B2: '=Umsatz!B2' }, // ambiguous name, home sheet is Zweitblatt -> Zweitblatt's own Umsatz + 'Zweitblatt', + ); + const drittOther = sheet( + 's-c-other', + 'Fremd', + { + B2: '=Umsatz!B2', // ambiguous, no same-sheet "Umsatz" on Dritt -> #REF + B3: '=Auswertung!B2', // "Auswertung" itself is NOT ambiguous (only on Analyse) -> resolves from anywhere + }, + 'Dritt', + ); + return new Workbook([analyseUmsatz, analyseOther, zweitUmsatz, zweitOther, drittOther]); + } + + it('an unqualified, otherwise-ambiguous name resolves to the table on the SAME sheet as the formula', () => { + const wb = multiSheetFixture(); + expect(val(wb, 's-a-other', 'B2')).toBe(100); + expect(val(wb, 's-b-other', 'B2')).toBe(999); + }); + + it('an explicit "Blatt!Tabelle::Zelle" always resolves to that sheet\'s table, regardless of home sheet', () => { + const wb = multiSheetFixture(); + expect(val(wb, 's-a-other', 'B3')).toBe(100); + expect(val(wb, 's-a-other', 'B4')).toBe(999); + }); + + it('an unqualified ambiguous name with no same-sheet match is #REF', () => { + const wb = multiSheetFixture(); + expect((val(wb, 's-c-other', 'B2') as FormulaError).code).toBe('#REF'); + }); + + it('a non-ambiguous (single-match) name still resolves without any sheet qualifier, from any sheet', () => { + const wb = multiSheetFixture(); + expect(val(wb, 's-c-other', 'B3')).toBe(100); // Auswertung!B2, transitively Analyse's Umsatz!B2 + }); + + it('recalculates correctly through a same-sheet-preferred resolution when the source cell changes', () => { + const wb = multiSheetFixture(); + wb.setCell('s-a-umsatz', 1, 1, '250'); // Analyse's Umsatz B2: 100 -> 250 + expect(val(wb, 's-a-other', 'B2')).toBe(250); + expect(val(wb, 's-a-other', 'B3')).toBe(250); // Auswertung!B2 chained through + expect(val(wb, 's-b-other', 'B2')).toBe(999); // Zweitblatt's own Umsatz, unaffected + }); + + it('an explicit sheet qualifier for an unknown sheet or table is #REF', () => { + const wb2 = new Workbook([ + sheet('s-x', 'X', { B2: '=Nirgendwo!Umsatz::A1' }, 'Analyse'), + sheet('s-a-umsatz', 'Umsatz', { A1: '5' }, 'Analyse'), + ]); + expect((val(wb2, 's-x', 'B2') as FormulaError).code).toBe('#REF'); + }); +}); diff --git a/src/engine/evaluator.ts b/src/engine/evaluator.ts index f846b29..9012f9c 100644 --- a/src/engine/evaluator.ts +++ b/src/engine/evaluator.ts @@ -67,7 +67,9 @@ function evalScalar(node: Node, host: EngineHost): CellValue { case 'cell': { const tableId = - node.qualifier == null ? host.homeTableId : host.resolveTableName(node.qualifier); + node.qualifier == null + ? host.homeTableId + : host.resolveTableName(node.qualifier, node.sheetQualifier ?? null); if (!tableId) return new FormulaError('#REF', `unknown table '${node.qualifier}'`); return host.getComputed(tableId, node.col, node.row); } @@ -104,7 +106,9 @@ function evalScalar(node: Node, host: EngineHost): CellValue { function evalArg(node: Node, host: EngineHost): FormulaError | CellValue[] { if (node.kind === 'range') { const tableId = - node.qualifier == null ? host.homeTableId : host.resolveTableName(node.qualifier); + node.qualifier == null + ? host.homeTableId + : host.resolveTableName(node.qualifier, node.sheetQualifier ?? null); if (!tableId) return new FormulaError('#REF', `unknown table '${node.qualifier}'`); return expandRange(node.start, node.end).map((c) => host.getComputed(tableId, c.col, c.row)); } diff --git a/src/engine/parser.ts b/src/engine/parser.ts index 1d9b986..c887b5b 100644 --- a/src/engine/parser.ts +++ b/src/engine/parser.ts @@ -22,10 +22,22 @@ * * funcall = IDENT "(" [ expression { ";" expression } ] ")" ; * - * reference = [ qualifier ] CELL [ ":" CELL ] ; + * reference = [ sheetQualifier ] [ qualifier ] CELL [ ":" CELL ] ; * qualifier = (IDENT | QUOTED) ("!" | "::") ; (* table name; "!" and the * Numbers-style "::" are * interchangeable *) + * sheetQualifier = (IDENT | QUOTED) "!" ; (* sheet name -- only + * recognised when it is + * immediately followed by + * a full "table::" -- + * i.e. only in the 3-part + * form "Blatt!Tabelle::A1". + * A bare "X!Y" with no + * "::" after Y keeps its + * original meaning: X is + * the TABLE qualifier + * (backward-compatible -- + * see reference() below). *) * CELL = /\$?[A-Za-z]{1,3}\$?[0-9]{1,7}/ ; * * Precedence, lowest to highest: + - < * / < unary +/- < ^ @@ -82,6 +94,11 @@ export interface CellNode { kind: 'cell'; /** raw table name as written ("Umsatz"), or null for "this table" */ qualifier: string | null; + /** raw sheet name as written ("Analyse") -- only set for the explicit + * "Blatt!Tabelle::Zelle" 3-part form; absent for every plain + * "Tabelle!Zelle" / "Tabelle::Zelle" reference (resolved without regard + * to sheet, same as before this existed). */ + sheetQualifier?: string; col: number; row: number; } @@ -89,6 +106,8 @@ export interface CellNode { export interface RangeNode { kind: 'range'; qualifier: string | null; + /** see {@link CellNode.sheetQualifier}. */ + sheetQualifier?: string; start: { col: number; row: number }; end: { col: number; row: number }; } @@ -190,10 +209,11 @@ class Parser { return inner; } if (t.type === 'QUOTED') { - // must be a table-qualified reference: 'Kosten Q3'!B2 or 'Kosten Q3'::B2 + // must be a qualified reference -- afterFirstQualifierToken() figures + // out whether it's a table qualifier ('Kosten Q3'!B2) or a sheet + // qualifier starting a 3-part ('Analyse'!Umsatz::B2) reference. this.next(); - this.expectQualifierSep(); - return this.reference(t.value); + return this.afterFirstQualifierToken(t.value); } if (t.type === 'IDENT') { this.next(); @@ -211,23 +231,58 @@ class Parser { this.expect('RPAREN'); return { kind: 'func', name: t.value, args }; } - // table-qualified reference? IDENT ("!" | "::") ... + // qualified reference? IDENT ("!" | "::") ... if (this.at('BANG') || this.at('DBLCOLON')) { - this.next(); - return this.reference(t.value); + return this.afterFirstQualifierToken(t.value); } // otherwise it must be a local cell reference - return this.reference(null, t); + return this.reference(null, null, t); } throw new ParseError(`Unexpected token ${t.type} at ${t.pos}`); } + /** + * Called right after consuming the first IDENT/QUOTED of what is *some* + * kind of qualified reference (current token is known to be "!" or "::"). + * Disambiguates the two qualifier grammars: + * - "X::..." or a bare "X!..." (not followed by another name + "::") is + * the original, single-level TABLE qualifier -- unchanged from before + * this existed, so every formula written before the sheet qualifier + * was added still parses exactly the same. + * - "X!Y::..." -- when "!" is immediately followed by another + * IDENT/QUOTED and *that* is followed by "::" -- is the 3-part form: + * X is the SHEET, Y is the TABLE. + */ + private afterFirstQualifierToken(first: string): Node { + if (this.at('BANG') && this.looksLikeSheetThenTable()) { + this.next(); // consume "!" + const tableTok = this.next(); // IDENT or QUOTED, guaranteed by the lookahead above + this.next(); // consume "::" + return this.reference(tableTok.value, first); + } + if (this.at('BANG') || this.at('DBLCOLON')) { + this.next(); + return this.reference(first, null); + } + throw new ParseError(`Expected '!' or '::' but got ${this.peek().type} at ${this.peek().pos}`); + } + + /** True iff, with the current token being "!", the shape ahead is + * IDENT/QUOTED "::" -- i.e. this "!" really introduces a SHEET name + * followed by a table qualifier, not a bare (old-style) table qualifier + * of its own. Pure lookahead, consumes nothing. */ + private looksLikeSheetThenTable(): boolean { + const n1 = this.toks[this.i + 1]; + const n2 = this.toks[this.i + 2]; + return !!n1 && (n1.type === 'IDENT' || n1.type === 'QUOTED') && !!n2 && n2.type === 'DBLCOLON'; + } + /** * Parse the cell/range part of a reference. `firstTok` (when given) is an * IDENT already consumed by the caller for the *local* case. */ - private reference(qualifier: string | null, firstTok?: Token): Node { + private reference(qualifier: string | null, sheetQualifier: string | null, firstTok?: Token): Node { const a = firstTok ?? this.expectCellIdent(); if (!firstTok && a.type !== 'IDENT') { throw new ParseError(`Expected cell reference at ${a.pos}`); @@ -243,6 +298,7 @@ class Parser { throw new ParseError(`'${a.value}' is not a valid cell reference (pos ${a.pos})`); } const start = parseA1(a.value); + const sheetField = sheetQualifier === null ? {} : { sheetQualifier }; if (this.at('COLON')) { this.next(); @@ -251,19 +307,9 @@ class Parser { throw new ParseError(`'${b.value}' is not a valid cell reference (pos ${b.pos})`); } const end = parseA1(b.value); - return { kind: 'range', qualifier, start, end }; + return { kind: 'range', qualifier, ...sheetField, start, end }; } - return { kind: 'cell', qualifier, col: start.col, row: start.row }; - } - - private expectQualifierSep(): void { - if (this.at('BANG') || this.at('DBLCOLON')) { - this.next(); - return; - } - throw new ParseError( - `Expected '!' or '::' but got ${this.peek().type} at ${this.peek().pos}`, - ); + return { kind: 'cell', qualifier, ...sheetField, col: start.col, row: start.row }; } private expectCellIdent(): Token { diff --git a/src/engine/types.ts b/src/engine/types.ts index 7d350fd..7ea08e1 100644 --- a/src/engine/types.ts +++ b/src/engine/types.ts @@ -70,19 +70,33 @@ export function parseKey(key: string): CellRef { * The document as seen by the engine. Implemented by the Vue store. * * The addressable unit is a **table** (a table object on a Numbers-style - * canvas). Table names are unique across the whole document, so a qualified - * reference like `Umsatz!B2` / `Umsatz::B2` resolves the same no matter which - * sheet the formula or the target table sits on. + * canvas). Table names are only unique *per sheet* -- two different sheets + * may each hold a table called "Umsatz". An unqualified table reference + * (`Umsatz!B2` / `Umsatz::B2`, no sheet given) resolves to whichever table + * named "Umsatz" lives on the *same sheet* as the formula doing the + * referencing, falling back to the single document-wide match when the name + * isn't ambiguous there. The explicit 3-part form (`Analyse!Umsatz::B2`) + * disambiguates when it would otherwise be genuinely ambiguous (name used on + * two *other* sheets, neither of them the formula's own). */ export interface EngineHost { /** Id of the table the formula currently being evaluated lives on. */ homeTableId: string; /** - * Resolve a table *name* (as written in a formula) to its id. Matching is - * case-insensitive and tolerant of surrounding whitespace. Returns `null` - * for an unknown / deleted table. + * Resolve a table *name* (as written in a formula) to its id, optionally + * narrowed to a specific *sheet* name (the "Blatt!" part of a 3-part + * `Blatt!Tabelle::Zelle` reference). Matching is case-insensitive and + * tolerant of surrounding whitespace. + * + * - `sheetName` given: only a table with that name **on that sheet** + * matches; otherwise `null` (including when the sheet itself is unknown). + * - `sheetName` omitted/`null`: matches a table by name anywhere in the + * document. Exactly one match resolves as before. Multiple matches (the + * name exists on more than one sheet) prefer a table on the same sheet + * as {@link homeTableId}; if that still doesn't narrow it to one, `null` + * (`#REF` -- the formula needs an explicit sheet qualifier). */ - resolveTableName(name: string): string | null; + resolveTableName(name: string, sheetName?: string | null): string | null; /** Dimensions of a table, used for range clamping and #REF detection. */ tableSize(tableId: string): { cols: number; rows: number } | null; /** diff --git a/src/engine/workbook.ts b/src/engine/workbook.ts index 470af4d..7f8ec6a 100644 --- a/src/engine/workbook.ts +++ b/src/engine/workbook.ts @@ -35,8 +35,12 @@ import { evaluate } from './evaluator'; export interface TableModel { id: string; - /** document-unique table name, used as the qualifier in formulas */ + /** table name, used as the qualifier in formulas -- unique *per sheet* + * only; the same name may occur on more than one sheet (see + * `sheetName` and {@link EngineHost.resolveTableName}). */ name: string; + /** name of the sheet this table lives on. */ + sheetName: string; cols: number; rows: number; /** key `${col},${row}` -> raw user input. Absent = empty cell. */ @@ -78,10 +82,26 @@ export class Workbook implements EngineHost { // ---- EngineHost --------------------------------------------------------- - resolveTableName(name: string): string | null { + resolveTableName(name: string, sheetName?: string | null): string | null { const needle = name.trim().toLowerCase(); - const hit = this.tables.find((t) => t.name.trim().toLowerCase() === needle); - return hit ? hit.id : null; + const candidates = this.tables.filter((t) => t.name.trim().toLowerCase() === needle); + if (candidates.length === 0) return null; + + if (sheetName != null) { + const sheetNeedle = sheetName.trim().toLowerCase(); + const hit = candidates.find((t) => t.sheetName.trim().toLowerCase() === sheetNeedle); + return hit ? hit.id : null; + } + if (candidates.length === 1) return candidates[0].id; + + // ambiguous (same name on more than one sheet) -- prefer a table on the + // same sheet as the formula doing the referencing, if there is one + const home = this.tableById(this.homeTableId); + if (home) { + const sameSheet = candidates.find((t) => t.sheetName === home.sheetName); + if (sameSheet) return sameSheet.id; + } + return null; // still ambiguous -> #REF, needs an explicit sheet qualifier } tableSize(tableId: string): { cols: number; rows: number } | null { @@ -167,9 +187,14 @@ export class Workbook implements EngineHost { /** Resolve every reference in `ast` to concrete cell keys (home = tableId). */ private precedentsOf(ast: Node, tableId: string): string[] { + // resolveTableName()'s "prefer the home sheet" ambiguity fallback reads + // this.homeTableId -- set it here too (not just in evaluateCell) so graph + // edges are wired using the *same* resolution an evaluation would use. + this.homeTableId = tableId; const out: string[] = []; for (const ref of collectRefs(ast)) { - const target = ref.qualifier == null ? tableId : this.resolveTableName(ref.qualifier); + const target = + ref.qualifier == null ? tableId : this.resolveTableName(ref.qualifier, ref.sheetQualifier ?? null); if (!target) continue; // unknown table -> no edge; evaluator emits #REF if (ref.kind === 'cell') { out.push(keyOf(target, ref.col, ref.row)); diff --git a/src/store/__tests__/document.spec.ts b/src/store/__tests__/document.spec.ts index 610ed8b..234af9e 100644 --- a/src/store/__tests__/document.spec.ts +++ b/src/store/__tests__/document.spec.ts @@ -1047,6 +1047,88 @@ describe('formula "point mode" (click/drag a cell to insert a reference)', () => s.registerFormulaEditor(null); expect(s.formulaPointModeActive()).toBe(false); }); + + it('a click on a same-named table on a DIFFERENT sheet inserts the full "Blatt!Tabelle::" form', () => { + const s = fresh(); + const home = s.addTable(s.doc.activeSheetId); // lives on "Analyse" + const blatt2 = s.doc.sheets[1].id; // "Blatt 2" + const other = s.addTable(blatt2); + s.renameObject(other, 'Umsatz'); // now ambiguous: Analyse already has an "Umsatz" too + const ed = mockEditor(home, '='); + s.registerFormulaEditor(ed.handle); + s.pointClick(other, 0, 0); + expect(ed.text).toBe("='Blatt 2'!Umsatz::A1"); // plain "Umsatz!A1" would be ambiguous + }); +}); + +describe('cross-sheet formulas: sheet qualifier + per-sheet table name uniqueness', () => { + it('renaming a table to a name already used on a DIFFERENT sheet is now allowed', () => { + const s = fresh(); + const blatt2 = s.doc.sheets[1].id; + const id = s.addTable(blatt2); + s.renameObject(id, 'Umsatz'); // "Umsatz" already exists, but on the OTHER (Analyse) sheet + expect(s.tableById(id)!.name).toBe('Umsatz'); + }); + + it('renaming a table to a name already used on the SAME sheet still gets uniquified', () => { + const s = fresh(); + const id = s.addTable(s.doc.activeSheetId); // Analyse already has an "Umsatz" + s.renameObject(id, 'Umsatz'); + expect(s.tableById(id)!.name).toBe('Umsatz 2'); + }); + + it("inserting a new table defaults its name against only its OWN sheet's objects", () => { + const s = fresh(); + const blatt2 = s.doc.sheets[1].id; + // Blatt 2 starts empty -- its first new table can be plain "Tabelle", + // unaffected by Analyse already having several named objects. + const id = s.addTable(blatt2); + expect(s.tableById(id)!.name).toBe('Tabelle'); + }); + + it('an unqualified, cross-sheet-ambiguous table name resolves to the one on the SAME sheet as the formula', () => { + const s = fresh(); + const analyse = s.doc.activeSheetId; // capture before addTable()'s activeSheetId side effect + const blatt2 = s.doc.sheets[1].id; + const dupId = s.addTable(blatt2); + s.renameObject(dupId, 'Umsatz'); // now two tables named "Umsatz", one per sheet + s.setCellInput(dupId, 1, 1, '777'); // Blatt 2's own Umsatz!B2 + + const blatt2RefId = s.addTable(blatt2); + s.setCellInput(blatt2RefId, 0, 0, '=Umsatz!B2'); + expect(s.cellValue(blatt2RefId, 0, 0)).toBe(777); // resolves to Blatt 2's own Umsatz + + const analyseRefId = s.addTable(analyse); + s.setCellInput(analyseRefId, 0, 0, '=Umsatz!B2'); + expect(s.cellValue(analyseRefId, 0, 0)).toBe(1200); // resolves to Analyse's (seed) Umsatz + }); + + it('an explicit "Blatt!Tabelle::Zelle" reference always targets that sheet\'s table', () => { + const s = fresh(); + const analyse = s.doc.activeSheetId; // capture before addTable()'s activeSheetId side effect + const blatt2 = s.doc.sheets[1].id; + const dupId = s.addTable(blatt2); + s.renameObject(dupId, 'Umsatz'); + s.setCellInput(dupId, 1, 1, '777'); // Blatt 2's Umsatz!B2 + + const refId = s.addTable(analyse); // lives on Analyse + s.setCellInput(refId, 0, 0, "='Blatt 2'!Umsatz::B2"); // explicit -> Blatt 2's, not Analyse's own + expect(s.cellValue(refId, 0, 0)).toBe(777); + s.setCellInput(refId, 0, 1, '=Analyse!Umsatz::B2'); // explicit -> Analyse's own + expect(s.cellValue(refId, 0, 1)).toBe(1200); + }); + + it('deleting the sheet that owned one of two same-named tables leaves the other resolvable unqualified', () => { + const s = fresh(); + const analyse = s.doc.activeSheetId; // capture before addTable()'s activeSheetId side effect + const blatt2 = s.doc.sheets[1].id; + const dupId = s.addTable(blatt2); + s.renameObject(dupId, 'Umsatz'); + s.deleteSheet(blatt2); // the ambiguity is gone -- only Analyse's Umsatz remains + const refId = s.addTable(analyse); + s.setCellInput(refId, 0, 0, '=Umsatz!B2'); + expect(s.cellValue(refId, 0, 0)).toBe(1200); + }); }); describe('text box objects', () => { diff --git a/src/store/document.ts b/src/store/document.ts index fe63bd5..e323182 100644 --- a/src/store/document.ts +++ b/src/store/document.ts @@ -72,13 +72,18 @@ function allTables(doc: DocumentState): TableObject[] { } function toTableModels(doc: DocumentState): TableModel[] { - return allTables(doc).map((t) => ({ - id: t.id, - name: t.name, - cols: t.cols, - rows: t.rows, - inputs: new Map(Object.entries(t.cells)), - })); + return doc.sheets.flatMap((s) => + s.objects + .filter((o): o is TableObject => o.kind === 'table') + .map((t) => ({ + id: t.id, + name: t.name, + sheetName: s.name, + cols: t.cols, + rows: t.rows, + inputs: new Map(Object.entries(t.cells)), + })), + ); } function toFunctionDefs(doc: DocumentState): FunctionDef[] { @@ -376,10 +381,16 @@ function createStore() { while (taken.has(`${b} ${i}`.toLowerCase())) i++; return `${b} ${i}`; } - const tableNames = () => new Set(allTables(doc).map((t) => t.name.toLowerCase())); const objectNames = () => new Set(doc.sheets.flatMap((s) => s.objects).map((o) => o.name.toLowerCase())); const sheetNames = () => new Set(doc.sheets.map((s) => s.name.toLowerCase())); + /** Table names are unique *per sheet* only -- two different sheets may + * each hold a table called "Umsatz" (see engine/types.ts's EngineHost + * doc comment for how a formula then resolves it). */ + const tableNamesOnSheet = (sheetId: string) => + new Set(tablesOf(sheetId).map((t) => t.name.toLowerCase())); + const objectNamesOnSheet = (sheetId: string) => + new Set((sheetById(sheetId)?.objects ?? []).map((o) => o.name.toLowerCase())); // ---- lookups -------------------------------------------------- const sheetById = (id: string): Sheet | undefined => doc.sheets.find((s) => s.id === id); @@ -514,7 +525,7 @@ function createStore() { const table: TableObject = { kind: 'table', id, - name: uniqueName('Tabelle', new Set([...tableNames(), ...objectNames()])), + name: uniqueName('Tabelle', new Set([...tableNamesOnSheet(sheetId), ...objectNamesOnSheet(sheetId)])), x: Math.max(0, Math.round(p.x)), y: Math.max(0, Math.round(p.y)), rows: 12, @@ -608,8 +619,12 @@ function createStore() { const clean = name.trim(); if (!o || !clean) return; if (o.kind === 'table') { + // uniqueness is per sheet only -- see engine/types.ts's EngineHost + const sheetId = sheetIdOfObject(objId); const others = new Set( - allTables(doc).filter((t) => t.id !== objId).map((t) => t.name.toLowerCase()), + (sheetId ? tablesOf(sheetId) : allTables(doc)) + .filter((t) => t.id !== objId) + .map((t) => t.name.toLowerCase()), ); o.name = others.has(clean.toLowerCase()) ? uniqueName(clean, others) : clean; syncWorkbook(); // formulas referencing the old/new name must re-resolve @@ -1366,11 +1381,31 @@ function createStore() { function formulaPointModeActive(): boolean { return !!formulaEditor.value && formulaEditor.value.getState().text.startsWith('='); } + /** `'` needs quoting whenever a name isn't a bare identifier (spaces, + * punctuation, ...) -- shared by the table- and sheet-qualifier cases. */ + function quoteQualifier(name: string): string { + return /^[A-Za-z_][A-Za-z0-9_]*$/.test(name) ? name : `'${name.replace(/'/g, "''")}'`; + } function qualifyRef(homeTableId: string, targetTableId: string, body: string): string { if (targetTableId === homeTableId) return body; - const name = tableById(targetTableId)?.name ?? targetTableId; - const qualifier = /^[A-Za-z_][A-Za-z0-9_]*$/.test(name) ? name : `'${name.replace(/'/g, "''")}'`; - return `${qualifier}!${body}`; + const target = tableById(targetTableId); + const tableQ = quoteQualifier(target?.name ?? targetTableId); + // a plain "Tabelle!..." is enough as long as it resolves unambiguously -- + // either the name is document-unique, or the target happens to be on the + // same sheet as the formula being edited (resolveTableName()'s "prefer + // the home sheet" fallback picks it correctly either way); only a name + // that collides *and* lives on a different sheet needs the explicit + // "Blatt!Tabelle::..." 3-part form to disambiguate. + const homeSheetId = sheetIdOfObject(homeTableId); + const targetSheetId = sheetIdOfObject(targetTableId); + const nameCollides = target + ? allTables(doc).filter((t) => t.name.toLowerCase() === target.name.toLowerCase()).length > 1 + : false; + if (!nameCollides || targetSheetId === homeSheetId) { + return `${tableQ}!${body}`; + } + const sheetName = (targetSheetId && sheetById(targetSheetId)?.name) || ''; + return `${quoteQualifier(sheetName)}!${tableQ}::${body}`; } function applyPointRef(targetTableId: string, body: string): boolean { const ed = formulaEditor.value;