diff --git a/build/tasks/verify/verify-json.mts b/build/tasks/verify/verify-json.mts index 0c0d8ad9..f96fd03d 100644 --- a/build/tasks/verify/verify-json.mts +++ b/build/tasks/verify/verify-json.mts @@ -5,7 +5,13 @@ * @module {type ES6Module} build/tasks/verify/verify-json */ -import { exec, glob, matched, quote } from '@openinf/.github/build/utils'; +import { + exec, + glob, + matched, + quote, + reportFormattingFixes, +} from '@openinf/.github/build/utils'; const EXCLUDED = ['!lib/', '!node_modules/']; @@ -31,5 +37,9 @@ const scripts = [ for (const element of scripts) { exitCode = await exec(element); - if (exitCode !== 0) process.exitCode = exitCode; + if (exitCode !== 0) { + process.exitCode = exitCode; + + if (element.startsWith('prettier')) reportFormattingFixes(json5Files); + } } diff --git a/build/tasks/verify/verify-md.mts b/build/tasks/verify/verify-md.mts index 522c5a5c..306fd104 100644 --- a/build/tasks/verify/verify-md.mts +++ b/build/tasks/verify/verify-md.mts @@ -5,7 +5,13 @@ * @module {type ES6Module} build/tasks/verify/verify-md */ -import { exec, glob, matched, quote } from '@openinf/.github/build/utils'; +import { + exec, + glob, + matched, + quote, + reportFormattingFixes, +} from '@openinf/.github/build/utils'; const markdownFiles = await glob([ '**/*.md', @@ -28,5 +34,9 @@ const scripts = matched(markdownFiles, '**/*.md') for (const element of scripts) { exitCode = await exec(element); - if (exitCode !== 0) process.exitCode = exitCode; + if (exitCode !== 0) { + process.exitCode = exitCode; + + if (element.startsWith('prettier')) reportFormattingFixes(markdownFiles); + } } diff --git a/build/tasks/verify/verify-yaml.mts b/build/tasks/verify/verify-yaml.mts index 6d1e4199..b68296e0 100644 --- a/build/tasks/verify/verify-yaml.mts +++ b/build/tasks/verify/verify-yaml.mts @@ -5,7 +5,13 @@ * @module {type ES6Module} build/tasks/verify/verify-yaml */ -import { exec, glob, matched, quote } from '@openinf/.github/build/utils'; +import { + exec, + glob, + matched, + quote, + reportFormattingFixes, +} from '@openinf/.github/build/utils'; const yamlFiles = await glob([ '**/*.yml', @@ -24,5 +30,9 @@ const scripts = matched(yamlFiles, '**/*.yml, **/*.yaml') for (const element of scripts) { exitCode = await exec(element); - if (exitCode !== 0) process.exitCode = exitCode; + if (exitCode !== 0) { + process.exitCode = exitCode; + + if (element.startsWith('prettier')) reportFormattingFixes(yamlFiles); + } } diff --git a/build/utils.mts b/build/utils.mts index 5ea86abc..03041540 100644 --- a/build/utils.mts +++ b/build/utils.mts @@ -9,6 +9,7 @@ // Requirements // ----------------------------------------------------------------------------- +import { spawnSync } from 'node:child_process'; import { glob as nodeGlob } from 'node:fs/promises'; import { join as pathJoin, relative as pathRelative } from 'node:path'; import { catchWrap } from '@isaacs/catcher'; @@ -172,3 +173,66 @@ export async function glob(patterns: string | string[]) { pathRelative(process.cwd(), pathJoin(entry.parentPath, entry.name)) ); } + +/** + * Says what prettier would change, as a diff, for the files it would change. + * + * `prettier --check` names a file and nothing else. Whoever reads that in a + * CI log is left to reproduce the run to learn whether it objected to a line + * that ran long, a list marker or a table, and a contributor who cannot run + * the tools locally has no way to learn it at all. The diff answers that on + * the spot, the same way a reviewer would: here is the line, here is what it + * should be. + * + * Tools are run without a shell, so a path reaches them as one argument + * whatever it contains; only the leading `./` that `quote` adds, for a name + * that would read as an option, is still needed. Their output is not capped: + * `spawnSync` kills a child that prints more than 1 MiB by default, and + * a large file would then lose its diff for no reason the reader could see. + * @param {string[]} files The files the check was handed. + * @returns {string} A unified diff per file prettier would change, or nothing. + */ +export function formattingFixes(files: string[]) { + const paths = files.map((path) => + path.startsWith('-') ? `./${path}` : path + ); + const options = { + encoding: 'utf8', + maxBuffer: Number.POSITIVE_INFINITY, + } as const; + const listed = spawnSync('prettier', ['--list-different', ...paths], options); + const changed = (listed.stdout ?? '').split('\n').filter(Boolean); + let fixes = ''; + + for (const path of changed) { + const formatted = spawnSync('prettier', [path], options); + + // A file prettier cannot parse has no formatted version to compare, and + // the check already printed why. + if (formatted.status !== 0) continue; + + const diff = spawnSync( + 'diff', + ['-u', '--label', path, '--label', `${path} (formatted)`, path, '-'], + { ...options, input: formatted.stdout } + ); + + fixes += diff.stdout ?? ''; + } + + return fixes; +} + +/** + * Prints what prettier would change, after a check of those files failed. + * @param {string[]} files The files the check was handed. + */ +export function reportFormattingFixes(files: string[]) { + const fixes = formattingFixes(files); + + if (fixes === '') return; + + console.error( + `\nWhat prettier would change (\`nps format.all\` applies it):\n\n${fixes}` + ); +} diff --git a/build/utils.test.mts b/build/utils.test.mts index 7752cee2..02e68296 100644 --- a/build/utils.test.mts +++ b/build/utils.test.mts @@ -10,7 +10,12 @@ import { mkdir, mkdtemp, readFile, writeFile } from 'node:fs/promises'; import { tmpdir } from 'node:os'; import { dirname, join as pathJoin } from 'node:path'; import { after, before, describe, test } from 'node:test'; -import { exec, glob, quote } from '@openinf/.github/build/utils'; +import { + exec, + formattingFixes, + glob, + quote, +} from '@openinf/.github/build/utils'; // Every pattern a build task writes is relative to the directory the task // runs in, so the fixture has to become that directory. @@ -178,3 +183,29 @@ describe('quote', () => { } }); }); + +describe('formattingFixes', () => { + before(async () => { + const root = await mkdtemp(pathJoin(tmpdir(), 'openinf-format-')); + + await writeFile(pathJoin(root, 'untidy.md'), '# Title\n\n* item\n'); + await writeFile(pathJoin(root, 'tidy.md'), '# Title\n\n- item\n'); + process.chdir(root); + }); + + after(() => { + process.chdir(cwd); + }); + + test('shows the lines prettier would change, and what to', () => { + const fixes = formattingFixes(['untidy.md', 'tidy.md']); + + ok(fixes.includes('--- untidy.md\n+++ untidy.md (formatted)\n')); + ok(fixes.includes('\n-# Title\n+# Title\n')); + ok(fixes.includes('\n-* item\n+- item\n')); + }); + + test('says nothing about a file already formatted', () => { + deepStrictEqual(formattingFixes(['tidy.md']), ''); + }); +});