Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
58 changes: 58 additions & 0 deletions src/conventions/displayColumns.ts
Original file line number Diff line number Diff line change
@@ -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-<digits>`. */
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;
}
1 change: 1 addition & 0 deletions src/diagnostics.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
10 changes: 9 additions & 1 deletion src/instrument/lstsvTypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, string>;
Expand Down Expand Up @@ -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 {
Expand Down
5 changes: 5 additions & 0 deletions src/lstsv/columns.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand All @@ -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];
Expand Down
30 changes: 30 additions & 0 deletions src/pipelines/xlsform2lstsv/appearanceHandler.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -59,6 +65,14 @@ export class AppearanceHandler {
base: string,
): Record<string, string> {
let columns: Record<string, string> = {};
// 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;
Expand All @@ -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<string, string> {
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`. */
Expand Down
5 changes: 5 additions & 0 deletions src/pipelines/xlsform2lstsv/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -652,6 +652,11 @@ class Conversion {
),
...fields.answerBounds,
...this.arrayFilterAttribute(questionName),
...this.appearanceHandler.columnsAttributes(
lsType,
appearance,
xfTypeInfo.base,
),
...this.appearanceHandler.themeAttributes(
lsType,
appearance,
Expand Down
26 changes: 26 additions & 0 deletions src/xlsform/validate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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(
Expand Down
11 changes: 11 additions & 0 deletions tests/ts/unit/diagnosticCodes.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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?' }],
Expand Down
110 changes: 110 additions & 0 deletions tests/ts/unit/displayColumns.test.ts
Original file line number Diff line number Diff line change
@@ -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<string, unknown>[]) {
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<string, string>)[
'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<string, string>)[
'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',
);
});
});
Loading