From 1d9688c9b84a09e515398c1479441fc18bb984df Mon Sep 17 00:00:00 2001 From: jstet Date: Wed, 4 Nov 2026 20:55:40 +0100 Subject: [PATCH] feat(lstsv): map appearance columns-n to display_columns (#239) Co-Authored-By: Claude Opus 5.5 --- src/conventions/displayColumns.ts | 58 +++++++++ src/diagnostics.ts | 1 + src/instrument/lstsvTypes.ts | 10 +- src/lstsv/columns.ts | 5 + .../xlsform2lstsv/appearanceHandler.ts | 30 +++++ src/pipelines/xlsform2lstsv/index.ts | 5 + src/xlsform/validate.ts | 26 +++++ tests/ts/unit/diagnosticCodes.test.ts | 11 ++ tests/ts/unit/displayColumns.test.ts | 110 ++++++++++++++++++ 9 files changed, 255 insertions(+), 1 deletion(-) create mode 100644 src/conventions/displayColumns.ts create mode 100644 tests/ts/unit/displayColumns.test.ts diff --git a/src/conventions/displayColumns.ts b/src/conventions/displayColumns.ts new file mode 100644 index 0000000..6a6f79a --- /dev/null +++ b/src/conventions/displayColumns.ts @@ -0,0 +1,58 @@ +/** + * ODK `columns-n` (and plain `columns`) on a select question: the options in + * n columns, LimeSurvey's `display_columns` question attribute (type L or M; + * its renderer adds the class `nbcol-n`). The count is part of the appearance + * name, so this is a pattern, not a registry allowlist entry. Plain `columns` + * lets ODK fit as many columns as the screen takes, which LimeSurvey cannot, + * so it becomes {@link DEFAULT_COLUMNS}. `display_columns=n` reads back as + * `columns-n`. + */ + +/** The LimeSurvey question attribute holding the column count. */ +export const DISPLAY_COLUMNS_ATTRIBUTE = 'display_columns'; + +/** Column count for plain `columns`. */ +export const DEFAULT_COLUMNS = 2; + +/** Largest count LimeSurvey's column selector offers. */ +export const MAX_COLUMNS = 8; + +/** Base types (after from_file folding) and LS codes the attribute applies to. */ +export const COLUMNS_BASE_TYPES = [ + 'select_one', + 'select_multiple', + 'select_one_from_file', + 'select_multiple_from_file', +]; +export const COLUMNS_LS_TYPES = ['L', 'M']; + +/** Whether an appearance token is `columns` or `columns-`. */ +export function isColumnsAppearance(token: string): boolean { + return /^columns(-\d+)?$/.test(token); +} + +/** The column count of a `columns*` token; undefined when out of 1..8. */ +export function columnsOf(token: string): number | undefined { + const m = /^columns(?:-(\d+))?$/.exec(token); + if (!m) return undefined; + const n = m[1] === undefined ? DEFAULT_COLUMNS : Number(m[1]); + return n >= 1 && n <= MAX_COLUMNS ? n : undefined; +} + +/** The appearance for a `display_columns` cell (`3` → `columns-3`), if valid. */ +export function columnsAppearanceOf(cell: string): string | undefined { + const t = cell.trim(); + if (!/^\d+$/.test(t)) return undefined; + const n = Number(t); + return n >= 1 && n <= MAX_COLUMNS ? `columns-${n}` : undefined; +} + +/** The count of the last valid `columns*` token in an appearance string. */ +export function columnsInAppearance(appearance: string): number | undefined { + let n: number | undefined; + for (const part of appearance.split(/\s+/)) { + const c = columnsOf(part); + if (c !== undefined) n = c; + } + return n; +} diff --git a/src/diagnostics.ts b/src/diagnostics.ts index 5f9b659..aca71fd 100644 --- a/src/diagnostics.ts +++ b/src/diagnostics.ts @@ -42,6 +42,7 @@ export type DiagnosticCode = | 'vocab-csv-invalid' | 'vocab-codes-derived' | 'appearance-unregistered' + | 'appearance-invalid-columns' | 'appearance-invalid-for-type' | 'exclusive-invalid' | 'exclusive-no-effect' diff --git a/src/instrument/lstsvTypes.ts b/src/instrument/lstsvTypes.ts index b0272af..82e0f72 100644 --- a/src/instrument/lstsvTypes.ts +++ b/src/instrument/lstsvTypes.ts @@ -5,6 +5,10 @@ */ import { APPEARANCES } from '../generated/Appearances.js'; import { TYPE_MAPPINGS } from '../generated/TypeMappings.js'; +import { + DISPLAY_COLUMNS_ATTRIBUTE, + columnsAppearanceOf, +} from '../conventions/displayColumns.js'; import { themedAppearanceOf } from '../conventions/questionTheme.js'; type Row = Record; @@ -106,7 +110,11 @@ export interface ResolvedType { export function resolveType(lsCode: string, row: Row): ResolvedType { const resolved = resolveBaseType(lsCode, row); const themed = themedAppearanceOf(lsCode, resolved.base, row); - return themed ? { ...resolved, appearance: themed } : resolved; + const columns = ['L', 'M'].includes(lsCode) + ? columnsAppearanceOf(cell(row, DISPLAY_COLUMNS_ATTRIBUTE)) + : undefined; + const appearance = [themed, columns].filter(Boolean).join(' '); + return appearance ? { ...resolved, appearance } : resolved; } function resolveBaseType(lsCode: string, row: Row): ResolvedType { diff --git a/src/lstsv/columns.ts b/src/lstsv/columns.ts index 99b43ad..6360231 100644 --- a/src/lstsv/columns.ts +++ b/src/lstsv/columns.ts @@ -61,6 +61,10 @@ export const BASE_COLUMNS = [ * theme from `question_theme_name` and stores the other columns as * question attributes. * + * - `display_columns`: the number of columns an L / M question's options are + * laid out in, from the appearance `columns-n` (convention in + * `conventions/displayColumns.ts`). + * * A registry attribute missing here would be dropped silently; a test checks * the registry against this list. */ @@ -83,6 +87,7 @@ export const ATTRIBUTE_COLUMNS = [ 'question_theme_name', 'show_search', 'width_entry', + 'display_columns', ] as const; export type BaseColumn = (typeof BASE_COLUMNS)[number]; diff --git a/src/pipelines/xlsform2lstsv/appearanceHandler.ts b/src/pipelines/xlsform2lstsv/appearanceHandler.ts index f8965a7..8aa7f15 100644 --- a/src/pipelines/xlsform2lstsv/appearanceHandler.ts +++ b/src/pipelines/xlsform2lstsv/appearanceHandler.ts @@ -1,6 +1,12 @@ import { APPEARANCES } from '../../generated/Appearances.js'; import type { AppearanceSpec } from '../../generated/Appearances.js'; import { QUESTION_THEME_COLUMN } from '../../conventions/questionTheme.js'; +import { + COLUMNS_BASE_TYPES, + COLUMNS_LS_TYPES, + DISPLAY_COLUMNS_ATTRIBUTE, + columnsInAppearance, +} from '../../conventions/displayColumns.js'; import { LSType } from './typeMapper.js'; import { consoleWarning } from '../../diagnostics.js'; import type { WarningHandler } from '../../diagnostics.js'; @@ -59,6 +65,14 @@ export class AppearanceHandler { base: string, ): Record { let columns: Record = {}; + // An explicit column count wins over a button row (`horizontal`). + if ( + this.columnsAttributes(lsType, appearance, base)[ + DISPLAY_COLUMNS_ATTRIBUTE + ] + ) { + return columns; + } for (const spec of applicable(appearance, base)) { const theme = spec.lsQuestionTheme; if (!theme || (!spec.lsTypeOverride && lsType.type !== 'L')) continue; @@ -70,6 +84,22 @@ export class AppearanceHandler { } return columns; } + + /** + * `columns-n` / `columns` on a list (L) or multiple choice (M) → + * `display_columns` (convention in `conventions/displayColumns.ts`). A + * dropdown (`minimal`, `autocomplete`) has no columns: none is written. + */ + columnsAttributes( + lsType: LSType, + appearance: string, + base: string, + ): Record { + if (!COLUMNS_BASE_TYPES.includes(base)) return {}; + if (!COLUMNS_LS_TYPES.includes(lsType.type)) return {}; + const n = columnsInAppearance(appearance); + return n === undefined ? {} : { [DISPLAY_COLUMNS_ATTRIBUTE]: String(n) }; + } } /** The registered appearances in `appearance` that are valid on `base`. */ diff --git a/src/pipelines/xlsform2lstsv/index.ts b/src/pipelines/xlsform2lstsv/index.ts index e66a596..e48c574 100644 --- a/src/pipelines/xlsform2lstsv/index.ts +++ b/src/pipelines/xlsform2lstsv/index.ts @@ -652,6 +652,11 @@ class Conversion { ), ...fields.answerBounds, ...this.arrayFilterAttribute(questionName), + ...this.appearanceHandler.columnsAttributes( + lsType, + appearance, + xfTypeInfo.base, + ), ...this.appearanceHandler.themeAttributes( lsType, appearance, diff --git a/src/xlsform/validate.ts b/src/xlsform/validate.ts index c815b16..e53df7b 100644 --- a/src/xlsform/validate.ts +++ b/src/xlsform/validate.ts @@ -7,6 +7,12 @@ import type { WarningHandler, } from '../diagnostics.js'; import { APPEARANCES } from '../generated/Appearances.js'; +import { + COLUMNS_BASE_TYPES, + MAX_COLUMNS, + columnsOf, + isColumnsAppearance, +} from '../conventions/displayColumns.js'; import { TYPE_MAPPINGS } from '../generated/TypeMappings.js'; import { SurveyRow, ChoiceRow } from './types.js'; @@ -913,6 +919,26 @@ export class XLSValidator { const baseType = (row.type || '').trim().split(/\s+/)[0]; const name = typeof row.name === 'string' ? row.name : ''; for (const part of appearance.split(/\s+/)) { + if (isColumnsAppearance(part)) { + if (columnsOf(part) === undefined) { + violations.push( + warning( + 'appearance-invalid-columns', + `appearance "${part}" on "${name}" needs a column count from 1 to ${MAX_COLUMNS} and will be ignored`, + name, + ), + ); + } else if (!COLUMNS_BASE_TYPES.includes(baseType)) { + violations.push( + warning( + 'appearance-invalid-for-type', + `appearance "${part}" on "${name}" is not valid for type "${baseType}" and will be ignored`, + name, + ), + ); + } + continue; + } const spec = APPEARANCES[part]; if (!spec) { violations.push( diff --git a/tests/ts/unit/diagnosticCodes.test.ts b/tests/ts/unit/diagnosticCodes.test.ts index 8309e03..aa2df90 100644 --- a/tests/ts/unit/diagnosticCodes.test.ts +++ b/tests/ts/unit/diagnosticCodes.test.ts @@ -181,6 +181,17 @@ describe('validateSubset', () => { 'appearance-invalid-for-type', [{ type: 'text', name: 'q', label: 'Q?', appearance: 'likert' }], ], + [ + 'appearance-invalid-columns', + [ + { + type: 'select_one yn', + name: 'q', + label: 'Q?', + appearance: 'columns-0', + }, + ], + ], [ 'exclusive-invalid', [{ type: 'select_multiple l', name: 'q', label: 'Q?' }], diff --git a/tests/ts/unit/displayColumns.test.ts b/tests/ts/unit/displayColumns.test.ts new file mode 100644 index 0000000..3197eb8 --- /dev/null +++ b/tests/ts/unit/displayColumns.test.ts @@ -0,0 +1,110 @@ +/** + * #239: `appearance: columns-n` (and `columns`) on select_one / select_multiple + * becomes LimeSurvey's `display_columns` question attribute, and comes back. + */ +import { describe, expect, test } from 'vitest'; + +import { XLSFormToTSVConverter } from '../../../src/pipelines/xlsform2lstsv/index.js'; +import { lstsvToXlsform } from '../../../src/pipelines/lstsv2xlsform/index.js'; +import type { Diagnostic } from '../../../src/diagnostics.js'; +import { findRowByName, parseTSV } from './helpers'; + +const choices = [ + { list_name: 'yn', name: 'ja', label: 'Ja' }, + { list_name: 'yn', name: 'nein', label: 'Nein' }, +]; + +async function convert(survey: Record[]) { + const warnings: Diagnostic[] = []; + const tsv = await new XLSFormToTSVConverter({ + onWarning: (w) => warnings.push(w), + }).convert(survey, choices, [{ form_title: 'T', default_language: 'de' }]); + return { tsv, rows: parseTSV(tsv), warnings }; +} + +const q = (appearance: string, type = 'select_one yn') => ({ + type, + name: 'q1', + label: 'Q', + appearance, +}); + +const cols = (rows: unknown[]) => + ((findRowByName(rows as never, 'q1') ?? {}) as Record)[ + 'display_columns' + ] ?? ''; + +describe('columns-n → display_columns', () => { + test.each([ + ['columns-2', 'select_one yn', '2'], + ['columns-3', 'select_one yn', '3'], + ['columns-3', 'select_multiple yn', '3'], + ['columns', 'select_one yn', '2'], + ['columns', 'select_multiple yn', '2'], + ['columns-8', 'select_one yn', '8'], + ])('%s on %s sets display_columns=%s, no warning', async (a, t, n) => { + const { rows, warnings } = await convert([q(a, t)]); + expect(cols(rows)).toBe(n); + expect(warnings).toEqual([]); + }); + + test('no display_columns column without the appearance', async () => { + const { tsv } = await convert([q('')]); + expect(tsv.split('\n')[0].split('\t')).not.toContain('display_columns'); + }); + + test.each(['columns-0', 'columns-9', 'columns-x', 'columns-'])( + '%s warns and sets nothing', + async (a) => { + const { rows, warnings } = await convert([q(a)]); + expect(cols(rows)).toBe(''); + expect(warnings.map((w) => w.code)).toEqual( + a === 'columns-x' || a === 'columns-' + ? ['appearance-unregistered'] + : ['appearance-invalid-columns'], + ); + }, + ); + + test('columns on a text question warns', async () => { + const { rows, warnings } = await convert([q('columns-2', 'text')]); + expect(cols(rows)).toBe(''); + expect(warnings.map((w) => w.code)).toEqual([ + 'appearance-invalid-for-type', + ]); + }); + + test('columns wins over horizontal: no button theme', async () => { + const { rows } = await convert([q('horizontal columns-3')]); + expect(cols(rows)).toBe('3'); + expect( + (findRowByName(rows, 'q1') as Record)[ + 'question_theme_name' + ] ?? '', + ).toBe(''); + }); + + test('a dropdown has no columns', async () => { + const { rows } = await convert([q('minimal columns-2')]); + expect(findRowByName(rows, 'q1')!['type/scale']).toBe('!'); + expect(cols(rows)).toBe(''); + }); + + test.each(['select_one yn', 'select_multiple yn'])( + 'round trip %s: columns-3 comes back', + async (type) => { + const { tsv } = await convert([q('columns-3', type)]); + const back = await lstsvToXlsform(tsv); + const row = back.survey.find((r) => r.name === 'q1')!; + expect(row['appearance']).toBe('columns-3'); + }, + ); + + test('plain columns comes back as columns-2', async () => { + const { tsv } = await convert([q('columns')]); + const back = await lstsvToXlsform(tsv); + expect(back.survey.find((r) => r.name === 'q1')!['appearance']).toBe( + 'columns-2', + ); + }); +});