From a26ad803857489893026e5f5f548873be1442104 Mon Sep 17 00:00:00 2001 From: Aleksandr Shevchuk Date: Tue, 10 Nov 2026 16:49:39 +0200 Subject: [PATCH 1/4] fix(author): a source's own fields no longer hide its content, for a chat with no skill text (#174) A chat given a link to the skill and none of its text writes a source's outer shape its own way. The build checked a source's own fields, then the type's name, then the content, and stopped at each, so every layer cost a round in which nothing inside was looked at. A build now reads a source with the pack's names and takes what can only be meant, each said once with what to write, and checks the content in the same build wherever the type is known: - the content type under another key, `contentType`, or named loosely as `type`, "interactive-book", "Question Set": the one library of those letters (names.mjs). A name that is none has the nearest offered; - the title, language, licence and authors in an object, as H5P keeps them in `metadata`; - the content's fields beside `type`, with no `params` around them. Sub-content that names its library under another key, with its parameters in `params` or beside the name, is taken the same way, in a library field and in the place of the group that holds one. A title beside the parameters that is no field of the library is the sub-content's own. Measured on Haiku 5.5 with no skill text, six content types, two runs each, the build page's copy pasted back: 0.7.0 had 3 of 12 right by the second round and 8 by the fourth, four never; with this, 10 by the second round and all 12 by the third. Of 263 sources the earlier tests stored, 257 have the same problems and none builds with one build and not the other; the 36 sources of the type files and the quiz format build the same package. --- design/author.md | 36 ++++- packages/author/README.md | 13 ++ packages/author/lib/browser.mjs | 9 +- packages/author/lib/build.mjs | 188 +++++++++++++++++++----- packages/author/lib/generator.mjs | 78 ++++++++-- packages/author/lib/libraries.mjs | 20 +++ packages/author/lib/names.mjs | 70 +++++++++ packages/author/lib/node.mjs | 10 +- packages/author/tests/build.test.ts | 94 +++++++++++- packages/author/tests/generator.test.ts | 72 ++++++++- packages/author/tests/names.test.ts | 49 ++++++ 11 files changed, 577 insertions(+), 62 deletions(-) create mode 100644 packages/author/lib/names.mjs create mode 100644 packages/author/tests/names.test.ts diff --git a/design/author.md b/design/author.md index 6d01c7a..f782a6f 100644 --- a/design/author.md +++ b/design/author.md @@ -15,7 +15,7 @@ already holds text to the content check's rules, from the pack an untrusted chec `apps/skills` plays every example untrusted too. Measured on a five-question quiz, at four characters a token: `content.json` was about 3,470 tokens to write by hand, after reading 3,900 to 9,500 of a type's semantics; the source is 1,000 to 1,800 tokens, written from a type file of 200 -to 3,000. Eighteen things the code does not say by itself: +to 3,000. Nineteen things the code does not say by itself: - **One generator, three ways to run it.** The third, `saved`, checks content as H5P stores it (*Saved content is checked as the build checks what it writes*, below). `ContentGenerator` was the sweep's @@ -396,6 +396,40 @@ to 3,000. Eighteen things the code does not say by itself: Interactive Video, Image Hotspots, Image Choice, Game Map): of 12 first replies 2 were right at once with the sentence and 2 without, the Accordions. The skill gives the type files' address on GitHub too, for a chat that has its text and can open a link. +- **A source's own fields do not hide its content, and its outer shape is taken as it is meant** + (2026-11-10, #174). Gemini in Chrome, given a link to the skill's folder, which shows a chat the + file names and none of the text, wrote a book its own way: `contentType` for `type`, and the + title, licence and authors in a `metadata` object, as H5P keeps them. The build checked a + source's own fields, then the type's name, then the content, and stopped at each: seven + problems of the outer shape, then thirty of the content, and a package at the third try. + Measured on Haiku in that position, the request as it was worded and no skill text, six content + types, two runs each, the build page's copy pasted back: with 0.7.0 none was right at once, 3 + by the second round, 8 of 12 by the fourth, and four never. All twelve first sources had the + content's fields beside `type` with no `params`, nine named the type as it is spoken of, and six + of the nine second rounds were one problem, `the library pack has no content type + interactive-book`, with no name offered: the next guesses were `book` and `flashcards`. + + So a build reads a source with the pack's names (`sourceOf`), and takes what can only be meant: + the content type under another key, or named loosely as `type`, the same letters and digits in + any case with or without `H5P.` (`names.mjs`); the source's own fields in an object; the + content's fields beside `type`. Each is a problem, said once with what to write, and the content + is generated and checked in the same build wherever the type is known, with a stand-in for what + is missing. A name that is no type has the nearest offered: those its letters are part of, and + those they end with, not one they begin with, since the "image" of "image choice" is no lead. + A title is not tried, "Multiple Choice" or "Image Choice": the pack's index has none, and + reading them takes every part, which a page cannot. Sub-content that names its library under + another key, `contentType` or `type`, is taken the same way, its parameters in `params` or + beside the name, where one text names a library the field takes; and a title beside them that + is no field of the library is the sub-content's own, its `metadata.title`, which had been two + problems a chapter. + + With it 10 of the 12 were right by the second round and all 12 by the third, and every one + plays. Of the 263 sources the earlier tests stored, written with the skill's text, 257 have the + same problems and none builds with one build and not the other; the 36 sources of the type + files and the quiz format build the same package. `readSource` keeps its contract, a source's + own problems thrown, without the pack's names. What the build page must do with it is its own: + hand the type's file back for a type that is named loosely, which its rule, the source's `type` + as written, does not. Not built: editing an existing package, which a tool beside it does with `checkContent` as its check; layout for Drag and Drop and Crossword; and any judgement of whether content is right, which diff --git a/packages/author/README.md b/packages/author/README.md index 74b7309..80d3521 100644 --- a/packages/author/README.md +++ b/packages/author/README.md @@ -97,6 +97,19 @@ h5p-author: The source has 3 problems: params.slides[0].elements[0].params.text: has

, which this field does not take; it takes
… ``` +A source's own fields are read the same way, for a chat that wrote one with no skill beside it. +What its outer shape plainly means is taken and said once, and the content is checked in the same +round: the content type under another key or named as it is spoken of (`"interactive-book"`, +`"Question Set"`), the title, licence and authors in an object, the content's fields beside `type`. + +``` +h5p-author: The source has 4 problems: + contentType: is not a field of a source: the content type is its type: write "type": "H5P.InteractiveBook" + metadata: is not a field of a source: title, license, authors are written beside type, not in an object: write "title": …, "license": …, "authors": … at the top + language: expected a BCP 47 language tag, such as "en" or "de-AT" + params.chapters[0]: is the params of sub-content, without the sub-content around them: write params.chapters[0] as { "library": "H5P.Column", "metadata": { "title": … }, "params": { "content": … } } +``` + The quiz format the h5p-author skill published first (`{ "kind": "question-set", "questions": [ … ] }`) is still taken, for a Multiple Choice question or a Question Set of them. diff --git a/packages/author/lib/browser.mjs b/packages/author/lib/browser.mjs index 7fb0836..9d98827 100644 --- a/packages/author/lib/browser.mjs +++ b/packages/author/lib/browser.mjs @@ -1,9 +1,10 @@ import { BlobReader, BlobWriter, ZipWriter } from '@zip.js/zip.js' import { crc32 } from '@missing-elements/h5p-normalize/crc32' import { packLibrariesIn } from '@missing-elements/h5p-offline-player/plan' -import { composePackage, readSource, writePackage } from './build.mjs' +import { composePackage, sourceOf, typeNames, writePackage } from './build.mjs' import { MediaStore } from './media.mjs' import { Pack } from './pack.mjs' +import { SourceError } from './source-error.mjs' export * from './core.mjs' @@ -154,11 +155,13 @@ export async function buildPackage(input, options) { secure() const { libraries, files = {}, fetch: fetchImpl = pageFetch } = options ?? {} if (!libraries) throw new TypeError('Name the library pack: a pack openPack opened, the pack export of @missing-elements/h5p-libraries, or the address of a copy of its parts/ folder') - const source = readSource(input) const pack = libraries instanceof Pack ? libraries : await openPack(libraries) try { + // With the pack's names, as the command's build reads a source (#174). + const read = sourceOf(input, typeNames(pack)) + if (!read.typed) throw new SourceError(read.problems) const media = new MediaStore(pageMedia(files), { fetch: fetchImpl }) - const built = await composePackage(source, pack, media) + const built = await composePackage(read.source, pack, media, read.problems) const writer = new ZipWriter(new BlobWriter('application/zip'), { useWebWorkers: false }) await writePackage(writer, built, pack, async () => { for (const item of media.files) { diff --git a/packages/author/lib/build.mjs b/packages/author/lib/build.mjs index e5a160d..73d5051 100644 --- a/packages/author/lib/build.mjs +++ b/packages/author/lib/build.mjs @@ -5,6 +5,7 @@ import { ContentGenerator, libraryString } from './generator.mjs' import { installClosure } from './install.mjs' import { loadLibraries } from './libraries.mjs' import { CC_VERSIONS, LICENSES, ROLES } from './metadata.mjs' +import { machineNameOf, nearestNames } from './names.mjs' import { quizProblems, quizSource } from './quiz.mjs' import { SourceError } from './source-error.mjs' import { escapeText } from './text.mjs' @@ -34,55 +35,166 @@ export { SourceError } /** @param {unknown} value @returns {value is Record} */ const isObject = (value) => value !== null && typeof value === 'object' && !Array.isArray(value) +/** A source's fields. */ +const FIELDS = ['type', 'title', 'language', 'license', 'licenseVersion', 'authors', 'source', 'yearFrom', 'yearTo', 'params'] +/** Those a source says of itself, which H5P keeps in an object of their own, `metadata`. */ +const OWN = FIELDS.filter((name) => name !== 'type' && name !== 'params') + /** - * The general source checked for its own fields; a quiz in the published format mapped onto it. + * The machine names a source's `type` may be: the pack's libraries but those carried only for an + * H5P site's upload check. Which of them are content types the index does not say; the build does. + * @param {import('./pack.mjs').Pack} pack + * @returns {string[]} + * @internal + */ +export const typeNames = (pack) => [...new Set(pack.libraries.filter((entry) => !entry.installOnly).map((entry) => entry.machineName))] + +/** + * A source read for a build: its own fields checked, and what it plainly means taken where it is + * written another way, so that its content is checked in the same build (#174). A chat with no + * skill beside it writes a source's outer shape its own way, and each of these cost it a round in + * which nothing inside was looked at: + * + * - the content type under another name, `contentType` or `library`, or named loosely as `type`, + * "interactive-book", which only `names` can tell; + * - the title, language, licence and authors in an object, as H5P keeps them in `metadata`; + * - the content's fields beside `type`, with no `params` around them. + * + * Each is a problem, said once with what to write. `typed` is whether the content type is known, + * and so whether the content can be checked: `source` then stands for the source as it would be + * written right, with a stand-in for what is missing, and is not for building while `problems` + * has any. * @param {unknown} input - * @returns {Source} + * @param {string[]} [names] the machine names `type` may be, to tell a name written loosely + * @returns {{ source: Source, problems: Problem[], typed: boolean }} * @internal */ -export function readSource(input) { +export function sourceOf(input, names) { if (isObject(input) && 'kind' in input) { const problems = quizProblems(input) if (problems.length) throw new SourceError(problems) - return /** @type {Source} */ (quizSource(/** @type {import('./quiz.mjs').Quiz} */ (input))) + return { source: /** @type {Source} */ (quizSource(/** @type {import('./quiz.mjs').Quiz} */ (input))), problems: [], typed: true } } /** @type {Problem[]} */ const problems = [] const report = (/** @type {string} */ path, /** @type {string} */ message) => problems.push({ path, message }) if (!isObject(input)) throw new SourceError([{ path: 'source', message: 'expected an object: { "type", "title", "language", "license", "authors", "params" }' }]) - const known = ['type', 'title', 'language', 'license', 'licenseVersion', 'authors', 'source', 'yearFrom', 'yearTo', 'params'] - for (const key of Object.keys(input)) if (!known.includes(key)) report(key, `is not a field of a source; the fields are ${known.join(', ')}`) - if (typeof input.type === 'string' && /^\S+\s+\d+\.\d+$/.test(input.type.trim())) { - // checkContent takes a version, as saved content names one; a build takes the pack's newest. - const [name] = input.type.trim().split(/\s/) - report('type', `takes the machine name alone, "${name}": the version is the pack's newest`) - } else if (typeof input.type !== 'string' || !/^\S+$/.test(input.type.trim())) report('type', 'expected a content type\'s machine name, such as "H5P.Blanks"') - if (typeof input.title !== 'string' || !input.title.trim()) report('title', 'expected the activity\'s title') - else if (input.title.length > 255) report('title', 'takes at most 255 characters') - if (typeof input.language !== 'string') report('language', 'expected a BCP 47 language tag, such as "en" or "de-AT"') - else { - try { - Intl.getCanonicalLocales(input.language) - } catch { - report('language', 'expected a BCP 47 language tag, such as "en" or "de-AT"') + const fields = `the fields are ${FIELDS.join(', ')}` + /** @type {Record} what is written for each field */ + const given = Object.fromEntries(FIELDS.filter((name) => input[name] !== undefined).map((name) => [name, input[name]])) + /** @type {Record} where each is written */ + const at = Object.fromEntries(FIELDS.map((name) => [name, name])) + let rest = Object.keys(input).filter((key) => !FIELDS.includes(key)) + + // The content type under another name: the one text beside the fields that names one. + let named = '' + if (given.type === undefined) { + const keys = rest.filter((key) => typeof input[key] === 'string' && input[key].length <= 64 && + (names ? machineNameOf(input[key], names) !== null : /^\s*H5P\.[^\s.]+(?:\s+\d+\.\d+(?:\.\d+)?)?\s*$/.test(input[key]))) + if (keys.length === 1) { + const [key] = keys + named = names ? /** @type {{ machineName: string }} */ (machineNameOf(input[key], names)).machineName : input[key].trim().split(/\s+/)[0] + report(key, `is not a field of a source: the content type is its type: write "type": "${named}"`) + given.type = named + rest = rest.filter((each) => each !== key) } } - if (!LICENSES.includes(/** @type {string} */ (input.license))) report('license', `use one of ${LICENSES.map((each) => JSON.stringify(each)).join(', ')}; "U" when it is not known`) - if (input.licenseVersion !== undefined) { - if (!String(input.license).startsWith('CC ') || input.license === 'CC0 1.0' || input.license === 'CC PDM') report('licenseVersion', 'is for a Creative Commons BY licence only') - else if (!CC_VERSIONS.includes(input.licenseVersion)) report('licenseVersion', `use one of ${CC_VERSIONS.join(', ')}`) + // What a source says of itself, in an object of its own. + for (const key of [...rest]) { + const value = input[key] + if (!isObject(value)) continue + const own = OWN.filter((name) => given[name] === undefined && value[name] !== undefined) + if (!own.length) continue + report(key, `is not a field of a source: ${own.join(', ')} ${own.length === 1 ? 'is' : 'are'} written beside type, not in an object: write ${own.map((name) => `"${name}": …`).join(', ')} at the top`) + for (const name of own) { + given[name] = value[name] + at[name] = `${key}.${name}` + } + for (const inner of Object.keys(value)) { + if (!own.includes(inner)) report(`${key}.${inner}`, OWN.includes(inner) ? 'is written beside type already' : `is not a field of a source; ${fields}`) + } + rest = rest.filter((each) => each !== key) } - if (!Array.isArray(input.authors) || !input.authors.length) report('authors', 'name at least one author: [{ "name": "…", "role": "Author" }]') - else input.authors.forEach((/** @type {unknown} */ author, /** @type {number} */ i) => { - if (!isObject(author) || typeof author.name !== 'string' || !author.name.trim()) report(`authors[${i}].name`, 'expected the author\'s name') - else if (!ROLES.includes(author.role)) report(`authors[${i}].role`, `use one of ${ROLES.join(', ')}`) - else for (const key of Object.keys(author)) if (key !== 'name' && key !== 'role') report(`authors[${i}].${key}`, 'is not part of an author; write name and role') + // The content's fields beside `type`. + if (rest.length && given.params === undefined) { + report('params', `is not written: the content's fields go in it, not beside type: write "params": { ${rest.map((key) => `"${key}": …`).join(', ')} }`) + given.params = Object.fromEntries(rest.map((key) => [key, input[key]])) + } else for (const key of rest) report(key, `is not a field of a source; ${fields}`) + + let typed = false + const type = typeof given.type === 'string' ? given.type.trim() : '' + if (named) typed = true + else if (!type) report(at.type, 'expected a content type\'s machine name, such as "H5P.Blanks"') + else if (names) { + const found = machineNameOf(type, names) + if (!found) { + const nearest = nearestNames(type, names) + report(at.type, `the library pack has no content type ${type}${nearest.length ? `; nearest: ${nearest.join(', ')}` : ''}`) + } else { + typed = true + // checkContent takes a version, as saved content names one; a build takes the pack's newest. + if (found.exact && /\s/.test(type)) report(at.type, `takes the machine name alone, "${found.machineName}": the version is the pack's newest`) + else if (!found.exact) report(at.type, `takes the machine name, "${found.machineName}"`) + given.type = found.machineName + } + } else if (/^\S+\s+\d+\.\d+$/.test(type)) { + const [name] = type.split(/\s/) + report(at.type, `takes the machine name alone, "${name}": the version is the pack's newest`) + given.type = name + typed = true + } else if (!/^\S+$/.test(type)) report(at.type, 'expected a content type\'s machine name, such as "H5P.Blanks"') + else typed = true + + /** @type {Record} */ + const source = { type: typed ? String(given.type).trim() : '', title: '', language: 'en', license: 'U', authors: [], params: {} } + if (typeof given.title !== 'string' || !given.title.trim()) report(at.title, 'expected the activity\'s title') + else if (given.title.length > 255) report(at.title, 'takes at most 255 characters') + else source.title = given.title + let tag = typeof given.language === 'string' + try { + if (tag) Intl.getCanonicalLocales(given.language) + } catch { + tag = false + } + if (tag) source.language = given.language + else report(at.language, 'expected a BCP 47 language tag, such as "en" or "de-AT"') + if (LICENSES.includes(given.license)) source.license = given.license + else report(at.license, `use one of ${LICENSES.map((each) => JSON.stringify(each)).join(', ')}; "U" when it is not known`) + if (given.licenseVersion !== undefined) { + if (!String(given.license).startsWith('CC ') || given.license === 'CC0 1.0' || given.license === 'CC PDM') report(at.licenseVersion, 'is for a Creative Commons BY licence only') + else if (!CC_VERSIONS.includes(given.licenseVersion)) report(at.licenseVersion, `use one of ${CC_VERSIONS.join(', ')}`) + else source.licenseVersion = given.licenseVersion + } + if (!Array.isArray(given.authors) || !given.authors.length) report(at.authors, 'name at least one author: [{ "name": "…", "role": "Author" }]') + else given.authors.forEach((/** @type {unknown} */ author, /** @type {number} */ i) => { + const before = problems.length + if (!isObject(author) || typeof author.name !== 'string' || !author.name.trim()) report(`${at.authors}[${i}].name`, 'expected the author\'s name') + else if (!ROLES.includes(author.role)) report(`${at.authors}[${i}].role`, `use one of ${ROLES.join(', ')}`) + else for (const key of Object.keys(author)) if (key !== 'name' && key !== 'role') report(`${at.authors}[${i}].${key}`, 'is not part of an author; write name and role') + if (problems.length === before) source.authors.push(author) }) - if (input.source !== undefined && (typeof input.source !== 'string' || !/^https?:\/\/\S+$/i.test(input.source))) report('source', 'expected the address the content comes from') - for (const key of ['yearFrom', 'yearTo']) if (input[key] !== undefined && !Number.isInteger(input[key])) report(key, 'expected a year') - if (input.params !== undefined && !isObject(input.params)) report('params', 'expected an object of the content type\'s fields') + if (given.source !== undefined && (typeof given.source !== 'string' || !/^https?:\/\/\S+$/i.test(given.source))) report(at.source, 'expected the address the content comes from') + else if (given.source !== undefined) source.source = given.source + for (const key of ['yearFrom', 'yearTo']) { + if (given[key] !== undefined && !Number.isInteger(given[key])) report(at[key], 'expected a year') + else if (given[key] !== undefined) source[key] = given[key] + } + if (given.params !== undefined && !isObject(given.params)) report('params', 'expected an object of the content type\'s fields') + else if (given.params !== undefined) source.params = given.params + return { source: /** @type {Source} */ (source), problems, typed } +} + +/** + * The general source checked for its own fields; a quiz in the published format mapped onto it. + * @param {unknown} input + * @returns {Source} + * @throws {SourceError} with every problem of its own fields + * @internal + */ +export function readSource(input) { + const { source, problems } = sourceOf(input) if (problems.length) throw new SourceError(problems) - return /** @type {Source} */ ({ ...input, type: String(input.type).trim(), params: input.params ?? {} }) + return source } /** @@ -136,21 +248,25 @@ async function closure(pack, main, content) { * the libraries' parts in the order the package lists them, and what is worth telling. Nothing is * written: the caller writes the package (`writePackage`) and its media. * @template F - * @param {Source} source as `readSource` returned it + * @param {Source} source as `readSource` returned it, or `sourceOf` with its content type known * @param {import('./pack.mjs').Pack} pack * @param {import('./media.mjs').MediaStore} media + * @param {Problem[]} [said] the problems `sourceOf` found in the source's own fields, which come + * first among the content's: all of them in one round (#174) * @returns {Promise<{ manifest: Record, content: Record, selected: Map, install: Map, translations: TranslationReport[], notes: Problem[] }>} * `selected`, the libraries the package plays with; `install`, those an H5P site's upload check wants with them * @internal */ -export async function composePackage(source, pack, media) { +export async function composePackage(source, pack, media, said = []) { /** @type {Problem[]} */ const problems = [] /** @type {Problem[]} */ const notes = [] const report = (/** @type {string} */ path, /** @type {string} */ message) => problems.push({ path, message }) const note = (/** @type {string} */ path, /** @type {string} */ message) => notes.push({ path, message }) - const { entries, main: mainLibrary, translations } = await loadLibraries(pack, source) + const { entries, main: mainLibrary, translations } = await loadLibraries(pack, source).catch((error) => { + throw error instanceof SourceError ? new SourceError([...said, ...error.problems]) : error + }) const main = mainLibrary.library.machineName const authors = source.authors.map((author) => ({ name: escapeText(author.name), role: author.role })) const generator = new ContentGenerator(entries, { @@ -208,7 +324,7 @@ export async function composePackage(source, pack, media) { if (!known && !shortened(path)) problems.push({ path, message }) } for (const added of checked.notes) if (!notes.some(({ path, message }) => path === added.path && message === added.message)) notes.push(added) - if (problems.length) throw new SourceError(problems) + if (said.length || problems.length) throw new SourceError([...said, ...problems]) const content = checked.params const { selected, addons } = await closure(pack, mainLibrary.library, content) diff --git a/packages/author/lib/generator.mjs b/packages/author/lib/generator.mjs index db909e1..79b0ff2 100644 --- a/packages/author/lib/generator.mjs +++ b/packages/author/lib/generator.mjs @@ -1,4 +1,5 @@ import { checkMetadata } from './metadata.mjs' +import { machineNameOf, namingKey } from './names.mjs' import { isInterfaceText } from './schema.mjs' import { allowedTags, markupText, plainText, takesMarkup } from './text.mjs' @@ -101,6 +102,13 @@ export class ContentGenerator { */ #written = new Map() + /** + * Sub-content put together from an object that had its parameters beside its library's name, + * by where those are written: its problems are named there. + * @type {WeakMap} + */ + #flat = new WeakMap() + /** * @param {Map} pack every library folder the content may use, by folder name * @param {GeneratorOptions} [options] @@ -353,6 +361,23 @@ export class ContentGenerator { * @returns {Record} */ #placed(fields, given, path) { + // Sub-content in the group's place that names its library under another key than `library` + // (#174), its parameters in `params` or beside the name: what is no field of the group. + let flat = false + const holders = fields.filter((field) => field.type === 'library') + if (typeof given.library !== 'string' && holders.length) { + const outside = Object.keys(given).filter((key) => !SUB_CONTENT.includes(key) && !fields.some((field) => field.name === key)) + const named = namingKey(pick(given, outside), holders.flatMap((field) => optionNames(field))) + const beside = named ? outside.filter((key) => key !== named.key) : [] + const loose = named && !isObject(given.params) && beside.length > 0 + const meant = !named ? null : loose + ? { ...omit(given, [named.key, ...beside]), library: named.machineName, params: pick(given, beside) } + : { ...omit(given, [named.key]), library: named.machineName } + if (meant && holderOf(fields, meant)) { + given = meant + flat = Boolean(loose) + } + } const holder = holderOf(fields, given) if (holder) { const subContent = pick(given, SUB_CONTENT) @@ -360,6 +385,11 @@ export class ContentGenerator { .map((key) => key === 'library' ? `"library": ${JSON.stringify(given.library)}` : `"${key}": { … }`) const beside = Object.keys(given).filter((key) => !SUB_CONTENT.includes(key) && fields.some((field) => field.name === key)).map((key) => `, "${key}": …`) this.report(path, `is sub-content, but here it goes inside an object, in its field ${holder.name}: write ${path} as { "${holder.name}": { ${shown.join(', ')} }${beside.join('')} }`) + if (flat) { + // Its parameters are where the object is, not in a `params` of it: the nearer move first. + this.#flat.set(subContent, path) + this.#move(path, `${path}.${holder.name}.params`) + } this.#move(path, `${path}.${holder.name}`) given = { ...omit(given, SUB_CONTENT), [holder.name]: subContent } } @@ -483,20 +513,37 @@ export class ContentGenerator { return items.length > 0 || !optional ? items : undefined } case 'library': - if (isObject(written) && typeof written.library === 'string') return this.writtenLibrary(written, stack, path, field) + if (isObject(written) && typeof written.library === 'string') return this.writtenLibrary(written, stack, path, field, this.#flat.get(written)) if (written !== undefined && !this.lenient) { - // The parameters of the one library the field takes, without the sub-content around - // them, as a book's chapters written `{ "content": [ … ] }` (#172): said so, and checked - // as its parameters in the same round, named by where they are written. - const entry = !this.saved && isObject(written) ? this.#only(field) : null - const own = entry ? Object.keys(written).filter((key) => (entry.semantics ?? []).some((/** @type {any} */ each) => each.name === key)) : [] + // Sub-content that names its library under another key than `library`, `contentType` or + // `type`, as a source written with no skill beside it does (#174): the one text in it + // that names a library the field takes. + const named = !this.saved && isObject(written) ? namingKey(omit(written, SUB_CONTENT), optionNames(field)) : null + const major = named ? libraryOptions(field).find((option) => option.machineName === named.machineName)?.major : undefined + const entry = this.saved || !isObject(written) ? null : named ? this.find(named.machineName, major) : this.#only(field) + if (named && isObject(written.params)) { + this.report(`${path}.${named.key}`, `is the sub-content's library: write "library": "${named.machineName}"`) + // Its parameters are checked where the library's fields are at hand. + return entry?.semantics ? this.writtenLibrary({ ...omit(written, [named.key]), library: named.machineName }, stack, path, field) : undefined + } + // The parameters of sub-content without the sub-content around them: of the one library + // the field takes, as a book's chapters written `{ "content": [ … ] }` (#172), or of the + // one named beside them, `{ "type": "H5P.MultiChoice", "question": … }` (#174). Said so, + // and checked as its parameters in the same round, named by where they are written. + let params = named ? omit(written, [named.key]) : written + const own = entry ? Object.keys(params).filter((key) => (entry.semantics ?? []).some((/** @type {any} */ each) => each.name === key)) : [] if (entry && own.length) { - const metadata = own.includes('metadata') ? undefined : written.metadata - this.report(path, `is the params of sub-content, without the sub-content around them: write ${path} as { "library": "${entry.library.machineName}", "params": { "${own[0]}": … }${metadata === undefined ? '' : ', "metadata": { … }'} }`) + let metadata = own.includes('metadata') ? undefined : params.metadata + // A title beside them that is no field of the library is the sub-content's own, as a + // chapter's is: its `metadata.title`. + const titled = metadata === undefined && typeof params.title === 'string' && !own.includes('title') + if (titled) metadata = { title: params.title } + if (metadata !== undefined) params = omit(params, titled ? ['title'] : ['metadata']) + this.report(path, `is the params of sub-content, without the sub-content around them: write ${path} as { "library": "${entry.library.machineName}"${metadata === undefined ? '' : titled ? ', "metadata": { "title": … }' : ', "metadata": { … }'}, "params": { "${own[0]}": … } }`) this.#move(path, `${path}.params`) - return this.writtenLibrary({ library: entry.library.machineName, params: metadata === undefined ? written : omit(written, ['metadata']), ...(metadata === undefined ? {} : { metadata }) }, stack, path, field, path) + return this.writtenLibrary({ library: entry.library.machineName, params, ...(metadata === undefined ? {} : { metadata }) }, stack, path, field, path) } - this.report(path, `expected sub-content, ${subContentOf(field, written)}`) + this.report(path, `expected sub-content, ${subContentOf(field, named ? named.machineName : written)}`) return undefined } // A default names the content the editor starts with (h5peditor.js puts it in the @@ -657,6 +704,12 @@ export class ContentGenerator { if (!entry) return written return { ...this.subContent(entry, written.params, undefined, stack, path), ...omit(written, ['params']), library: libraryString(entry.library) } } + // A library named loosely, "multichoice", where only one the field takes can be meant (#174). + const loose = field && !this.saved ? machineNameOf(written.library, optionNames(field)) : null + if (loose && !loose.exact) { + this.report(`${path}.library`, `takes the machine name, "${loose.machineName}"`) + written = { ...written, library: loose.machineName } + } const match = /^\s*(\S+?)(?:\s+(\d+)\.(\d+))?\s*$/.exec(written.library) if (!match) { this.report(`${path}.library`, `expected a library's machine name, such as ${optionNames(field)[0] ?? '"H5P.MultiChoice"'}`) @@ -690,6 +743,11 @@ export class ContentGenerator { const minor = match[3] === undefined ? undefined : Number(match[3]) return this.subContent(entry, written.params, written.metadata, stack, path, { id: written.subContentId, minor, closed: started }) } + // Sub-content's title is its `metadata.title`; written beside `library`, it is taken for it. + if (typeof written.title === 'string' && written.metadata === undefined) { + this.report(`${path}.title`, 'is the sub-content\'s title, which is in its metadata: write "metadata": { "title": … }') + written = { ...omit(written, ['title']), metadata: { title: written.title } } + } for (const key of Object.keys(written)) { if (!['library', 'params', 'metadata'].includes(key)) this.report(`${path}.${key}`, 'is not part of sub-content; write library, params and metadata') } diff --git a/packages/author/lib/libraries.mjs b/packages/author/lib/libraries.mjs index be4ec45..b107c40 100644 --- a/packages/author/lib/libraries.mjs +++ b/packages/author/lib/libraries.mjs @@ -1,5 +1,6 @@ import { ownLibraries } from '@missing-elements/h5p-offline-player/plan' import { libraryOptions, libraryString } from './generator.mjs' +import { machineNameOf } from './names.mjs' import { SourceError } from './source-error.mjs' import { localize, translationFiles } from './translate.mjs' import { patchSemantics } from './types.mjs' @@ -74,6 +75,11 @@ export async function loadLibraries(pack, { type, language, params }) { const namedFolders = new Set() /** @type {Map} by folder, in the order loaded */ const reports = new Map() + const names = [...new Set(pack.libraries.map((entry) => entry.machineName))] + /** Every major of a library the generator may take what is written to mean, as one a field takes unnamed. */ + const meant = (/** @type {string} */ machineName) => { + for (const entry of pack.libraries) if (entry.machineName === machineName) queue.push({ machineName, major: entry.majorVersion, unnamed: true }) + } /** @param {unknown} value */ const named = (value) => { if (Array.isArray(value)) value.forEach(named) @@ -83,6 +89,20 @@ export async function loadLibraries(pack, { type, language, params }) { // pack has: each is loaded. if (match && match[2] !== undefined) queue.push({ machineName: match[1], major: Number(match[2]) }) else if (match) for (const entry of pack.libraries) if (entry.machineName === match[1]) queue.push({ machineName: match[1], major: entry.majorVersion }) + // What the generator takes to name a library though it is not written as one (#174), so + // that what is written for it is checked: `library` written loosely, "multichoice", and a + // machine name under another key, `contentType` or `type`. A name under another key is + // taken only as it is, so that a text that reads like one, a slide titled "Summary", loads + // nothing. + if (typeof value.library === 'string') { + const found = machineNameOf(value.library, names) + if (found && !found.exact) meant(found.machineName) + } else { + for (const text of Object.values(value)) { + const found = typeof text === 'string' && text.length <= 64 ? machineNameOf(text, names) : null + if (found?.exact) meant(found.machineName) + } + } Object.values(value).forEach(named) } } diff --git a/packages/author/lib/names.mjs b/packages/author/lib/names.mjs new file mode 100644 index 0000000..3442091 --- /dev/null +++ b/packages/author/lib/names.mjs @@ -0,0 +1,70 @@ +/** + * A library named another way than by its machine name. A source written with no skill beside it + * names a content type as it is spoken of, "interactive-book", "Question Set", "dialogcards", and + * the build said the pack has no such type, a round for each guess (#174). What can only mean one + * library is taken to mean it, and the problem says the name to write: the same letters and digits + * in any case, with or without `H5P.` before them and a version after. A library's title is not + * tried: the pack's index has none, and reading them would open every part. + */ + +/** + * A name's letters and digits, lower case, without `H5P.` before them or a version after. + * @param {string} text + */ +const letters = (text) => text.toLowerCase().replace(/\s+\d+(?:\.\d+)*\s*$/, '').replace(/^\s*h5p(?![a-z0-9])[\s._-]*/, '').replace(/[^a-z0-9]/g, '') + +/** + * The machine name that was written, of the names given: the name itself, with a version or + * without, or the one name of the same letters. `exact` is false for one written loosely. + * @param {unknown} written + * @param {Iterable} names machine names + * @returns {{ machineName: string, exact: boolean } | null} + */ +export function machineNameOf(written, names) { + if (typeof written !== 'string' || !written.trim()) return null + const all = [...new Set(names)] + const match = /^\s*(\S+?)(?:\s+\d+\.\d+(?:\.\d+)?)?\s*$/.exec(written) + if (match && all.includes(match[1])) return { machineName: match[1], exact: true } + const key = letters(written) + if (!key) return null + const same = all.filter((name) => letters(name) === key) + return same.length === 1 ? { machineName: same[0], exact: false } : null +} + +/** + * The names nearest to what was written, for a problem to offer: those its letters are part of, + * "book", and those its letters end with, "fill-in-the-blanks". Not one they begin with: the + * "image" of "image choice" is no lead to it. At most five. + * @param {unknown} written + * @param {Iterable} names machine names + * @returns {string[]} + */ +export function nearestNames(written, names) { + const key = typeof written === 'string' ? letters(written) : '' + if (key.length < 3) return [] + return [...new Set(names)].filter((name) => { + const own = letters(name) + return own.length >= 3 && (own.includes(key) || key.endsWith(own)) + }).slice(0, 5) +} + +/** + * The key under which an object names a library where `library` would, as `contentType` or `type` + * does in a source written with no skill beside it: its one text that names a library of those + * given. None where two keys do. + * @param {Record} given + * @param {Iterable} names machine names + * @returns {{ key: string, machineName: string } | null} + */ +export function namingKey(given, names) { + const all = [...new Set(names)] + /** @type {{ key: string, machineName: string }[]} */ + const found = [] + for (const [key, value] of Object.entries(given)) { + // A name is short: a sentence that happens to hold one is not it. + if (typeof value !== 'string' || value.length > 64) continue + const named = machineNameOf(value, all) + if (named) found.push({ key, machineName: named.machineName }) + } + return found.length === 1 ? found[0] : null +} diff --git a/packages/author/lib/node.mjs b/packages/author/lib/node.mjs index 0f14bd0..bb7e6b2 100644 --- a/packages/author/lib/node.mjs +++ b/packages/author/lib/node.mjs @@ -5,10 +5,11 @@ import { Readable, Writable } from 'node:stream' import { fileURLToPath, pathToFileURL } from 'node:url' import { Uint8ArrayReader, ZipWriter } from '@zip.js/zip.js' import { normalizeArchive } from '@missing-elements/h5p-normalize' -import { composePackage, readSource, writePackage } from './build.mjs' +import { composePackage, sourceOf, typeNames, writePackage } from './build.mjs' import { bundleLibraries, contentTypes } from './bundle.mjs' import { MediaStore } from './media.mjs' import { Pack } from './pack.mjs' +import { SourceError } from './source-error.mjs' /** * What only Node does of a build: the pack read from disk, a source's files read beside it and @@ -142,13 +143,16 @@ export async function buildPackage(input, output, options = {}) { // Before any work, and again as the package lands, for a file that appears in between. if (await lstat(output).then(() => true, () => false)) throw exists(output) if (!(await stat(dirname(output)).catch(() => null))?.isDirectory()) throw new Error(`${dirname(output)} is not a folder to write the package in`) - const source = readSource(input) const pack = await openPack(options.libraries) let temporary try { + // With the pack's names, so that a content type named loosely is known, and the content of a + // source whose own fields have problems is checked in the same round (#174). + const read = sourceOf(input, typeNames(pack)) + if (!read.typed) throw new SourceError(read.problems) temporary = await mkdtemp(join(dirname(output), '.h5p-author-')) const media = new MediaStore(nodeMedia({ base: options.base ?? process.cwd(), temporary }), { fetch: options.fetch }) - const built = await composePackage(source, pack, media) + const built = await composePackage(read.source, pack, media, read.problems) const staged = join(temporary, 'package.h5p') const file = await open(staged, 'wx') try { diff --git a/packages/author/tests/build.test.ts b/packages/author/tests/build.test.ts index c3a56e6..87a54b4 100644 --- a/packages/author/tests/build.test.ts +++ b/packages/author/tests/build.test.ts @@ -319,8 +319,12 @@ describe('any content type in the pack', () => { it('lists every problem a source has at once, each by its path, and writes nothing', async () => { expect(await problems({ ...META, type: 'H5P.Nope', params: {} })).toEqual(['type: the library pack has no content type H5P.Nope']) // A version is saved content's, which checkContent takes; a build plays the pack's newest (#162). - expect(await problems({ ...META, type: 'H5P.Blanks 1.14', params: {} })).toEqual(['type: takes the machine name alone, "H5P.Blanks": the version is the pack\'s newest']) - expect(await problems({ ...META, license: 'MIT', authors: [{ name: 'A' }], extra: 1, type: 'H5P.Blanks' })).toEqual([ + // What is wrong in the content is said in the same round as what is wrong beside it (#174). + expect(await problems({ ...META, type: 'H5P.Blanks 1.14', params: {} })).toEqual([ + 'type: takes the machine name alone, "H5P.Blanks": the version is the pack\'s newest', + 'params.questions: is required: write at least 1 item' + ]) + expect(await problems({ ...META, license: 'MIT', authors: [{ name: 'A' }], extra: 1, type: 'H5P.Blanks', params: { questions: ['The *Danube* flows east.'] } })).toEqual([ 'extra: is not a field of a source; the fields are type, title, language, license, licenseVersion, authors, source, yearFrom, yearTo, params', expect.stringMatching(/^license: use one of/), 'authors[0].role: use one of Author, Editor, Licensee, Originator' @@ -345,10 +349,10 @@ describe('any content type in the pack', () => { expect(await problems({ ...META, type: 'H5P.InteractiveBook', params: { chapters: [ { title: 'Sources', content: [{ library: 'H5P.AdvancedText', params: { text: 'A river begins at its source.' } }] } ] } })).toEqual([ - 'params.chapters[0]: is the params of sub-content, without the sub-content around them: write params.chapters[0] as { "library": "H5P.Column", "params": { "content": … } }', - 'params.chapters[0].content[0]: is sub-content, but here it goes inside an object, in its field content: write params.chapters[0].content[0] as { "content": { "library": "H5P.AdvancedText", "params": { … } } }', - 'params.chapters[0].title: is not a field here; the fields are content', - 'params.chapters[0].metadata.title: is required: the chapter\'s name in the table of contents' + // Its title beside them is the chapter's own, shown where it goes (#174): it was two more + // problems, "is not a field here" and "metadata.title: is required". + 'params.chapters[0]: is the params of sub-content, without the sub-content around them: write params.chapters[0] as { "library": "H5P.Column", "metadata": { "title": … }, "params": { "content": … } }', + 'params.chapters[0].content[0]: is sub-content, but here it goes inside an object, in its field content: write params.chapters[0].content[0] as { "content": { "library": "H5P.AdvancedText", "params": { … } } }' ]) // A chapter refused whole leaves the list shorter: no count of what is left, and nothing said // of the chapter after it under the index of the one refused. @@ -377,6 +381,84 @@ describe('any content type in the pack', () => { ]) }) + it('checks the content in the same build as the source\'s own fields, and takes what its outer shape plainly means (#174)', async () => { + // A book as Gemini wrote it from a link to the skill and none of its text: the content type as + // `contentType`, the source's own fields in `metadata`, as H5P keeps them, and sub-content that + // names its library the same way. It was seven problems, all of the outer shape, and the round + // after thirty, three an item, one of them a list of 35 libraries. + expect(await problems({ + contentType: 'H5P.InteractiveBook', + metadata: { title: 'Physics', license: 'CC BY', authors: [{ name: 'Ann Author', role: 'Author' }], defaultLanguage: 'en' }, + params: { chapters: [{ title: 'Newton', content: [{ contentType: 'H5P.AdvancedText', params: { text: '

F = ma

' } }] }] } + })).toEqual([ + 'contentType: is not a field of a source: the content type is its type: write "type": "H5P.InteractiveBook"', + 'metadata: is not a field of a source: title, license, authors are written beside type, not in an object: write "title": …, "license": …, "authors": … at the top', + 'metadata.defaultLanguage: is not a field of a source; the fields are type, title, language, license, licenseVersion, authors, source, yearFrom, yearTo, params', + 'language: expected a BCP 47 language tag, such as "en" or "de-AT"', + 'params.chapters[0]: is the params of sub-content, without the sub-content around them: write params.chapters[0] as { "library": "H5P.Column", "metadata": { "title": … }, "params": { "content": … } }', + 'params.chapters[0].content[0]: is sub-content, but here it goes inside an object, in its field content: write params.chapters[0].content[0] as { "content": { "library": "H5P.AdvancedText", "params": { … } } }', + expect.stringMatching(/^params\.chapters\[0\]\.content\[0\]\.params\.text: has

, which this field does not take/) + ]) + // As the weakest model wrote one with no skill: the content's fields beside `type`, the type + // by the name it is spoken of. Each round had gone to one of these, with nothing inside looked at. + const chapter = { library: 'H5P.Column', metadata: { title: 'Newton' }, params: { content: [{ content: { library: 'H5P.AdvancedText', params: { text: 'F = ma' } } }] } } + expect(await problems({ type: 'interactive-book', title: 'Physics', chapters: [chapter, 'Einstein'] })).toEqual([ + 'params: is not written: the content\'s fields go in it, not beside type: write "params": { "chapters": … }', + 'type: takes the machine name, "H5P.InteractiveBook"', + 'language: expected a BCP 47 language tag, such as "en" or "de-AT"', + expect.stringMatching(/^license: use one of/), + 'authors: name at least one author: [{ "name": "…", "role": "Author" }]', + 'params.chapters[1]: expected sub-content, { "library": "H5P.Column", "params": { … } }' + ]) + // With `params` written, what is beside `type` is no field of the content. + expect(await problems({ ...META, type: 'H5P.InteractiveBook', chapters: [], params: { chapters: [chapter] } })).toEqual([ + 'chapters: is not a field of a source; the fields are type, title, language, license, licenseVersion, authors, source, yearFrom, yearTo, params' + ]) + // A source's own field that the top has too is said so, and a year in the object is taken. + expect(await problems({ ...META, type: 'H5P.InteractiveBook', metadata: { title: 'Other', yearFrom: 2020 }, params: { chapters: [chapter] } })).toEqual([ + 'metadata: is not a field of a source: yearFrom is written beside type, not in an object: write "yearFrom": … at the top', + 'metadata.title: is written beside type already' + ]) + + // A type named as it is spoken of is the one type of those letters; one that is none has the nearest named. + expect(await problems({ ...META, type: 'Question Set', params: { questions: [{ library: 'H5P.TrueFalse', params: { question: 'Yes?', correct: 'true' } }] } })).toEqual(['type: takes the machine name, "H5P.QuestionSet"']) + expect(await problems({ ...META, type: 'book', params: {} })).toEqual(['type: the library pack has no content type book; nearest: H5P.InteractiveBook']) + expect(await problems({ ...META, type: 'fill-in-the-blanks', params: {} })).toEqual(['type: the library pack has no content type fill-in-the-blanks; nearest: H5P.Blanks']) + // Two texts beside the fields that name a type: neither is taken for it. + expect(await problems({ ...META, contentType: 'H5P.Blanks', mainLibrary: 'H5P.DragText', params: {} })).toEqual([ + expect.stringMatching(/^contentType: is not a field of a source; the fields are/), + expect.stringMatching(/^mainLibrary: is not a field of a source; the fields are/), + 'type: expected a content type\'s machine name, such as "H5P.Blanks"' + ]) + + // Sub-content with its library under another key and its parameters beside it, in a field + // that takes several libraries: the parameters of the one it names, checked as such. + expect(await problems({ ...META, type: 'H5P.QuestionSet', params: { questions: [ + { type: 'H5P.MultiChoice', question: 'Which?', answers: [{ text: 'a', correct: true }, { text: 'b', correct: false }], sideways: 1 } + ] } })).toEqual([ + 'params.questions[0]: is the params of sub-content, without the sub-content around them: write params.questions[0] as { "library": "H5P.MultiChoice", "params": { "question": … } }', + expect.stringMatching(/^params\.questions\[0\]\.sideways: is not a field here; the fields are media, question, answers/) + ]) + // The same in the place of the group that holds it: what is no field of the group is its parameters. + expect(await problems({ ...META, type: 'H5P.CoursePresentation', params: { presentation: { slides: [ + { elements: [{ type: 'H5P.AdvancedText', x: 5, y: 5, width: 90, height: 20, text: '

Water

' }] } + ] } } })).toEqual([ + 'params.presentation.slides[0].elements[0]: is sub-content, but here it goes inside an object, in its field action: write params.presentation.slides[0].elements[0] as { "action": { "library": "H5P.AdvancedText", "params": { … } }, "x": …, "y": …, "width": …, "height": … }', + expect.stringMatching(/^params\.presentation\.slides\[0\]\.elements\[0\]\.text: has

, which this field does not take/) + ]) + // A library named loosely, a title beside `library`, and two keys that each name a library. + expect(await problems({ ...META, type: 'H5P.Column', params: { content: [{ content: { library: 'advancedtext', params: { text: 'x' } } }] } })).toEqual([ + 'params.content[0].content.library: takes the machine name, "H5P.AdvancedText"' + ]) + expect(await problems({ ...META, type: 'H5P.InteractiveBook', params: { chapters: [{ ...chapter, metadata: undefined, title: 'Newton' }] } })).toEqual([ + 'params.chapters[0].title: is the sub-content\'s title, which is in its metadata: write "metadata": { "title": … }' + ]) + expect(await problems({ ...META, type: 'H5P.QuestionSet', params: { questions: [{ type: 'H5P.MultiChoice', kind: 'H5P.TrueFalse', question: 'Which?' }] } })).toEqual([ + expect.stringMatching(/^params\.questions\[0\]: expected sub-content, \{ "library": "…", "params": \{ … \} \}, of H5P\.MultiChoice, /) + ]) + expect((await readdir(dir)).filter(name => name.startsWith('failed-') || name.startsWith('.h5p-author-'))).toEqual([]) + }) + it('takes sub-content by machine name, at the pack\'s version, with its title', async () => { const { content, manifest } = await build({ ...META, diff --git a/packages/author/tests/generator.test.ts b/packages/author/tests/generator.test.ts index 9280dd8..e3a6322 100644 --- a/packages/author/tests/generator.test.ts +++ b/packages/author/tests/generator.test.ts @@ -322,11 +322,12 @@ describe('content authored from semantics', () => { metadata: (entry, written) => ({ contentType: entry.library.title, title: written?.title }) }) expect(problems.map(({ path, message }) => `${path}: ${message}`)).toEqual([ - 'params.chapters[0]: is the params of sub-content, without the sub-content around them: write params.chapters[0] as { "library": "H5P.AdvancedText", "params": { "text": … } }', + // A title beside them that is no field of the library is the sub-content's own (#174). + 'params.chapters[0]: is the params of sub-content, without the sub-content around them: write params.chapters[0] as { "library": "H5P.AdvancedText", "metadata": { "title": … }, "params": { "text": … } }', expect.stringMatching(/^params\.chapters\[0\]\.text: has

, which this field does not take/), - 'params.chapters[0].title: is not a field here; the fields are text', - 'params.chapters[1]: is the params of sub-content, without the sub-content around them: write params.chapters[1] as { "library": "H5P.AdvancedText", "params": { "text": … }, "metadata": { … } }' + 'params.chapters[1]: is the params of sub-content, without the sub-content around them: write params.chapters[1] as { "library": "H5P.AdvancedText", "metadata": { … }, "params": { "text": … } }' ]) + expect(params.chapters[0].metadata.title).toBe('First') expect(params.chapters[1]).toMatchObject({ library: 'H5P.AdvancedText 1.1', params: { text: '
Two
' }, metadata: { title: 'Second' } }) expect(moved).toEqual([{ from: 'params.chapters[0]', to: 'params.chapters[0].params' }, { from: 'params.chapters[1]', to: 'params.chapters[1].params' }]) expect(shortened).toEqual([]) @@ -335,6 +336,71 @@ describe('content authored from semantics', () => { expect(other.problems).toEqual([{ path: 'params.chapters[0]', message: 'expected sub-content, { "library": "H5P.AdvancedText", "params": { … } }' }]) }) + it('takes sub-content that names its library under another key, its params in params or beside the name (#174)', () => { + const CHOICE = { machineName: 'H5P.MultiChoice', minor: 16, title: 'Multiple Choice', semantics: [{ name: 'question', type: 'text', label: 'Question' }] } + const titled: GeneratorOptions = { ...STRICT, metadata: (entry, written) => ({ contentType: entry.library.title, title: written?.title }) } + const show = (problems: { path: string, message: string }[]) => problems.map(({ path, message }) => `${path}: ${message}`) + // A field that takes two libraries: the one text in the object that names one says which. + const set = [{ name: 'questions', type: 'list', min: 1, field: { name: 'question', type: 'library', options: ['H5P.MultiChoice 1.16', 'H5P.AdvancedText 1.1'] } }] + const { params, problems, moved } = generate(set, { questions: [ + { contentType: 'H5P.MultiChoice', params: { question: 'Which?', sideways: 1 } }, + { type: 'advancedtext', text: '

Two

', title: 'Second' }, + { type: 'H5P.MultiChoice', kind: 'H5P.AdvancedText', question: 'Which?' }, + { type: 'essay', question: 'Why?' } + ] }, [TEXT, CHOICE], titled) + expect(show(problems)).toEqual([ + 'params.questions[0].contentType: is the sub-content\'s library: write "library": "H5P.MultiChoice"', + 'params.questions[0].params.sideways: is not a field here; the fields are question', + // Beside the name, they are its parameters, and a title that is no field of it its own. + 'params.questions[1]: is the params of sub-content, without the sub-content around them: write params.questions[1] as { "library": "H5P.AdvancedText", "metadata": { "title": … }, "params": { "text": … } }', + expect.stringMatching(/^params\.questions\[1\]\.text: has

, which this field does not take/), + // Two keys that each name a library, and a name that is none the field takes: nothing is guessed. + 'params.questions[2]: expected sub-content, { "library": "…", "params": { … } }, of H5P.MultiChoice, H5P.AdvancedText', + 'params.questions[3]: expected sub-content, { "library": "…", "params": { … } }, of H5P.MultiChoice, H5P.AdvancedText' + ]) + expect(params.questions[0]).toMatchObject({ library: 'H5P.MultiChoice 1.16', params: { question: 'Which?' } }) + expect(params.questions[1]).toMatchObject({ library: 'H5P.AdvancedText 1.1', metadata: { title: 'Second' } }) + expect(moved).toEqual([{ from: 'params.questions[1]', to: 'params.questions[1].params' }]) + + // In the place of the group that holds it, what is no field of the group is its parameters, + // named by where the source has them; with `params` written, it stays what it is. + const slide = [{ name: 'elements', type: 'list', field: { name: 'element', type: 'group', fields: [ + { name: 'action', type: 'library', options: ['H5P.AdvancedText 1.1', 'H5P.MultiChoice 1.16'] }, + { name: 'x', type: 'number', default: 0 } + ] } }] + const held = generate(slide, { elements: [ + { type: 'H5P.AdvancedText', x: 5, text: '

One

' }, + { contentType: 'H5P.MultiChoice 1.16', params: { question: 'Which?' }, colour: 'red' } + ] }, [TEXT, CHOICE], STRICT) + expect(show(held.problems)).toEqual([ + 'params.elements[0]: is sub-content, but here it goes inside an object, in its field action: write params.elements[0] as { "action": { "library": "H5P.AdvancedText", "params": { … } }, "x": … }', + expect.stringMatching(/^params\.elements\[0\]\.text: has

, which this field does not take/), + 'params.elements[1]: is sub-content, but here it goes inside an object, in its field action: write params.elements[1] as { "action": { "library": "H5P.MultiChoice", "params": { … } } }', + 'params.elements[1].colour: is not a field here; the fields are action, x' + ]) + expect(held.params.elements[0]).toMatchObject({ x: 5, action: { library: 'H5P.AdvancedText 1.1' } }) + // The nearer move first, so that what a later check finds in the parameters is named where they are written. + expect(held.moved).toEqual([ + { from: 'params.elements[0]', to: 'params.elements[0].action.params' }, { from: 'params.elements[0]', to: 'params.elements[0].action' }, + { from: 'params.elements[1]', to: 'params.elements[1].action' } + ]) + + // `library` written loosely is the one library of those letters the field takes, and a title beside it the sub-content's. + const loose = generate(set, { questions: [ + { library: 'multichoice', params: { question: 'Which?' } }, + { library: 'H5P.MultiChoice', title: 'First', params: { question: 'Which?' } } + ] }, [TEXT, CHOICE], titled) + expect(show(loose.problems)).toEqual([ + 'params.questions[0].library: takes the machine name, "H5P.MultiChoice"', + 'params.questions[1].title: is the sub-content\'s title, which is in its metadata: write "metadata": { "title": … }' + ]) + expect(loose.params.questions.map(({ library }: { library: string }) => library)).toEqual(['H5P.MultiChoice 1.16', 'H5P.MultiChoice 1.16']) + expect(loose.params.questions[1].metadata.title).toBe('First') + // Saved content is as it is: a name under another key is no library of it. + const saved = generate(set, { questions: [{ contentType: 'H5P.MultiChoice', params: { question: 'Which?' } }] }, [TEXT, CHOICE], { saved: true }) + expect(show(saved.problems)).toEqual(['params.questions[0]: expected sub-content, { "library": "…", "params": { … } }, of H5P.MultiChoice, H5P.AdvancedText']) + }) + it('says where a field written above the group that has it belongs, and checks it there in the same round (#172)', () => { const semantics = [ { name: 'presentation', type: 'group', fields: [ diff --git a/packages/author/tests/names.test.ts b/packages/author/tests/names.test.ts new file mode 100644 index 0000000..633b70b --- /dev/null +++ b/packages/author/tests/names.test.ts @@ -0,0 +1,49 @@ +import { describe, expect, it } from 'vitest' +import { machineNameOf, namingKey, nearestNames } from '../lib/names.mjs' + +// The names a chat with no skill beside it wrote for a content type (#174), against a pack's. +const NAMES = ['H5P.InteractiveBook', 'H5P.Blanks', 'H5P.AdvancedBlanks', 'H5P.Dialogcards', 'H5P.MultiChoice', 'H5P.QuestionSet', 'H5P.CoursePresentation', 'H5P.Flashcards', 'H5P.Text', 'H5P.AdvancedText', 'H5PEditor.Blanks'] + +describe('a library named another way than by its machine name', () => { + it('is the one library of those letters, with H5P. or without, in any case', () => { + for (const [written, machineName] of [ + ['interactive-book', 'H5P.InteractiveBook'], ['interactiveBook', 'H5P.InteractiveBook'], ['Interactive Book', 'H5P.InteractiveBook'], + ['blanks', 'H5P.Blanks'], ['h5p blanks', 'H5P.Blanks'], ['H5P_Blanks', 'H5P.Blanks'], + ['dialog-cards', 'H5P.Dialogcards'], ['multichoice', 'H5P.MultiChoice'], ['Question Set', 'H5P.QuestionSet'], ['Course Presentation', 'H5P.CoursePresentation'] + ]) expect(machineNameOf(written, NAMES), written).toEqual({ machineName, exact: false }) + // The name itself, with a version or without, is exact; a version on a loose name is left off too. + expect(machineNameOf('H5P.Blanks', NAMES)).toEqual({ machineName: 'H5P.Blanks', exact: true }) + expect(machineNameOf(' H5P.Blanks 1.14 ', NAMES)).toEqual({ machineName: 'H5P.Blanks', exact: true }) + expect(machineNameOf('blanks 1.14', NAMES)).toEqual({ machineName: 'H5P.Blanks', exact: false }) + // An editor library keeps its H5PEditor: "blanks" is not it. + expect(machineNameOf('h5peditorblanks', NAMES)).toEqual({ machineName: 'H5PEditor.Blanks', exact: false }) + }) + + it('is none where the letters are no library\'s, or two libraries\'', () => { + // A title that is not the machine name's letters: the pack's index has no titles. + for (const written of ['book', 'fill-in-the-blanks', 'Multiple Choice', 'H5P.Nope', '', ' ', 'H5P.', 7, null, undefined]) expect(machineNameOf(written, NAMES), String(written)).toBeNull() + expect(machineNameOf('ab', ['H5P.AB', 'H5P.Ab'])).toBeNull() + }) + + it('offers the nearest names: those the letters are part of, and those the letters end with', () => { + expect(nearestNames('book', NAMES)).toEqual(['H5P.InteractiveBook']) + expect(nearestNames('fill-in-the-blanks', NAMES)).toEqual(['H5P.Blanks']) + // A name the letters begin with is no lead: an "image choice" is not an H5P.Image. + expect(nearestNames('H5P.ImageChoice', [...NAMES, 'H5P.Image'])).toEqual([]) + expect(nearestNames('text', NAMES)).toEqual(['H5P.Text', 'H5P.AdvancedText']) + expect(nearestNames('Multiple Choice', NAMES)).toEqual([]) + // Too short to mean anything, and no more than five. + expect(nearestNames('bo', NAMES)).toEqual([]) + expect(nearestNames('h5p.a', ['H5P.A1', 'H5P.A2'])).toEqual([]) + expect(nearestNames('card', Array.from({ length: 8 }, (_, i) => `H5P.Card${i}`))).toHaveLength(5) + }) + + it('finds the key under which an object names a library, where one key does', () => { + expect(namingKey({ contentType: 'H5P.AdvancedText', text: 'Hello' }, NAMES)).toEqual({ key: 'contentType', machineName: 'H5P.AdvancedText' }) + expect(namingKey({ type: 'multichoice', question: 'Which?' }, NAMES)).toEqual({ key: 'type', machineName: 'H5P.MultiChoice' }) + // Two keys that each name one, a text that only holds a name, and nothing that names one. + expect(namingKey({ type: 'text', text: 'Blanks' }, NAMES)).toBeNull() + expect(namingKey({ text: 'Fill in the blanks of this long sentence about H5P.Blanks, which is no name of a library but a sentence' }, NAMES)).toBeNull() + expect(namingKey({ question: 'Which?', answers: ['H5P.Blanks'] }, NAMES)).toBeNull() + }) +}) From 8cc0be3c5b76dd2c39798343357cd80e94a275dc Mon Sep 17 00:00:00 2001 From: Aleksandr Shevchuk Date: Tue, 10 Nov 2026 16:51:34 +0200 Subject: [PATCH 2/4] docs(author): 259 of the 263 stored sources keep their problems, not 257 (#174) Counted again with the nearest names as they are now, those the letters end with and not those they begin with: the two sources that named H5P.ImageChoice have the problem they had, with no H5P.Image offered. The four that differ are the books whose chapters had a title beside them. The count in the commit before this one, 257, was of the rule before. --- design/author.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/design/author.md b/design/author.md index f782a6f..44eca45 100644 --- a/design/author.md +++ b/design/author.md @@ -424,8 +424,9 @@ to 3,000. Nineteen things the code does not say by itself: problems a chapter. With it 10 of the 12 were right by the second round and all 12 by the third, and every one - plays. Of the 263 sources the earlier tests stored, written with the skill's text, 257 have the - same problems and none builds with one build and not the other; the 36 sources of the type + plays. Of the 263 sources the earlier tests stored, written with the skill's text, 259 have the + same problems, the other four being books whose chapters had a title beside them, 13 problems + now 7, and none builds with one build and not the other; the 36 sources of the type files and the quiz format build the same package. `readSource` keeps its contract, a source's own problems thrown, without the pack's names. What the build page must do with it is its own: hand the type's file back for a type that is named loosely, which its rule, the source's `type` From ec952b815f60abf68807a70b901a12b0cd5a6ce4 Mon Sep 17 00:00:00 2001 From: Aleksandr Shevchuk Date: Tue, 10 Nov 2026 17:21:18 +0200 Subject: [PATCH 3/4] fix(author): a library is named only under type or contentType, and keeps what is written with it (#174) From the review of #175, cubic's eleven findings and CodeQL's alert: - A name under `type` or `contentType` written loosely, "advancedtext", was taken by the generator but not loaded, so in a field that takes several libraries what was written for it was checked against no fields ("is not a field here; the fields are" and none). The loader now loads what the generator takes: a name under those keys, as it is or loosely. - Only those keys name sub-content's library, and `contentType` or `library` a source's own type. Under any key, a title "Text", a card's answer "table" or a text that is a library's name was taken for one; in the 328 sources the tests stored, every name found under another key was such a text. - A version written with the name is kept where the field takes that major, "H5P.Child 2.0", and not the field's first. - A title beside sub-content in its group's place, with its parameters beside the name, in `params` or with `library`, is its `metadata.title`, not a parameter or a field of the group. - Only `metadata` holds a source's own fields: Question Set's `introPage` has a title too, and was taken for them in a source with none at the top. - "Question Set 2" is no Question Set: a number with no point is no version. A version of three parts is read loosely, since what reads a library's name takes two. - Names are read without a regular expression that backtracks (CodeQL js/polynomial-redos), and a text longer than 64 characters is no name. - The design note says what reading titles would cost, every part, instead of that a page cannot. The 263 stored sources have the same problems as with the commit before, the 36 sources of the type files and the quiz format build the same package as 0.7.0, every entry, note and translation report, and the 14 failed rounds of the no-skill test give the problems the model was given. --- design/author.md | 20 ++++--- packages/author/README.md | 4 +- packages/author/lib/build.mjs | 41 +++++++-------- packages/author/lib/generator.mjs | 60 ++++++++++++++------- packages/author/lib/libraries.mjs | 16 +++--- packages/author/lib/names.mjs | 69 ++++++++++++++++++------- packages/author/tests/build.test.ts | 44 +++++++++++++--- packages/author/tests/generator.test.ts | 62 ++++++++++++++++++++-- packages/author/tests/libraries.test.ts | 16 ++++++ packages/author/tests/names.test.ts | 44 ++++++++++++---- 10 files changed, 279 insertions(+), 97 deletions(-) diff --git a/design/author.md b/design/author.md index 44eca45..9ad905a 100644 --- a/design/author.md +++ b/design/author.md @@ -410,18 +410,24 @@ to 3,000. Nineteen things the code does not say by itself: interactive-book`, with no name offered: the next guesses were `book` and `flashcards`. So a build reads a source with the pack's names (`sourceOf`), and takes what can only be meant: - the content type under another key, or named loosely as `type`, the same letters and digits in - any case with or without `H5P.` (`names.mjs`); the source's own fields in an object; the + the content type under `contentType` or `library`, or named loosely as `type`, the same letters + and digits in any case with or without `H5P.` (`names.mjs`); the source's own fields in + `metadata`, and in no other object, since Question Set's `introPage` has a title too; the content's fields beside `type`. Each is a problem, said once with what to write, and the content is generated and checked in the same build wherever the type is known, with a stand-in for what is missing. A name that is no type has the nearest offered: those its letters are part of, and those they end with, not one they begin with, since the "image" of "image choice" is no lead. A title is not tried, "Multiple Choice" or "Image Choice": the pack's index has none, and - reading them takes every part, which a page cannot. Sub-content that names its library under - another key, `contentType` or `type`, is taken the same way, its parameters in `params` or - beside the name, where one text names a library the field takes; and a title beside them that - is no field of the library is the sub-content's own, its `metadata.title`, which had been two - problems a chapter. + reading them means fetching every part, 147 of them and some 15 MB, to name one type. A number + with no point is no version: "Question Set 2" is no Question Set. Sub-content that names its + library under `type` or `contentType` is taken the same way, its parameters in `params` or + beside the name, where one of the two names a library the field takes, at the major written + where the field takes it, and the loader loads that library, so what is written for it is + checked against its fields and not against none. No other key: in the 328 sources the tests + stored, every name found under another one was a text that reads like one, a card's answer + "table" six times, `main: "Question"` three times, `title` and `libraryTitle` "Text". A title beside the + parameters that is no field of the library is the sub-content's own, its `metadata.title`, in a + library field's place or in its group's, which had been two problems a chapter. With it 10 of the 12 were right by the second round and all 12 by the third, and every one plays. Of the 263 sources the earlier tests stored, written with the skill's text, 259 have the diff --git a/packages/author/README.md b/packages/author/README.md index 80d3521..f469b96 100644 --- a/packages/author/README.md +++ b/packages/author/README.md @@ -99,8 +99,8 @@ h5p-author: The source has 3 problems: A source's own fields are read the same way, for a chat that wrote one with no skill beside it. What its outer shape plainly means is taken and said once, and the content is checked in the same -round: the content type under another key or named as it is spoken of (`"interactive-book"`, -`"Question Set"`), the title, licence and authors in an object, the content's fields beside `type`. +round: the content type under `contentType` or named as it is spoken of (`"interactive-book"`, +`"Question Set"`), the title, licence and authors in `metadata`, the content's fields beside `type`. ``` h5p-author: The source has 4 problems: diff --git a/packages/author/lib/build.mjs b/packages/author/lib/build.mjs index 73d5051..b598fe6 100644 --- a/packages/author/lib/build.mjs +++ b/packages/author/lib/build.mjs @@ -5,7 +5,7 @@ import { ContentGenerator, libraryString } from './generator.mjs' import { installClosure } from './install.mjs' import { loadLibraries } from './libraries.mjs' import { CC_VERSIONS, LICENSES, ROLES } from './metadata.mjs' -import { machineNameOf, nearestNames } from './names.mjs' +import { machineNameOf, NAMING_KEYS, namingKey, nearestNames } from './names.mjs' import { quizProblems, quizSource } from './quiz.mjs' import { SourceError } from './source-error.mjs' import { escapeText } from './text.mjs' @@ -55,9 +55,9 @@ export const typeNames = (pack) => [...new Set(pack.libraries.filter((entry) => * skill beside it writes a source's outer shape its own way, and each of these cost it a round in * which nothing inside was looked at: * - * - the content type under another name, `contentType` or `library`, or named loosely as `type`, + * - the content type under another key, `contentType` or `library`, or named loosely as `type`, * "interactive-book", which only `names` can tell; - * - the title, language, licence and authors in an object, as H5P keeps them in `metadata`; + * - the title, language, licence and authors in `metadata`, where H5P keeps them; * - the content's fields beside `type`, with no `params` around them. * * Each is a problem, said once with what to write. `typed` is whether the content type is known, @@ -86,34 +86,33 @@ export function sourceOf(input, names) { const at = Object.fromEntries(FIELDS.map((name) => [name, name])) let rest = Object.keys(input).filter((key) => !FIELDS.includes(key)) - // The content type under another name: the one text beside the fields that names one. + // The content type under another key, `contentType` or `library`: the one that names one. let named = '' if (given.type === undefined) { - const keys = rest.filter((key) => typeof input[key] === 'string' && input[key].length <= 64 && - (names ? machineNameOf(input[key], names) !== null : /^\s*H5P\.[^\s.]+(?:\s+\d+\.\d+(?:\.\d+)?)?\s*$/.test(input[key]))) - if (keys.length === 1) { - const [key] = keys - named = names ? /** @type {{ machineName: string }} */ (machineNameOf(input[key], names)).machineName : input[key].trim().split(/\s+/)[0] - report(key, `is not a field of a source: the content type is its type: write "type": "${named}"`) + const others = Object.fromEntries(rest.map((key) => [key, input[key]])) + const keys = names ? [] : rest.filter((key) => NAMING_KEYS.includes(key) && typeof input[key] === 'string' && input[key].length <= 64 && /^\s*H5P\.[^\s.]+(?:\s+\d+\.\d+)?\s*$/.test(input[key])) + const found = names ? namingKey(others, names) : keys.length === 1 ? { key: keys[0], machineName: input[keys[0]].trim().split(/\s+/)[0] } : null + if (found) { + named = found.machineName + report(found.key, `is not a field of a source: the content type is its type: write "type": "${named}"`) given.type = named - rest = rest.filter((each) => each !== key) + rest = rest.filter((each) => each !== found.key) } } - // What a source says of itself, in an object of its own. - for (const key of [...rest]) { - const value = input[key] - if (!isObject(value)) continue - const own = OWN.filter((name) => given[name] === undefined && value[name] !== undefined) - if (!own.length) continue - report(key, `is not a field of a source: ${own.join(', ')} ${own.length === 1 ? 'is' : 'are'} written beside type, not in an object: write ${own.map((name) => `"${name}": …`).join(', ')} at the top`) + // What a source says of itself, in an object of its own: `metadata`, as H5P keeps it. Only + // there, since a field of the content, Question Set's `introPage`, may have a title too. + const value = rest.includes('metadata') && isObject(input.metadata) ? input.metadata : null + const own = value ? OWN.filter((name) => given[name] === undefined && value[name] !== undefined) : [] + if (value && own.length) { + report('metadata', `is not a field of a source: ${own.join(', ')} ${own.length === 1 ? 'is' : 'are'} written beside type, not in an object: write ${own.map((name) => `"${name}": …`).join(', ')} at the top`) for (const name of own) { given[name] = value[name] - at[name] = `${key}.${name}` + at[name] = `metadata.${name}` } for (const inner of Object.keys(value)) { - if (!own.includes(inner)) report(`${key}.${inner}`, OWN.includes(inner) ? 'is written beside type already' : `is not a field of a source; ${fields}`) + if (!own.includes(inner)) report(`metadata.${inner}`, OWN.includes(inner) ? 'is written beside type already' : `is not a field of a source; ${fields}`) } - rest = rest.filter((each) => each !== key) + rest = rest.filter((each) => each !== 'metadata') } // The content's fields beside `type`. if (rest.length && given.params === undefined) { diff --git a/packages/author/lib/generator.mjs b/packages/author/lib/generator.mjs index 79b0ff2..ab9b3ff 100644 --- a/packages/author/lib/generator.mjs +++ b/packages/author/lib/generator.mjs @@ -368,11 +368,16 @@ export class ContentGenerator { if (typeof given.library !== 'string' && holders.length) { const outside = Object.keys(given).filter((key) => !SUB_CONTENT.includes(key) && !fields.some((field) => field.name === key)) const named = namingKey(pick(given, outside), holders.flatMap((field) => optionNames(field))) - const beside = named ? outside.filter((key) => key !== named.key) : [] - const loose = named && !isObject(given.params) && beside.length > 0 - const meant = !named ? null : loose - ? { ...omit(given, [named.key, ...beside]), library: named.machineName, params: pick(given, beside) } - : { ...omit(given, [named.key]), library: named.machineName } + const taken = named ? takenAs(holders, named) : null + // A title beside them that is no field of its library is its own, as one beside `library` + // is: it stays there, for `writtenLibrary` to take for its `metadata.title`. + const fieldsOfIt = taken ? this.find(taken.machineName, taken.major)?.semantics ?? [] : [] + const titled = typeof given.title === 'string' && !fieldsOfIt.some((/** @type {any} */ field) => field.name === 'title') + const beside = taken ? outside.filter((key) => key !== taken.key && !(titled && key === 'title')) : [] + const loose = taken && !isObject(given.params) && beside.length > 0 + const meant = !taken ? null : loose + ? { ...omit(given, [taken.key, ...beside]), library: taken.library, params: pick(given, beside) } + : { ...omit(given, [taken.key]), library: taken.library } if (meant && holderOf(fields, meant)) { given = meant flat = Boolean(loose) @@ -380,7 +385,10 @@ export class ContentGenerator { } const holder = holderOf(fields, given) if (holder) { - const subContent = pick(given, SUB_CONTENT) + // A title beside it that is no field of the group is the sub-content's own, which + // `writtenLibrary` takes for its `metadata.title` and says so: not a field of the group. + const kept = typeof given.title === 'string' && !fields.some((field) => field.name === 'title') ? [...SUB_CONTENT, 'title'] : SUB_CONTENT + const subContent = pick(given, kept) const shown = ['library', 'params', 'metadata'].filter((key) => Object.hasOwn(subContent, key)) .map((key) => key === 'library' ? `"library": ${JSON.stringify(given.library)}` : `"${key}": { … }`) const beside = Object.keys(given).filter((key) => !SUB_CONTENT.includes(key) && fields.some((field) => field.name === key)).map((key) => `, "${key}": …`) @@ -391,7 +399,7 @@ export class ContentGenerator { this.#move(path, `${path}.${holder.name}.params`) } this.#move(path, `${path}.${holder.name}`) - given = { ...omit(given, SUB_CONTENT), [holder.name]: subContent } + given = { ...omit(given, kept), [holder.name]: subContent } } for (const [key, value] of Object.entries(given)) { if (value === null || value === undefined || fields.some((field) => field.name === key)) continue @@ -516,21 +524,21 @@ export class ContentGenerator { if (isObject(written) && typeof written.library === 'string') return this.writtenLibrary(written, stack, path, field, this.#flat.get(written)) if (written !== undefined && !this.lenient) { // Sub-content that names its library under another key than `library`, `contentType` or - // `type`, as a source written with no skill beside it does (#174): the one text in it - // that names a library the field takes. + // `type`, as a source written with no skill beside it does (#174): the one of them that + // names a library the field takes, at the major written where the field takes it. const named = !this.saved && isObject(written) ? namingKey(omit(written, SUB_CONTENT), optionNames(field)) : null - const major = named ? libraryOptions(field).find((option) => option.machineName === named.machineName)?.major : undefined - const entry = this.saved || !isObject(written) ? null : named ? this.find(named.machineName, major) : this.#only(field) - if (named && isObject(written.params)) { - this.report(`${path}.${named.key}`, `is the sub-content's library: write "library": "${named.machineName}"`) + const taken = named ? takenAs([field], named) : null + const entry = this.saved || !isObject(written) ? null : taken ? this.find(taken.machineName, taken.major) : this.#only(field) + if (taken && isObject(written.params)) { + this.report(`${path}.${taken.key}`, `is the sub-content's library: write "library": "${taken.library}"`) // Its parameters are checked where the library's fields are at hand. - return entry?.semantics ? this.writtenLibrary({ ...omit(written, [named.key]), library: named.machineName }, stack, path, field) : undefined + return entry?.semantics ? this.writtenLibrary({ ...omit(written, [taken.key]), library: taken.library }, stack, path, field) : undefined } // The parameters of sub-content without the sub-content around them: of the one library // the field takes, as a book's chapters written `{ "content": [ … ] }` (#172), or of the // one named beside them, `{ "type": "H5P.MultiChoice", "question": … }` (#174). Said so, // and checked as its parameters in the same round, named by where they are written. - let params = named ? omit(written, [named.key]) : written + let params = taken ? omit(written, [taken.key]) : written const own = entry ? Object.keys(params).filter((key) => (entry.semantics ?? []).some((/** @type {any} */ each) => each.name === key)) : [] if (entry && own.length) { let metadata = own.includes('metadata') ? undefined : params.metadata @@ -539,11 +547,12 @@ export class ContentGenerator { const titled = metadata === undefined && typeof params.title === 'string' && !own.includes('title') if (titled) metadata = { title: params.title } if (metadata !== undefined) params = omit(params, titled ? ['title'] : ['metadata']) - this.report(path, `is the params of sub-content, without the sub-content around them: write ${path} as { "library": "${entry.library.machineName}"${metadata === undefined ? '' : titled ? ', "metadata": { "title": … }' : ', "metadata": { … }'}, "params": { "${own[0]}": … } }`) + const library = taken ? taken.library : entry.library.machineName + this.report(path, `is the params of sub-content, without the sub-content around them: write ${path} as { "library": "${library}"${metadata === undefined ? '' : titled ? ', "metadata": { "title": … }' : ', "metadata": { … }'}, "params": { "${own[0]}": … } }`) this.#move(path, `${path}.params`) - return this.writtenLibrary({ library: entry.library.machineName, params, ...(metadata === undefined ? {} : { metadata }) }, stack, path, field, path) + return this.writtenLibrary({ library, params, ...(metadata === undefined ? {} : { metadata }) }, stack, path, field, path) } - this.report(path, `expected sub-content, ${subContentOf(field, named ? named.machineName : written)}`) + this.report(path, `expected sub-content, ${subContentOf(field, taken ? taken.machineName : written)}`) return undefined } // A default names the content the editor starts with (h5peditor.js puts it in the @@ -947,6 +956,21 @@ function optionNames(field) { return libraryOptions(field).map((option) => option.machineName) } +/** + * The library a name under another key than `library` stands for (#174), of those the fields + * take: at the major written, where one of them takes it, else at the first they take; `library`, + * the text to write for it, has the version only where it was written and is taken. + * @param {any[]} fields library fields + * @param {{ key: string, machineName: string, major?: number, library: string }} named + * @returns {{ key: string, machineName: string, major: number | undefined, library: string }} + */ +function takenAs(fields, named) { + const offered = fields.flatMap((field) => libraryOptions(field)).filter((option) => option.machineName === named.machineName) + const option = offered.find((each) => each.major === named.major) ?? offered[0] + const kept = option !== undefined && option.major === named.major + return { key: named.key, machineName: named.machineName, major: option?.major, library: kept ? named.library : named.machineName } +} + /** * Sub-content as a field takes it, for a problem to show: with the library named where the field * takes one alone, or where what was written names one it takes; else with every library it diff --git a/packages/author/lib/libraries.mjs b/packages/author/lib/libraries.mjs index b107c40..1cc2ff2 100644 --- a/packages/author/lib/libraries.mjs +++ b/packages/author/lib/libraries.mjs @@ -1,6 +1,6 @@ import { ownLibraries } from '@missing-elements/h5p-offline-player/plan' import { libraryOptions, libraryString } from './generator.mjs' -import { machineNameOf } from './names.mjs' +import { machineNameOf, NAMING_KEYS } from './names.mjs' import { SourceError } from './source-error.mjs' import { localize, translationFiles } from './translate.mjs' import { patchSemantics } from './types.mjs' @@ -90,17 +90,17 @@ export async function loadLibraries(pack, { type, language, params }) { if (match && match[2] !== undefined) queue.push({ machineName: match[1], major: Number(match[2]) }) else if (match) for (const entry of pack.libraries) if (entry.machineName === match[1]) queue.push({ machineName: match[1], major: entry.majorVersion }) // What the generator takes to name a library though it is not written as one (#174), so - // that what is written for it is checked: `library` written loosely, "multichoice", and a - // machine name under another key, `contentType` or `type`. A name under another key is - // taken only as it is, so that a text that reads like one, a slide titled "Summary", loads - // nothing. + // that what is written for it is checked and not reported against no fields: `library` + // written loosely, "multichoice", and a name under `type` or `contentType`, as it is or + // loosely. Those keys only, as the generator reads them, so that a text that reads like a + // name under another, a card's answer "table", loads nothing. if (typeof value.library === 'string') { const found = machineNameOf(value.library, names) if (found && !found.exact) meant(found.machineName) } else { - for (const text of Object.values(value)) { - const found = typeof text === 'string' && text.length <= 64 ? machineNameOf(text, names) : null - if (found?.exact) meant(found.machineName) + for (const key of NAMING_KEYS) { + const found = machineNameOf(value[key], names) + if (found) meant(found.machineName) } } Object.values(value).forEach(named) diff --git a/packages/author/lib/names.mjs b/packages/author/lib/names.mjs index 3442091..d431165 100644 --- a/packages/author/lib/names.mjs +++ b/packages/author/lib/names.mjs @@ -8,26 +8,55 @@ */ /** - * A name's letters and digits, lower case, without `H5P.` before them or a version after. + * The keys a library is named under in place of `library`, or of a source's `type`: `type` and + * `contentType` for sub-content, as a chat with no skill beside it writes them, and `contentType` + * and `library` for a source's own. No other: in the sources the tests stored, every name found + * under another key was a text that reads like one, a card's answer "table", a title "Text". + */ +export const NAMING_KEYS = ['type', 'contentType', 'library'] + +/** The longest text taken for a name: a sentence that happens to hold one is not it. */ +const LONGEST = 64 + +/** + * A name as written, and the version after it: `major.minor`, or `major.minor.patch` as a + * library's own version is written. Found without a regular expression that can backtrack, since + * what is written is any text. + * @param {string} text + * @returns {{ name: string, version?: number[] }} + */ +function split(text) { + const trimmed = text.trim() + let at = trimmed.length + while (at > 0 && !/\s/.test(trimmed[at - 1])) at-- + const version = /^\d+\.\d+(?:\.\d+)?$/.test(trimmed.slice(at)) && at > 0 ? trimmed.slice(at).split('.').map(Number) : undefined + return version ? { name: trimmed.slice(0, at).trim(), version } : { name: trimmed } +} + +/** + * A name's letters and digits, lower case, without `H5P.` before them or a version after. A + * number with no point after it is no version: "Question Set 2" is not Question Set. * @param {string} text */ -const letters = (text) => text.toLowerCase().replace(/\s+\d+(?:\.\d+)*\s*$/, '').replace(/^\s*h5p(?![a-z0-9])[\s._-]*/, '').replace(/[^a-z0-9]/g, '') +const letters = (text) => split(text).name.toLowerCase().replace(/^h5p(?![a-z0-9])[\s._-]*/, '').replace(/[^a-z0-9]/g, '') /** - * The machine name that was written, of the names given: the name itself, with a version or - * without, or the one name of the same letters. `exact` is false for one written loosely. + * The machine name that was written, of the names given: the name itself, alone or with a major + * and minor after it, as sub-content names its library, or the one name of the same letters. + * `exact` is false for one written loosely, which a version of three parts is too, since what + * reads a library's name takes two. `major` and `minor` are those written with an exact name. * @param {unknown} written * @param {Iterable} names machine names - * @returns {{ machineName: string, exact: boolean } | null} + * @returns {{ machineName: string, exact: boolean, major?: number, minor?: number } | null} */ export function machineNameOf(written, names) { - if (typeof written !== 'string' || !written.trim()) return null + if (typeof written !== 'string' || written.length > LONGEST || !written.trim()) return null const all = [...new Set(names)] - const match = /^\s*(\S+?)(?:\s+\d+\.\d+(?:\.\d+)?)?\s*$/.exec(written) - if (match && all.includes(match[1])) return { machineName: match[1], exact: true } + const { name, version } = split(written) + if (all.includes(name) && (!version || version.length === 2)) return { machineName: name, exact: true, ...(version ? { major: version[0], minor: version[1] } : {}) } const key = letters(written) if (!key) return null - const same = all.filter((name) => letters(name) === key) + const same = all.filter((each) => letters(each) === key) return same.length === 1 ? { machineName: same[0], exact: false } : null } @@ -40,7 +69,7 @@ export function machineNameOf(written, names) { * @returns {string[]} */ export function nearestNames(written, names) { - const key = typeof written === 'string' ? letters(written) : '' + const key = typeof written === 'string' && written.length <= LONGEST ? letters(written) : '' if (key.length < 3) return [] return [...new Set(names)].filter((name) => { const own = letters(name) @@ -49,22 +78,22 @@ export function nearestNames(written, names) { } /** - * The key under which an object names a library where `library` would, as `contentType` or `type` - * does in a source written with no skill beside it: its one text that names a library of those - * given. None where two keys do. + * The key under which an object names a library where `library` would: of `NAMING_KEYS`, the one + * whose text names a library of those given. None where two do. `library` is the text to write, + * with the major and minor that were written. * @param {Record} given * @param {Iterable} names machine names - * @returns {{ key: string, machineName: string } | null} + * @returns {{ key: string, machineName: string, major?: number, library: string } | null} */ export function namingKey(given, names) { const all = [...new Set(names)] - /** @type {{ key: string, machineName: string }[]} */ + /** @type {{ key: string, machineName: string, major?: number, library: string }[]} */ const found = [] - for (const [key, value] of Object.entries(given)) { - // A name is short: a sentence that happens to hold one is not it. - if (typeof value !== 'string' || value.length > 64) continue - const named = machineNameOf(value, all) - if (named) found.push({ key, machineName: named.machineName }) + for (const key of NAMING_KEYS) { + const named = Object.hasOwn(given, key) ? machineNameOf(given[key], all) : null + if (!named) continue + const { machineName, major, minor } = named + found.push({ key, machineName, ...(major === undefined ? {} : { major }), library: major === undefined ? machineName : `${machineName} ${major}.${minor}` }) } return found.length === 1 ? found[0] : null } diff --git a/packages/author/tests/build.test.ts b/packages/author/tests/build.test.ts index 87a54b4..d5167cf 100644 --- a/packages/author/tests/build.test.ts +++ b/packages/author/tests/build.test.ts @@ -424,12 +424,24 @@ describe('any content type in the pack', () => { expect(await problems({ ...META, type: 'Question Set', params: { questions: [{ library: 'H5P.TrueFalse', params: { question: 'Yes?', correct: 'true' } }] } })).toEqual(['type: takes the machine name, "H5P.QuestionSet"']) expect(await problems({ ...META, type: 'book', params: {} })).toEqual(['type: the library pack has no content type book; nearest: H5P.InteractiveBook']) expect(await problems({ ...META, type: 'fill-in-the-blanks', params: {} })).toEqual(['type: the library pack has no content type fill-in-the-blanks; nearest: H5P.Blanks']) - // Two texts beside the fields that name a type: neither is taken for it. - expect(await problems({ ...META, contentType: 'H5P.Blanks', mainLibrary: 'H5P.DragText', params: {} })).toEqual([ + // "Question Set 2" is no Question Set: a number with no point is no version. + expect(await problems({ ...META, type: 'Question Set 2', params: {} })).toEqual(['type: the library pack has no content type Question Set 2']) + // Two keys that each name a type: neither is taken for it. A text under another key is no + // name of the type, however it reads, nor is an object other than `metadata` the source's + // own fields: Question Set's `introPage` has a title too. + expect(await problems({ ...META, contentType: 'H5P.Blanks', library: 'H5P.DragText', params: {} })).toEqual([ expect.stringMatching(/^contentType: is not a field of a source; the fields are/), - expect.stringMatching(/^mainLibrary: is not a field of a source; the fields are/), + expect.stringMatching(/^library: is not a field of a source; the fields are/), 'type: expected a content type\'s machine name, such as "H5P.Blanks"' ]) + expect(await problems({ ...META, description: 'Summary', params: {} })).toEqual([ + expect.stringMatching(/^description: is not a field of a source; the fields are/), + 'type: expected a content type\'s machine name, such as "H5P.Blanks"' + ]) + expect(await problems({ ...META, title: undefined, type: 'H5P.QuestionSet', introPage: { showIntroPage: true, title: 'Welcome' }, questions: [{ library: 'H5P.TrueFalse', params: { question: 'Yes?', correct: 'true' } }] })).toEqual([ + 'params: is not written: the content\'s fields go in it, not beside type: write "params": { "introPage": …, "questions": … }', + 'title: expected the activity\'s title' + ]) // Sub-content with its library under another key and its parameters beside it, in a field // that takes several libraries: the parameters of the one it names, checked as such. @@ -446,14 +458,32 @@ describe('any content type in the pack', () => { 'params.presentation.slides[0].elements[0]: is sub-content, but here it goes inside an object, in its field action: write params.presentation.slides[0].elements[0] as { "action": { "library": "H5P.AdvancedText", "params": { … } }, "x": …, "y": …, "width": …, "height": … }', expect.stringMatching(/^params\.presentation\.slides\[0\]\.elements\[0\]\.text: has

, which this field does not take/) ]) - // A library named loosely, a title beside `library`, and two keys that each name a library. - expect(await problems({ ...META, type: 'H5P.Column', params: { content: [{ content: { library: 'advancedtext', params: { text: 'x' } } }] } })).toEqual([ - 'params.content[0].content.library: takes the machine name, "H5P.AdvancedText"' + // A name under `type` or `contentType` written loosely, in a field that takes several + // libraries: the library it names is loaded, so what is written for it is checked against its + // fields in the same round, and not against none. + expect(await problems({ ...META, type: 'H5P.Column', params: { content: [{ type: 'advancedtext', text: '

Hi

' }, { content: { contentType: 'advanced-text', params: { text: '

Hi

' } } }] } })).toEqual([ + 'params.content[0]: is sub-content, but here it goes inside an object, in its field content: write params.content[0] as { "content": { "library": "H5P.AdvancedText", "params": { … } } }', + expect.stringMatching(/^params\.content\[0\]\.text: has

, which this field does not take/), + 'params.content[1].content.contentType: is the sub-content\'s library: write "library": "H5P.AdvancedText"', + expect.stringMatching(/^params\.content\[1\]\.content\.params\.text: has

, which this field does not take/) + ]) + // A title beside sub-content in its group's place is its own, with `library` or another key. + expect(await problems({ ...META, type: 'H5P.Column', params: { content: [{ library: 'H5P.AdvancedText', title: 'One', params: { text: 'x' } }, { type: 'H5P.AdvancedText', title: 'Two', text: 'y' }] } })).toEqual([ + 'params.content[0]: is sub-content, but here it goes inside an object, in its field content: write params.content[0] as { "content": { "library": "H5P.AdvancedText", "params": { … } } }', + 'params.content[0].title: is the sub-content\'s title, which is in its metadata: write "metadata": { "title": … }', + 'params.content[1]: is sub-content, but here it goes inside an object, in its field content: write params.content[1] as { "content": { "library": "H5P.AdvancedText", "params": { … } } }', + 'params.content[1].title: is the sub-content\'s title, which is in its metadata: write "metadata": { "title": … }' + ]) + // A library named loosely, or with a version of three parts, a title beside `library`, and + // two keys that each name a library. + expect(await problems({ ...META, type: 'H5P.Column', params: { content: [{ content: { library: 'advancedtext', params: { text: 'x' } } }, { content: { library: 'H5P.AdvancedText 1.1.5', params: { text: 'x' } } }] } })).toEqual([ + 'params.content[0].content.library: takes the machine name, "H5P.AdvancedText"', + 'params.content[1].content.library: takes the machine name, "H5P.AdvancedText"' ]) expect(await problems({ ...META, type: 'H5P.InteractiveBook', params: { chapters: [{ ...chapter, metadata: undefined, title: 'Newton' }] } })).toEqual([ 'params.chapters[0].title: is the sub-content\'s title, which is in its metadata: write "metadata": { "title": … }' ]) - expect(await problems({ ...META, type: 'H5P.QuestionSet', params: { questions: [{ type: 'H5P.MultiChoice', kind: 'H5P.TrueFalse', question: 'Which?' }] } })).toEqual([ + expect(await problems({ ...META, type: 'H5P.QuestionSet', params: { questions: [{ type: 'H5P.MultiChoice', contentType: 'H5P.TrueFalse', question: 'Which?' }] } })).toEqual([ expect.stringMatching(/^params\.questions\[0\]: expected sub-content, \{ "library": "…", "params": \{ … \} \}, of H5P\.MultiChoice, /) ]) expect((await readdir(dir)).filter(name => name.startsWith('failed-') || name.startsWith('.h5p-author-'))).toEqual([]) diff --git a/packages/author/tests/generator.test.ts b/packages/author/tests/generator.test.ts index e3a6322..93a5c36 100644 --- a/packages/author/tests/generator.test.ts +++ b/packages/author/tests/generator.test.ts @@ -345,7 +345,7 @@ describe('content authored from semantics', () => { const { params, problems, moved } = generate(set, { questions: [ { contentType: 'H5P.MultiChoice', params: { question: 'Which?', sideways: 1 } }, { type: 'advancedtext', text: '

Two

', title: 'Second' }, - { type: 'H5P.MultiChoice', kind: 'H5P.AdvancedText', question: 'Which?' }, + { type: 'H5P.MultiChoice', contentType: 'H5P.AdvancedText', question: 'Which?' }, { type: 'essay', question: 'Why?' } ] }, [TEXT, CHOICE], titled) expect(show(problems)).toEqual([ @@ -359,9 +359,37 @@ describe('content authored from semantics', () => { 'params.questions[3]: expected sub-content, { "library": "…", "params": { … } }, of H5P.MultiChoice, H5P.AdvancedText' ]) expect(params.questions[0]).toMatchObject({ library: 'H5P.MultiChoice 1.16', params: { question: 'Which?' } }) - expect(params.questions[1]).toMatchObject({ library: 'H5P.AdvancedText 1.1', metadata: { title: 'Second' } }) + expect(params.questions[1]).toMatchObject({ library: 'H5P.AdvancedText 1.1', params: { text: '

Two

' }, metadata: { title: 'Second' } }) expect(moved).toEqual([{ from: 'params.questions[1]', to: 'params.questions[1].params' }]) + // Under another key than type or contentType a text is no name: a field's only library takes + // it as its own, `{ "text": "H5P.AdvancedText" }` its text, and a title "Text" is a title. + const only = [{ name: 'intro', type: 'library', options: ['H5P.AdvancedText 1.1'] }] + const named = generate(only, { intro: { text: 'H5P.AdvancedText' } }, [TEXT, CHOICE], STRICT) + expect(show(named.problems)).toEqual(['params.intro: is the params of sub-content, without the sub-content around them: write params.intro as { "library": "H5P.AdvancedText", "params": { "text": … } }']) + expect(named.params.intro).toMatchObject({ library: 'H5P.AdvancedText 1.1', params: { text: '
H5P.AdvancedText
' } }) + const textTitled = generate(set, { questions: [{ contentType: 'H5P.AdvancedText', title: 'Text', params: { text: 'One' } }] }, [{ ...TEXT, machineName: 'H5P.Text' }, TEXT, CHOICE], titled) + expect(show(textTitled.problems)).toEqual([ + 'params.questions[0].contentType: is the sub-content\'s library: write "library": "H5P.AdvancedText"', + 'params.questions[0].title: is the sub-content\'s title, which is in its metadata: write "metadata": { "title": … }' + ]) + expect(textTitled.params.questions[0]).toMatchObject({ library: 'H5P.AdvancedText 1.1', params: { text: '
One
' }, metadata: { title: 'Text' } }) + + // A version written with the name is kept, where the field takes two majors of the library: + // what is written is checked against the major it names, not the first one the field takes. + const CHILD = { machineName: 'H5P.Child', semantics: [{ name: 'old', type: 'text' }] } + const CHILD2 = { machineName: 'H5P.Child', major: 2, semantics: [{ name: 'new', type: 'text' }] } + const both = [{ name: 'child', type: 'library', options: ['H5P.Child 1.0', 'H5P.Child 2.0'] }] + const second = generate(both, { child: { type: 'H5P.Child 2.0', params: { new: 'x' } } }, [CHILD, CHILD2], STRICT) + expect(show(second.problems)).toEqual(['params.child.type: is the sub-content\'s library: write "library": "H5P.Child 2.0"']) + expect(second.params.child).toMatchObject({ library: 'H5P.Child 2.0', params: { new: 'x' } }) + const flat = generate(both, { child: { contentType: 'H5P.Child 2.0', new: 'x' } }, [CHILD, CHILD2], STRICT) + expect(show(flat.problems)).toEqual(['params.child: is the params of sub-content, without the sub-content around them: write params.child as { "library": "H5P.Child 2.0", "params": { "new": … } }']) + expect(flat.params.child).toMatchObject({ library: 'H5P.Child 2.0', params: { new: 'x' } }) + // A major the field does not take is left off, and the field's first is taken. + const none = generate(both, { child: { type: 'H5P.Child 3.0', params: { old: 'x' } } }, [CHILD, CHILD2], STRICT) + expect(show(none.problems)).toEqual(['params.child.type: is the sub-content\'s library: write "library": "H5P.Child"']) + // In the place of the group that holds it, what is no field of the group is its parameters, // named by where the source has them; with `params` written, it stays what it is. const slide = [{ name: 'elements', type: 'list', field: { name: 'element', type: 'group', fields: [ @@ -375,10 +403,36 @@ describe('content authored from semantics', () => { expect(show(held.problems)).toEqual([ 'params.elements[0]: is sub-content, but here it goes inside an object, in its field action: write params.elements[0] as { "action": { "library": "H5P.AdvancedText", "params": { … } }, "x": … }', expect.stringMatching(/^params\.elements\[0\]\.text: has

, which this field does not take/), - 'params.elements[1]: is sub-content, but here it goes inside an object, in its field action: write params.elements[1] as { "action": { "library": "H5P.MultiChoice", "params": { … } } }', + 'params.elements[1]: is sub-content, but here it goes inside an object, in its field action: write params.elements[1] as { "action": { "library": "H5P.MultiChoice 1.16", "params": { … } } }', 'params.elements[1].colour: is not a field here; the fields are action, x' ]) - expect(held.params.elements[0]).toMatchObject({ x: 5, action: { library: 'H5P.AdvancedText 1.1' } }) + expect(held.params.elements[0]).toMatchObject({ x: 5, action: { library: 'H5P.AdvancedText 1.1', params: { text: '

One

' } } }) + + // A title beside it, with its parameters beside the name, in `params` or with `library`, is + // its own where it is no field of the group or of its library: its `metadata.title`, said + // where it is written, and not a field of the group or a parameter. + const titledHeld = generate(slide, { elements: [ + { type: 'H5P.AdvancedText', title: 'One', text: 'First' }, + { contentType: 'H5P.AdvancedText', title: 'Two', params: { text: 'Second' } }, + { library: 'H5P.AdvancedText', title: 'Three', params: { text: 'Third' } } + ] }, [TEXT, CHOICE], titled) + expect(show(titledHeld.problems)).toEqual([ + 'params.elements[0]: is sub-content, but here it goes inside an object, in its field action: write params.elements[0] as { "action": { "library": "H5P.AdvancedText", "params": { … } } }', + 'params.elements[0].title: is the sub-content\'s title, which is in its metadata: write "metadata": { "title": … }', + 'params.elements[1]: is sub-content, but here it goes inside an object, in its field action: write params.elements[1] as { "action": { "library": "H5P.AdvancedText", "params": { … } } }', + 'params.elements[1].title: is the sub-content\'s title, which is in its metadata: write "metadata": { "title": … }', + 'params.elements[2]: is sub-content, but here it goes inside an object, in its field action: write params.elements[2] as { "action": { "library": "H5P.AdvancedText", "params": { … } } }', + 'params.elements[2].title: is the sub-content\'s title, which is in its metadata: write "metadata": { "title": … }' + ]) + expect(titledHeld.params.elements.map((element: { action: unknown }) => element.action)).toMatchObject([ + { params: { text: '
First
' }, metadata: { title: 'One' } }, { params: { text: '
Second
' }, metadata: { title: 'Two' } }, { params: { text: '
Third
' }, metadata: { title: 'Three' } } + ]) + // A library with a title of its own takes the one beside its parameters as that. + const CARDS = { machineName: 'H5P.Dialogcards', minor: 9, semantics: [{ name: 'title', type: 'text', optional: true }, { name: 'description', type: 'text', optional: true }] } + const cards = generate([{ name: 'elements', type: 'list', field: { name: 'element', type: 'group', fields: [{ name: 'action', type: 'library', options: ['H5P.Dialogcards 1.9'] }, { name: 'x', type: 'number', default: 0 }] } }], + { elements: [{ type: 'H5P.Dialogcards', title: 'Words', description: 'Learn them' }] }, [CARDS], titled) + expect(show(cards.problems)).toEqual(['params.elements[0]: is sub-content, but here it goes inside an object, in its field action: write params.elements[0] as { "action": { "library": "H5P.Dialogcards", "params": { … } } }']) + expect(cards.params.elements[0].action).toMatchObject({ params: { title: 'Words', description: 'Learn them' } }) // The nearer move first, so that what a later check finds in the parameters is named where they are written. expect(held.moved).toEqual([ { from: 'params.elements[0]', to: 'params.elements[0].action.params' }, { from: 'params.elements[0]', to: 'params.elements[0].action' }, diff --git a/packages/author/tests/libraries.test.ts b/packages/author/tests/libraries.test.ts index e573e00..12580f8 100644 --- a/packages/author/tests/libraries.test.ts +++ b/packages/author/tests/libraries.test.ts @@ -83,6 +83,22 @@ describe('loadLibraries', () => { expect(entryOf(objects.entries, 'H5P.Column')?.semantics).not.toBeNull() }) + it('loads a library named under type or contentType, as it is or loosely, and nothing for a text under another key (#174)', async () => { + // Sub-content as a chat with no skill writes it, in a field that takes several libraries: the + // generator reads what is written for it against the library it names, so that is loaded. + const { entries, translations } = await loadLibraries(pack, { type: 'H5P.Column', language: 'de', params: { content: [ + { type: 'advancedtext', text: 'x' }, + { content: { contentType: 'H5P.TrueFalse', params: {} } }, + { content: { library: 'H5P.Dialogcards', params: { dialogs: [{ text: 'Tisch', answer: 'table' }] } } } + ] } }) + expect(entryOf(entries, 'H5P.AdvancedText')?.semantics).not.toBeNull() + expect(entryOf(entries, 'H5P.TrueFalse')?.semantics).not.toBeNull() + // A card's answer that reads like a library's name loads nothing, and what is loaded only + // because it may be meant is no translation the content reports. + expect(entryOf(entries, 'H5P.Table')).toMatchObject({ semantics: null }) + expect(translations.map((report) => report.library.split(' ')[0])).toEqual(['H5P.Column', 'H5P.Dialogcards']) + }) + it('marks the fields a layout sets optional, and leaves the pack\'s own semantics as they are', async () => { const { entries } = await loadLibraries(pack, content) expect(fieldAt(entryOf(entries, 'H5P.Video')!.semantics as Field[], 'visuals.poster')?.optional).toBe(true) diff --git a/packages/author/tests/names.test.ts b/packages/author/tests/names.test.ts index 633b70b..585d5f9 100644 --- a/packages/author/tests/names.test.ts +++ b/packages/author/tests/names.test.ts @@ -11,9 +11,13 @@ describe('a library named another way than by its machine name', () => { ['blanks', 'H5P.Blanks'], ['h5p blanks', 'H5P.Blanks'], ['H5P_Blanks', 'H5P.Blanks'], ['dialog-cards', 'H5P.Dialogcards'], ['multichoice', 'H5P.MultiChoice'], ['Question Set', 'H5P.QuestionSet'], ['Course Presentation', 'H5P.CoursePresentation'] ]) expect(machineNameOf(written, NAMES), written).toEqual({ machineName, exact: false }) - // The name itself, with a version or without, is exact; a version on a loose name is left off too. + // The name itself, alone or with the major and minor that sub-content names a library by, is + // exact, and says the version; a version of three parts is read loosely, as a version on a + // loose name, and left off, since what reads a library's name takes two. expect(machineNameOf('H5P.Blanks', NAMES)).toEqual({ machineName: 'H5P.Blanks', exact: true }) - expect(machineNameOf(' H5P.Blanks 1.14 ', NAMES)).toEqual({ machineName: 'H5P.Blanks', exact: true }) + expect(machineNameOf(' H5P.Blanks 1.14 ', NAMES)).toEqual({ machineName: 'H5P.Blanks', exact: true, major: 1, minor: 14 }) + expect(machineNameOf('H5P.Blanks\t1.14', NAMES)).toEqual({ machineName: 'H5P.Blanks', exact: true, major: 1, minor: 14 }) + expect(machineNameOf('H5P.Blanks 1.14.2', NAMES)).toEqual({ machineName: 'H5P.Blanks', exact: false }) expect(machineNameOf('blanks 1.14', NAMES)).toEqual({ machineName: 'H5P.Blanks', exact: false }) // An editor library keeps its H5PEditor: "blanks" is not it. expect(machineNameOf('h5peditorblanks', NAMES)).toEqual({ machineName: 'H5PEditor.Blanks', exact: false }) @@ -21,8 +25,19 @@ describe('a library named another way than by its machine name', () => { it('is none where the letters are no library\'s, or two libraries\'', () => { // A title that is not the machine name's letters: the pack's index has no titles. - for (const written of ['book', 'fill-in-the-blanks', 'Multiple Choice', 'H5P.Nope', '', ' ', 'H5P.', 7, null, undefined]) expect(machineNameOf(written, NAMES), String(written)).toBeNull() + for (const written of ['book', 'fill-in-the-blanks', 'Multiple Choice', 'H5P.Nope', '', ' ', 'H5P.', '1.14', 7, null, undefined]) expect(machineNameOf(written, NAMES), String(written)).toBeNull() expect(machineNameOf('ab', ['H5P.AB', 'H5P.Ab'])).toBeNull() + // A number with no point is no version: "Question Set 2" is not Question Set. + expect(machineNameOf('Question Set 2', NAMES)).toBeNull() + expect(machineNameOf('H5P.Blanks 2', NAMES)).toBeNull() + // A text longer than a name is none, however it ends, and is read in no time. + expect(machineNameOf(`H5P.Blanks${' '.repeat(60)}`, NAMES)).toBeNull() + const started = performance.now() + for (const written of [`a${'\t'.repeat(100_000)}x`, `${'\t'.repeat(100_000)}1.1x`, `a ${'1.'.repeat(50_000)}`]) { + expect(machineNameOf(written, NAMES)).toBeNull() + expect(nearestNames(written, NAMES)).toEqual([]) + } + expect(performance.now() - started).toBeLessThan(500) }) it('offers the nearest names: those the letters are part of, and those the letters end with', () => { @@ -38,12 +53,21 @@ describe('a library named another way than by its machine name', () => { expect(nearestNames('card', Array.from({ length: 8 }, (_, i) => `H5P.Card${i}`))).toHaveLength(5) }) - it('finds the key under which an object names a library, where one key does', () => { - expect(namingKey({ contentType: 'H5P.AdvancedText', text: 'Hello' }, NAMES)).toEqual({ key: 'contentType', machineName: 'H5P.AdvancedText' }) - expect(namingKey({ type: 'multichoice', question: 'Which?' }, NAMES)).toEqual({ key: 'type', machineName: 'H5P.MultiChoice' }) - // Two keys that each name one, a text that only holds a name, and nothing that names one. - expect(namingKey({ type: 'text', text: 'Blanks' }, NAMES)).toBeNull() - expect(namingKey({ text: 'Fill in the blanks of this long sentence about H5P.Blanks, which is no name of a library but a sentence' }, NAMES)).toBeNull() - expect(namingKey({ question: 'Which?', answers: ['H5P.Blanks'] }, NAMES)).toBeNull() + it('finds the key under which an object names a library, type or contentType, where one of them does', () => { + expect(namingKey({ contentType: 'H5P.AdvancedText', text: 'Hello' }, NAMES)).toEqual({ key: 'contentType', machineName: 'H5P.AdvancedText', library: 'H5P.AdvancedText' }) + expect(namingKey({ type: 'multichoice', question: 'Which?' }, NAMES)).toEqual({ key: 'type', machineName: 'H5P.MultiChoice', library: 'H5P.MultiChoice' }) + // The version written is kept, for a field that takes two majors of a library. + expect(namingKey({ type: 'H5P.MultiChoice 1.16' }, NAMES)).toEqual({ key: 'type', machineName: 'H5P.MultiChoice', major: 1, library: 'H5P.MultiChoice 1.16' }) + // A source's own type under another key than `type`. + expect(namingKey({ library: 'H5P.Blanks', mainLibrary: 'H5P.Text' }, NAMES)).toEqual({ key: 'library', machineName: 'H5P.Blanks', library: 'H5P.Blanks' }) + // Under another key a text is no name, however it reads: a title "Text", a card's answer, a + // text that is a library's name, a sentence that holds one. + expect(namingKey({ title: 'Text', contentType: 'H5P.AdvancedText' }, NAMES)).toEqual({ key: 'contentType', machineName: 'H5P.AdvancedText', library: 'H5P.AdvancedText' }) + for (const given of [{ title: 'Text' }, { answer: 'blanks' }, { text: 'H5P.AdvancedText' }, { kind: 'H5P.MultiChoice' }, { question: 'Which?', answers: ['H5P.Blanks'] }]) { + expect(namingKey(given, NAMES), JSON.stringify(given)).toBeNull() + } + expect(namingKey({ type: 'Fill in the blanks of this long sentence about H5P.Blanks, which is no name of a library' }, NAMES)).toBeNull() + // Two keys that each name one: neither is taken. + expect(namingKey({ type: 'text', contentType: 'Blanks' }, NAMES)).toBeNull() }) }) From c1ed43ecf30cebc950c1654272a07def7f51546f Mon Sep 17 00:00:00 2001 From: Aleksandr Shevchuk Date: Tue, 10 Nov 2026 17:31:06 +0200 Subject: [PATCH 4/4] docs(author): the README names library and language among what a source's outer shape may hold (#174) As the build reads it: the content type under `contentType` or `library`, and the title, language, licence and authors in `metadata`. --- packages/author/README.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/author/README.md b/packages/author/README.md index f469b96..da1e6c0 100644 --- a/packages/author/README.md +++ b/packages/author/README.md @@ -99,8 +99,9 @@ h5p-author: The source has 3 problems: A source's own fields are read the same way, for a chat that wrote one with no skill beside it. What its outer shape plainly means is taken and said once, and the content is checked in the same -round: the content type under `contentType` or named as it is spoken of (`"interactive-book"`, -`"Question Set"`), the title, licence and authors in `metadata`, the content's fields beside `type`. +round: the content type under `contentType` or `library`, or named as it is spoken of +(`"interactive-book"`, `"Question Set"`), the title, language, licence and authors in `metadata`, +the content's fields beside `type`. ``` h5p-author: The source has 4 problems: