From 94dd6a42dc784bff94b221bf6156aba18ed6e9a0 Mon Sep 17 00:00:00 2001 From: Franklin Koch Date: Thu, 28 Mar 2024 16:05:34 -0600 Subject: [PATCH] =?UTF-8?q?=F0=9F=AA=AA=20Tweaks=20to=20author=20referenci?= =?UTF-8?q?ng?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .changeset/light-teachers-give.md | 5 +++ .changeset/pretty-berries-ring.md | 5 +++ .../src/affiliations/validators.ts | 14 ++++---- .../src/contributors/authors.yml | 26 ++++++++++++++ .../src/contributors/contributors.yml | 34 +++++++++++++++++++ .../src/contributors/validators.ts | 22 ++++++++---- .../myst-frontmatter/src/site/validators.ts | 2 +- .../src/utils/referenceStash.spec.ts | 24 ++++++++++++- .../src/utils/referenceStash.ts | 28 ++++++++++----- 9 files changed, 137 insertions(+), 23 deletions(-) create mode 100644 .changeset/light-teachers-give.md create mode 100644 .changeset/pretty-berries-ring.md diff --git a/.changeset/light-teachers-give.md b/.changeset/light-teachers-give.md new file mode 100644 index 000000000..5bb9984c6 --- /dev/null +++ b/.changeset/light-teachers-give.md @@ -0,0 +1,5 @@ +--- +'myst-frontmatter': patch +--- + +Allow referencing contributors with ref: diff --git a/.changeset/pretty-berries-ring.md b/.changeset/pretty-berries-ring.md new file mode 100644 index 000000000..2c3ae08ce --- /dev/null +++ b/.changeset/pretty-berries-ring.md @@ -0,0 +1,5 @@ +--- +'myst-frontmatter': patch +--- + +Make no-give-name warning less agressive diff --git a/packages/myst-frontmatter/src/affiliations/validators.ts b/packages/myst-frontmatter/src/affiliations/validators.ts index ff2fdc9c3..a34a3fbd7 100644 --- a/packages/myst-frontmatter/src/affiliations/validators.ts +++ b/packages/myst-frontmatter/src/affiliations/validators.ts @@ -57,12 +57,19 @@ export function validateAffiliation(input: any, opts: ValidationOptions) { opts, ); if (value === undefined) return undefined; + // If affiliation only has an id, give it a matching name; this is equivalent to the case + // where a simple string is provided as an affiliation. + if (Object.keys(value).length === 1 && value.id) { + value.name = value.id; + } const output: Affiliation = {}; if (defined(value.id)) { output.id = validateString(value.id, incrementOptions('id', opts)); } if (defined(value.name)) { output.name = validateString(value.name, incrementOptions('name', opts)); + } else { + validationWarning('affiliation should include name/institution', opts); } if (defined(value.department)) { output.department = validateString(value.department, incrementOptions('department', opts)); @@ -120,12 +127,5 @@ export function validateAffiliation(input: any, opts: ValidationOptions) { if (defined(value.fax)) { output.fax = validateString(value.fax, incrementOptions('fax', opts)); } - // If affiliation only has an id, give it a matching name; this is equivalent to the case - // where a simple string is provided as an affiliation. - if (Object.keys(output).length === 1 && output.id) { - return stashPlaceholder(output.id); - } else if (!output.name) { - validationWarning('affiliation should include name/institution', opts); - } return output; } diff --git a/packages/myst-frontmatter/src/contributors/authors.yml b/packages/myst-frontmatter/src/contributors/authors.yml index e0c065628..0e13976e3 100644 --- a/packages/myst-frontmatter/src/contributors/authors.yml +++ b/packages/myst-frontmatter/src/contributors/authors.yml @@ -342,3 +342,29 @@ cases: given: Just A. family: Name warnings: 1 + - title: author as id resolves as placeholder + raw: + author: + id: Just A. Name + normalized: + authors: + - id: Just A. Name + name: Just A. Name + nameParsed: + literal: Just A. Name + given: Just A. + family: Name + - title: reviewer as ref resolves as placeholder + raw: + reviewer: + ref: Just A. Name + normalized: + reviewers: + - Just A. Name + contributors: + - id: Just A. Name + name: Just A. Name + nameParsed: + literal: Just A. Name + given: Just A. + family: Name diff --git a/packages/myst-frontmatter/src/contributors/contributors.yml b/packages/myst-frontmatter/src/contributors/contributors.yml index 40298a924..1c93f5369 100644 --- a/packages/myst-frontmatter/src/contributors/contributors.yml +++ b/packages/myst-frontmatter/src/contributors/contributors.yml @@ -262,3 +262,37 @@ cases: literal: John Doe family: Doe given: John + - title: author as id reference resolves to object + raw: + reviewers: + - id: janedoe + name: Jane Doe + nameParsed: + literal: Jane Doe + family: Doe + given: Jane + - id: johndoe + name: John Doe + nameParsed: + literal: John Doe + family: Doe + given: John + author: janedoe + normalized: + reviewers: + - janedoe + - johndoe + authors: + - id: janedoe + name: Jane Doe + nameParsed: + literal: Jane Doe + family: Doe + given: Jane + contributors: + - id: johndoe + name: John Doe + nameParsed: + literal: John Doe + family: Doe + given: John diff --git a/packages/myst-frontmatter/src/contributors/validators.ts b/packages/myst-frontmatter/src/contributors/validators.ts index 65f4083ae..f4d993581 100644 --- a/packages/myst-frontmatter/src/contributors/validators.ts +++ b/packages/myst-frontmatter/src/contributors/validators.ts @@ -12,12 +12,16 @@ import { validationError, validationWarning, } from 'simple-validators'; +import { orcid } from 'orcid'; import { validateAffiliation } from '../affiliations/validators.js'; import type { ReferenceStash } from '../utils/referenceStash.js'; -import { validateAndStashObject } from '../utils/referenceStash.js'; +import { + isStashPlaceholder, + stashPlaceholder, + validateAndStashObject, +} from '../utils/referenceStash.js'; import type { Contributor, Name } from './types.js'; import { formatName, parseName } from '../utils/parseName.js'; -import { orcid } from 'orcid'; const CONTRIBUTOR_KEYS = [ 'id', @@ -40,6 +44,7 @@ const CONTRIBUTOR_KEYS = [ 'fax', ]; const CONTRIBUTOR_ALIASES = { + ref: 'id', // Used in QMD to reference a contributor role: 'roles', 'equal-contributor': 'equal_contributor', affiliation: 'affiliations', @@ -118,9 +123,6 @@ export function validateName(input: any, opts: ValidationOptions) { if (!output.family) { validationWarning(`No family name for name '${output.literal}'`, opts); } - if (!output.given) { - validationWarning(`No given name for name '${output.literal}'`, opts); - } return output; } @@ -129,7 +131,7 @@ export function validateName(input: any, opts: ValidationOptions) { */ export function validateContributor(input: any, stash: ReferenceStash, opts: ValidationOptions) { if (typeof input === 'string') { - input = { id: input, name: input }; + input = stashPlaceholder(input); } const value = validateObjectKeys( input, @@ -137,6 +139,11 @@ export function validateContributor(input: any, stash: ReferenceStash, opts: Val opts, ); if (value === undefined) return undefined; + // If contributor only has an id, give it a matching name; this is equivalent to the case + // where a simple string is provided as a contributor. + if (Object.keys(value).length === 1 && value.id) { + value.name = value.id; + } const output: Contributor = {}; if (defined(value.id)) { output.id = validateString(value.id, incrementOptions('id', opts)); @@ -256,5 +263,8 @@ export function validateContributor(input: any, stash: ReferenceStash, opts: Val if (defined(value.note)) { output.note = validateString(value.note, incrementOptions('note', opts)); } + if (!isStashPlaceholder(output) && output.nameParsed && !output.nameParsed?.given) { + validationWarning(`No given name for name '${output.nameParsed.literal}'`, opts); + } return output; } diff --git a/packages/myst-frontmatter/src/site/validators.ts b/packages/myst-frontmatter/src/site/validators.ts index bd2aed642..27d06a873 100644 --- a/packages/myst-frontmatter/src/site/validators.ts +++ b/packages/myst-frontmatter/src/site/validators.ts @@ -211,7 +211,7 @@ export function validateSiteFrontmatterKeys(value: Record, opts: Va } } - // Contributor resolution should happen last + // Author/Contributor/Affiliation resolution should happen last const stashContribAuthors = stash.contributors?.filter((contrib) => stash.authorIds?.includes(contrib.id), ); diff --git a/packages/myst-frontmatter/src/utils/referenceStash.spec.ts b/packages/myst-frontmatter/src/utils/referenceStash.spec.ts index 74bb703e6..fa46e094e 100644 --- a/packages/myst-frontmatter/src/utils/referenceStash.spec.ts +++ b/packages/myst-frontmatter/src/utils/referenceStash.spec.ts @@ -1,7 +1,7 @@ import { describe, expect, it, beforeEach } from 'vitest'; import type { ValidationOptions } from 'simple-validators'; import { validateContributor } from '../contributors/validators'; -import { validateAndStashObject } from './referenceStash'; +import { isStashPlaceholder, validateAndStashObject } from './referenceStash'; let opts: ValidationOptions; @@ -207,3 +207,25 @@ describe('validateAndStashObject', () => { expect(opts.messages.warnings?.length).toEqual(1); }); }); + +describe('isStashPlaceholder', () => { + it.each([ + [{}, false], + [{ name: 'name' }, false], + [{ id: 'name' }, false], + [{ name: 'name', id: 'name' }, true], + [{ name: 'name', id: 'id' }, false], + [{ name: 'name', id: 'name', extra: 'name' }, false], + [{ name: 'name', nameParsed: { literal: 'name', family: 'name' } }, false], + [{ id: 'name', nameParsed: { literal: 'name', family: 'name' } }, false], + [{ name: 'name', id: 'name', nameParsed: { literal: 'name' } }, true], + [{ name: 'name', id: 'name', nameParsed: { literal: 'name', family: 'name' } }, true], + [{ name: 'name', id: 'name', nameParsed: { family: 'name' } }, false], + [ + { name: 'name', id: 'name', nameParsed: { literal: 'name', family: 'name' }, extra: 'name' }, + false, + ], + ])(`%s - %s`, async (input, result) => { + expect(isStashPlaceholder(input as any)).toBe(result); + }); +}); diff --git a/packages/myst-frontmatter/src/utils/referenceStash.ts b/packages/myst-frontmatter/src/utils/referenceStash.ts index a261c7ded..3c8f489b9 100644 --- a/packages/myst-frontmatter/src/utils/referenceStash.ts +++ b/packages/myst-frontmatter/src/utils/referenceStash.ts @@ -4,14 +4,16 @@ import type { Affiliation } from '../affiliations/types.js'; import type { Contributor } from '../contributors/types.js'; import { normalizeJsonToString } from './normalizeString.js'; +type WithId = T & { id: string }; + /** * Object to hold items referenced in multiple parts of frontmatter * * These will be normalized to the top level and replaced with ids elsewhere */ export type ReferenceStash = { - affiliations?: (Affiliation & { id: string })[]; - contributors?: (Contributor & { id: string })[]; + affiliations?: WithId[]; + contributors?: WithId[]; // Used to on resolution differentiate authors from other contributors authorIds?: string[]; }; @@ -38,9 +40,19 @@ export function stashPlaceholder(value: string) { * Return true if object: * - has 2 keys and only 2 keys: id and name * - the values for id and name are the same + * + * Also allows nameParsed on object if it matches the parsed object from id/name, + * as this must match the validated version of `stashPlaceholder` output */ -export function isStashPlaceholder(object: { id?: string; name?: string }) { - return Object.keys(object).length === 2 && object.name && object.id && object.name === object.id; +export function isStashPlaceholder(object: { + id?: string; + name?: string; + nameParsed?: { literal?: string }; +}) { + if (!object.name || !object.id || object.name !== object.id) return false; + const nKeys = Object.keys(object).length; + if (nKeys === 2) return true; + return nKeys === 3 && object.nameParsed?.literal === object.id; } /** @@ -64,11 +76,11 @@ export function validateAndStashObject validateFn: (v: any, o: ValidationOptions) => T | undefined, opts: ValidationOptions, ) { - const lookup: Record = {}; + const lookup: Record> = {}; const lookupNorm2Id: Record = {}; stash[kind]?.forEach((item) => { if (item.id) { - lookup[item.id] = item as T & { id: string }; + lookup[item.id] = item as WithId; lookupNorm2Id[normalizeJsonToString({ ...item, id: undefined })] = item.id; } }); @@ -93,10 +105,10 @@ export function validateAndStashObject } if (!Object.keys(lookup).includes(value.id)) { // Handle case of new id - add stash value - lookup[value.id] = value as T & { id: string }; + lookup[value.id] = value as WithId; } else if (isStashPlaceholder(lookup[value.id])) { // Handle case of existing placeholder { id: value, name: value } - replace stash value - lookup[value.id] = value as T & { id: string }; + lookup[value.id] = value as WithId; } else if (warnOnDuplicate) { // Warn on duplicate id - lose new object validationWarning(`duplicate id for ${kind} found in frontmatter: ${value.id}`, opts);