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 <noreply@anthropic.com>
This commit is contained in:
@@ -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', () => {
|
||||
|
||||
+48
-13
@@ -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;
|
||||
|
||||
Reference in New Issue
Block a user