diff --git a/app/client/components/ChartView.ts b/app/client/components/ChartView.ts index e5eb7fb7..6e5f47a8 100644 --- a/app/client/components/ChartView.ts +++ b/app/client/components/ChartView.ts @@ -897,7 +897,7 @@ export const chartTypes: {[name: string]: ChartFunc} = { function format(val: number) { if (dataOptions.totalFormatter) { - return dataOptions.totalFormatter.format(val); + return dataOptions.totalFormatter.formatAny(val); } return String(val); } diff --git a/app/client/components/CopySelection.ts b/app/client/components/CopySelection.ts index 66239b5f..393fae5c 100644 --- a/app/client/components/CopySelection.ts +++ b/app/client/components/CopySelection.ts @@ -33,7 +33,7 @@ export class CopySelection { this.rowStyle = options.rowStyle; this.colStyle = options.colStyle; this.columns = fields.map((f, i) => { - const formatter = f.visibleColFormatter(); + const formatter = f.formatter(); const _fmtGetter = tableData.getRowPropFunc(this.displayColIds[i])!; const _rawGetter = tableData.getRowPropFunc(this.colIds[i])!; diff --git a/app/client/lib/ReferenceUtils.ts b/app/client/lib/ReferenceUtils.ts index cafbbaf5..5b393ccd 100644 --- a/app/client/lib/ReferenceUtils.ts +++ b/app/client/lib/ReferenceUtils.ts @@ -11,7 +11,7 @@ import {BaseFormatter} from 'app/common/ValueFormatter'; export class ReferenceUtils { public readonly refTableId: string; public readonly tableData: TableData; - public readonly formatter: BaseFormatter; + public readonly visibleColFormatter: BaseFormatter; public readonly visibleColModel: ColumnRec; public readonly visibleColId: string; public readonly isRefList: boolean; @@ -30,7 +30,7 @@ export class ReferenceUtils { } this.tableData = tableData; - this.formatter = field.visibleColFormatter(); + this.visibleColFormatter = field.visibleColFormatter(); this.visibleColModel = field.visibleColModel(); this.visibleColId = this.visibleColModel.colId() || 'id'; this.isRefList = isRefListType(colType); @@ -38,13 +38,13 @@ export class ReferenceUtils { public idToText(value: unknown) { if (typeof value === 'number') { - return this.formatter.formatAny(this.tableData.getValue(value, this.visibleColId)); + return this.visibleColFormatter.formatAny(this.tableData.getValue(value, this.visibleColId)); } return String(value || ''); } public autocompleteSearch(text: string) { - const acIndex = this.tableData.columnACIndexes.getColACIndex(this.visibleColId, this.formatter); + const acIndex = this.tableData.columnACIndexes.getColACIndex(this.visibleColId, this.visibleColFormatter); return acIndex.search(text); } } diff --git a/app/client/models/SearchModel.ts b/app/client/models/SearchModel.ts index bc9cd6a2..e9aa6cf4 100644 --- a/app/client/models/SearchModel.ts +++ b/app/client/models/SearchModel.ts @@ -218,7 +218,7 @@ class FinderImpl implements IFinder { this._sectionTableData = tableModel.tableData; this._fieldStepper.array = section.viewFields().peek(); - this._fieldFormatters = this._fieldStepper.array.map(f => f.visibleColFormatter.peek()); + this._fieldFormatters = this._fieldStepper.array.map(f => f.formatter.peek()); return tableModel; } diff --git a/app/client/models/entities/ColumnRec.ts b/app/client/models/entities/ColumnRec.ts index 9439be3f..00198f34 100644 --- a/app/client/models/entities/ColumnRec.ts +++ b/app/client/models/entities/ColumnRec.ts @@ -2,7 +2,7 @@ import {KoArray} from 'app/client/lib/koArray'; import {DocModel, IRowModel, recordSet, refRecord, TableRec, ViewFieldRec} from 'app/client/models/DocModel'; import {jsonObservable, ObjObservable} from 'app/client/models/modelUtil'; import * as gristTypes from 'app/common/gristTypes'; -import {getReferencedTableId} from 'app/common/gristTypes'; +import {getReferencedTableId, isFullReferencingType} from 'app/common/gristTypes'; import {BaseFormatter, createFormatter} from 'app/common/ValueFormatter'; import * as ko from 'knockout'; @@ -56,6 +56,13 @@ export interface ColumnRec extends IRowModel<"_grist_Tables_column"> { // to the visibleCol associated with column. visibleColFormatter: ko.Computed; + // A formatter for values of this column. + // The difference between visibleColFormatter and formatter is especially important for ReferenceLists: + // `visibleColFormatter` is for individual elements of a list, sometimes hypothetical + // (i.e. they aren't actually referenced but they exist in the visible column and are relevant to e.g. autocomplete) + // `formatter` formats actual cell values, e.g. a whole list from the display column. + formatter: ko.Computed; + // Helper which adds/removes/updates column's displayCol to match the formula. saveDisplayFormula(formula: string): Promise|undefined; } @@ -118,6 +125,8 @@ export function createColumnRec(this: ColumnRec, docModel: DocModel): void { // Helper for Reference/ReferenceList columns, which returns a formatter according to the visibleCol // associated with this column. If no visible column available, return formatting for the column itself. this.visibleColFormatter = ko.pureComputed(() => visibleColFormatterForRec(this, this, docModel)); + + this.formatter = ko.pureComputed(() => formatterForRec(this, this, docModel, this.visibleColFormatter())); } export function visibleColFormatterForRec( @@ -125,7 +134,28 @@ export function visibleColFormatterForRec( ): BaseFormatter { const vcol = rec.visibleColModel(); const documentSettings = docModel.docInfoRow.documentSettingsJson(); - return (vcol.getRowId() !== 0) ? - createFormatter(vcol.type(), vcol.widgetOptionsJson(), documentSettings) : - createFormatter(colRec.type(), rec.widgetOptionsJson(), documentSettings); + const type = colRec.type(); + if (isFullReferencingType(type)) { + if (vcol.getRowId() === 0) { + // This column displays the Row ID, e.g. Table1[2] + // referencedTableId may actually be empty if the table is hidden + const referencedTableId: string = colRec.refTable()?.tableId() || ""; + return createFormatter('Id', {tableId: referencedTableId}, documentSettings); + } else { + return createFormatter(vcol.type(), vcol.widgetOptionsJson(), documentSettings); + } + } else { + // For non-reference columns, there's no 'visible column' and we just return a regular formatter + return createFormatter(type, rec.widgetOptionsJson(), documentSettings); + } +} + +export function formatterForRec( + rec: ColumnRec | ViewFieldRec, colRec: ColumnRec, docModel: DocModel, visibleColFormatter: BaseFormatter +): BaseFormatter { + const type = colRec.type(); + // Ref/RefList columns delegate most formatting to the visibleColFormatter + const widgetOpts = {...rec.widgetOptionsJson(), visibleColFormatter}; + const documentSettings = docModel.docInfoRow.documentSettingsJson(); + return createFormatter(type, widgetOpts, documentSettings); } diff --git a/app/client/models/entities/ViewFieldRec.ts b/app/client/models/entities/ViewFieldRec.ts index 8ecc4123..eb7eafed 100644 --- a/app/client/models/entities/ViewFieldRec.ts +++ b/app/client/models/entities/ViewFieldRec.ts @@ -1,5 +1,5 @@ import {ColumnRec, DocModel, IRowModel, refRecord, ViewSectionRec} from 'app/client/models/DocModel'; -import {visibleColFormatterForRec} from 'app/client/models/entities/ColumnRec'; +import {formatterForRec, visibleColFormatterForRec} from 'app/client/models/entities/ColumnRec'; import * as modelUtil from 'app/client/models/modelUtil'; import * as UserType from 'app/client/widgets/UserType'; import {DocumentSettings} from 'app/common/DocumentSettings'; @@ -74,6 +74,13 @@ export interface ViewFieldRec extends IRowModel<"_grist_Views_section_field"> { // to the visibleCol associated with field. visibleColFormatter: ko.Computed; + // A formatter for values of this column. + // The difference between visibleColFormatter and formatter is especially important for ReferenceLists: + // `visibleColFormatter` is for individual elements of a list, sometimes hypothetical + // (i.e. they aren't actually referenced but they exist in the visible column and are relevant to e.g. autocomplete) + // `formatter` formats actual cell values, e.g. a whole list from the display column. + formatter: ko.Computed; + createValueParser(): (value: string) => any; // Helper which adds/removes/updates field's displayCol to match the formula. @@ -167,6 +174,8 @@ export function createViewFieldRec(this: ViewFieldRec, docModel: DocModel): void // associated with this field. If no visible column available, return formatting for the field itself. this.visibleColFormatter = ko.pureComputed(() => visibleColFormatterForRec(this, this.column(), docModel)); + this.formatter = ko.pureComputed(() => formatterForRec(this, this.column(), docModel, this.visibleColFormatter())); + this.createValueParser = function() { const fieldRef = this.useColOptions.peek() ? undefined : this.id.peek(); return createParser(docModel.docData, this.colRef.peek(), fieldRef); diff --git a/app/client/widgets/ChoiceTextBox.ts b/app/client/widgets/ChoiceTextBox.ts index 9404e31b..dbc9763e 100644 --- a/app/client/widgets/ChoiceTextBox.ts +++ b/app/client/widgets/ChoiceTextBox.ts @@ -49,7 +49,7 @@ export class ChoiceTextBox extends NTextBox { dom.domComputed((use) => { if (this.isDisposed() || use(row._isAddRow)) { return null; } - const formattedValue = use(this.valueFormatter).format(use(value)); + const formattedValue = use(this.valueFormatter).formatAny(use(value)); if (formattedValue === '') { return null; } const choiceOptions = use(this._choiceOptionsByName).get(formattedValue); diff --git a/app/client/widgets/NTextBox.ts b/app/client/widgets/NTextBox.ts index eb98172d..c53a9656 100644 --- a/app/client/widgets/NTextBox.ts +++ b/app/client/widgets/NTextBox.ts @@ -45,7 +45,7 @@ export class NTextBox extends NewAbstractWidget { return dom('div.field_clip', dom.style('text-align', this.alignment), dom.cls('text_wrapping', this.wrapping), - dom.domComputed((use) => use(row._isAddRow) ? null : makeLinks(use(this.valueFormatter).format(use(value)))) + dom.domComputed((use) => use(row._isAddRow) ? null : makeLinks(use(this.valueFormatter).formatAny(use(value)))) ); } diff --git a/app/client/widgets/NewAbstractWidget.ts b/app/client/widgets/NewAbstractWidget.ts index 820339ec..774db4ab 100644 --- a/app/client/widgets/NewAbstractWidget.ts +++ b/app/client/widgets/NewAbstractWidget.ts @@ -43,7 +43,7 @@ export abstract class NewAbstractWidget extends Disposable { )).onWrite((val) => this.field.textColor(val === defaultTextColor ? undefined : val)); this.fillColor = fromKo(this.field.fillColor); - this.valueFormatter = fromKo(field.visibleColFormatter); + this.valueFormatter = fromKo(field.formatter); } /** diff --git a/app/client/widgets/Reference.ts b/app/client/widgets/Reference.ts index c0336233..eb2ee0a8 100644 --- a/app/client/widgets/Reference.ts +++ b/app/client/widgets/Reference.ts @@ -12,8 +12,6 @@ import {Computed, dom, styled} from 'grainjs'; * Reference - The widget for displaying references to another table's records. */ export class Reference extends NTextBox { - protected _formatValue: Computed<(val: any) => string>; - private _visibleColRef: Computed; private _validCols: Computed>>; @@ -37,19 +35,6 @@ export class Reference extends NTextBox { })) .concat([{label: 'Row ID', value: 0, icon: 'FieldColumn'}]); }); - - // Computed returns a function that formats cell values. - this._formatValue = Computed.create(this, (use) => { - // If the field is pulling values from a display column, use a general-purpose formatter. - if (use(this.field.displayColRef) !== use(this.field.colRef)) { - const fmt = use(this.field.visibleColFormatter); - return (val: any) => fmt.formatAny(val); - } else { - const refTable = use(use(this.field.column).refTable); - const refTableId = refTable ? use(refTable.tableId) : ""; - return (val: any) => val > 0 ? `${refTableId}[${val}]` : ""; - } - }); } public buildConfigDom() { @@ -100,8 +85,8 @@ export class Reference extends NTextBox { // but the content of its displayCol has changed. Postponing doing anything about // this until we have three-way information for computed columns. For now, // just showing one version of the cell. TODO: elaborate. - use(this._formatValue)(displayValue[1].local || displayValue[1].parent) : - use(this._formatValue)(displayValue); + use(this.field.formatter).formatAny(displayValue[1].local || displayValue[1].parent) : + use(this.field.formatter).formatAny(displayValue); hasBlankReference = referenceId.get() !== 0 && value.trim() === ''; diff --git a/app/client/widgets/ReferenceList.ts b/app/client/widgets/ReferenceList.ts index 296b2da4..5c183f67 100644 --- a/app/client/widgets/ReferenceList.ts +++ b/app/client/widgets/ReferenceList.ts @@ -37,7 +37,10 @@ export class ReferenceList extends Reference { // return use(this._formatValue)(content[1].local || content[1].parent); // } const items = isList(content) ? content.slice(1) : [content]; - return items.map(use(this._formatValue)); + // Use field.visibleColFormatter instead of field.formatter + // because we're formatting each list element to render tokens, not the whole list. + const formatter = use(this.field.visibleColFormatter); + return items.map(item => formatter.formatAny(item)); }, (input) => { if (!input) { diff --git a/app/common/ValueFormatter.ts b/app/common/ValueFormatter.ts index c9b0505d..b439cd1b 100644 --- a/app/common/ValueFormatter.ts +++ b/app/common/ValueFormatter.ts @@ -3,6 +3,7 @@ import {csvEncodeRow} from 'app/common/csvFormat'; import {CellValue} from 'app/common/DocActions'; import {DocumentSettings} from 'app/common/DocumentSettings'; +import {getReferencedTableId, isList} from 'app/common/gristTypes'; import * as gristTypes from 'app/common/gristTypes'; import * as gutil from 'app/common/gutil'; import {buildNumberFormat, NumberFormatOptions} from 'app/common/NumberFormat'; @@ -24,15 +25,24 @@ export function formatUnknown(value: CellValue): string { return formatDecoded(decodeObject(value)); } +/** + * Returns true if the array contains other arrays or structured objects, + * indicating that the list should be formatted like JSON rather than CSV. + */ +function hasNestedObjects(value: any[]) { + return value.some(v => typeof v === 'object' && v && (Array.isArray(v) || isPlainObject(v))); +} + /** * Formats a decoded Grist value for displaying it. For top-level values, formats them the way we - * like to see them in a cell or in, say, CSV export. For lists and objects, nested values are - * formatted slighly differently, with quoted strings and ISO format for dates. + * like to see them in a cell or in, say, CSV export. + * For top-level lists containing only simple values like strings and dates, formats them as a CSV row. + * Nested lists and objects are formatted slighly differently, with quoted strings and ISO format for dates. */ export function formatDecoded(value: unknown, isTopLevel: boolean = true): string { if (typeof value === 'object' && value) { if (Array.isArray(value)) { - if (!isTopLevel || value.some(v => typeof v === 'object' && v && (Array.isArray(v) || isPlainObject(v)))) { + if (!isTopLevel || hasNestedObjects(value)) { return '[' + value.map(v => formatDecoded(v, false)).join(', ') + ']'; } else { return csvEncodeRow(value.map(v => formatDecoded(v, true)), {prettier: true}); @@ -62,14 +72,6 @@ export class BaseFormatter { gristTypes.isRightType('Any')!; } - /** - * Formats a value that matches the type of this formatter. This should be overridden by derived - * classes to handle values in formatter-specific ways. - */ - public format(value: any): string { - return String(value); - } - /** * Formats using this.format() if a value is of the right type for this formatter, or using * AnyFormatter otherwise. This method the recommended API. There is no need to override it. @@ -77,6 +79,14 @@ export class BaseFormatter { public formatAny(value: any): string { return this.isRightType(value) ? this.format(value) : formatUnknown(value); } + + /** + * Formats a value that matches the type of this formatter. This should be overridden by derived + * classes to handle values in formatter-specific ways. + */ + protected format(value: any): string { + return String(value); + } } class AnyFormatter extends BaseFormatter { @@ -178,12 +188,86 @@ class DateTimeFormatter extends DateFormatter { } } -const formatters: {[name: string]: typeof BaseFormatter} = { +class RowIdFormatter extends BaseFormatter { + public widgetOpts: { tableId: string }; + + public format(value: number): string { + return value > 0 ? `${this.widgetOpts.tableId}[${value}]` : ""; + } +} + +interface ReferenceFormatOptions { + visibleColFormatter?: BaseFormatter; +} + +class ReferenceFormatter extends BaseFormatter { + public widgetOpts: ReferenceFormatOptions; + protected visibleColFormatter: BaseFormatter; + + constructor(type: string, widgetOpts: ReferenceFormatOptions, docSettings: DocumentSettings) { + super(type, widgetOpts, docSettings); + // widgetOpts.visibleColFormatter shouldn't be undefined, but it can be if a referencing column + // is displaying another referencing column, which is partially prohibited in the UI but still possible. + this.visibleColFormatter = widgetOpts.visibleColFormatter || + createFormatter('Id', {tableId: getReferencedTableId(type)}, docSettings); + } + + public formatAny(value: any): string { + /* + An invalid value in a referencing column is saved as a string and becomes AltText in the data engine. + Then the display column formula (e.g. $person.first_name) raises an InvalidTypedValue trying to access + an attribute of that AltText. + This would normally lead to the formatter displaying `#Invalid Ref[List]: ` before the string value. + That's inconsistent with how the cell is displayed (just the string value in pink) + and with how invalid values in other columns are formatted (just the string). + It's just a result of the formatter receiving a value from the display column, not the actual column. + It's also likely to inconvenience users trying to import/migrate/convert data. + So we suppress the error here and just show the text. + It's still technically possible for the column to display an actual InvalidTypedValue exception from a formula + and this will suppress that too, but this is unlikely and seems worth it. + */ + if ( + Array.isArray(value) + && value[0] === GristObjCode.Exception + && value[1] === "InvalidTypedValue" + && value[2]?.startsWith?.("Ref") + ) { + return value[3]; + } + return this.formatNotInvalidRef(value); + } + + protected formatNotInvalidRef(value: any) { + return this.visibleColFormatter.formatAny(value); + } +} + +class ReferenceListFormatter extends ReferenceFormatter { + protected formatNotInvalidRef(value: any): string { + // Part of this repeats the logic in BaseFormatter.formatAny which is overridden in ReferenceFormatter + // It also ensures that complex lists (e.g. if this RefList is displaying a ChoiceList) + // are formatted as JSON instead of CSV. + if (!isList(value) || hasNestedObjects(value)) { + return formatUnknown(value); + } + // In the most common case, lists of simple objects like strings or dates + // are formatted like a CSV. + // This is similar to formatUnknown except the inner values are + // formatted according to the visible column options. + const formattedValues = value.slice(1).map(v => super.formatNotInvalidRef(v)); + return csvEncodeRow(formattedValues, {prettier: true}); + } +} + +const formatters: { [name: string]: typeof BaseFormatter } = { Numeric: NumericFormatter, Int: IntFormatter, Bool: BaseFormatter, Date: DateFormatter, DateTime: DateTimeFormatter, + Ref: ReferenceFormatter, + RefList: ReferenceListFormatter, + Id: RowIdFormatter, // We don't list anything that maps to AnyFormatter, since that's the default. }; diff --git a/app/common/ValueParser.ts b/app/common/ValueParser.ts index ebf9e72b..8b71ec7d 100644 --- a/app/common/ValueParser.ts +++ b/app/common/ValueParser.ts @@ -11,6 +11,7 @@ import NumberParse from 'app/common/NumberParse'; import {parseDateStrict, parseDateTime} from 'app/common/parseDate'; import {MetaRowRecord, TableData} from 'app/common/TableData'; import {DateFormatOptions, DateTimeFormatOptions, formatDecoded, FormatOptions} from 'app/common/ValueFormatter'; +import {encodeObject} from 'app/plugin/objtypes'; import flatMap = require('lodash/flatMap'); import mapValues = require('lodash/mapValues'); @@ -177,7 +178,7 @@ export class ReferenceListParser extends ReferenceParser { // csvDecodeRow should never raise an exception values = csvDecodeRow(raw); } - values = values.map(v => typeof v === "string" ? this._visibleColParser(v) : v); + values = values.map(v => typeof v === "string" ? this._visibleColParser(v) : encodeObject(v)); if (!values.length || !raw) { return null; // null is the default value for a reference list column