Skip to content

Commit

Permalink
🚨 Surface warnings during tocfile extension resolution (#1254)
Browse files Browse the repository at this point in the history
  • Loading branch information
fwkoch authored Jun 5, 2024
1 parent 77d4f58 commit a665cb1
Show file tree
Hide file tree
Showing 5 changed files with 62 additions and 27 deletions.
5 changes: 5 additions & 0 deletions .changeset/large-ravens-shop.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'myst-cli': patch
---

Surface warnings during tocfile extension resolution
2 changes: 1 addition & 1 deletion packages/myst-cli/src/build/utils/collectExportOptions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ function resolveArticlesFromTOC(
} else if (exp.top_level === 'chapters') {
level = 0;
}
const proj = projectFromTOC(session, projectPath, toc, level);
const proj = projectFromTOC(session, projectPath, toc, level, vfile.path);
return resolveArticlesFromProject(exp, proj, vfile);
}

Expand Down
37 changes: 18 additions & 19 deletions packages/myst-cli/src/project/fromTOC.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,21 +35,16 @@ function pagesFromEntries(
if (isFile(entry)) {
// Level must be "chapter" or "section" for files
entryLevel = level < 0 ? 0 : level;
const file = resolveExtension(join(path, entry.file));
const file = resolveExtension(join(path, entry.file), (message, errorLevel, note) => {
addWarningForFile(session, configFile, message, errorLevel, {
ruleId: RuleId.tocContentsExist,
note,
});
});

if (file && fs.existsSync(file) && !isDirectory(file)) {
const { slug } = fileInfo(file, pageSlugs);
pages.push({ file, level: entryLevel, slug });
} else {
addWarningForFile(
session,
configFile,
`Referenced file not found: ${entry.file}`,
'error',
{
ruleId: RuleId.tocContentsExist,
},
);
}
} else if (isPattern(entry)) {
entryLevel = level < 0 ? 0 : level;
Expand Down Expand Up @@ -120,6 +115,7 @@ export function projectFromTOC(
path: string,
toc: TOC,
level: PageLevels = 1,
file?: string,
): Omit<LocalProject, 'bibliography'> {
const pageSlugs: PageSlugs = {};
const [root, ...entries] = toc;
Expand All @@ -129,15 +125,18 @@ export function projectFromTOC(
if (!isFile(root)) {
throw new Error(`First TOC item must be a file`);
}
const indexFile = resolveExtension(join(path, root.file));
const warnFile =
file ??
selectors.selectLocalConfigFile(session.store.getState(), path) ??
join(path, session.configFiles[0]);
const indexFile = resolveExtension(join(path, root.file), (message, errorLevel, note) => {
addWarningForFile(session, warnFile, message, errorLevel, {
ruleId: RuleId.tocContentsExist,
note,
});
});
if (!indexFile) {
throw Error(
`The table of contents could not find file "${
root.file
}" defined as the first (root) page. Please ensure that one of these files is defined:\n- ${VALID_FILE_EXTENSIONS.map(
(ext) => join(path, `${root.file}${ext}`),
).join('\n- ')}\n`,
);
throw Error(`Could not resolve project index file: ${root.file}`);
}
const { slug } = fileInfo(indexFile, pageSlugs);
const pages: (LocalProjectFolder | LocalProjectPage)[] = [];
Expand Down
2 changes: 1 addition & 1 deletion packages/myst-cli/src/project/load.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ export async function loadProjectFromDisk(
let legacyToc = false;
const sphinxTOCFile = validateSphinxTOC(session, path) ? tocFile(path) : undefined;
if (projectConfig?.toc !== undefined) {
newProject = projectFromTOC(session, path, projectConfig.toc);
newProject = projectFromTOC(session, path, projectConfig.toc, 1, projectConfigFile);
if (sphinxTOCFile) {
addWarningForFile(
session,
Expand Down
43 changes: 37 additions & 6 deletions packages/myst-cli/src/utils/resolveExtension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,45 @@ export function isValidFile(file: string): boolean {
return VALID_FILE_EXTENSIONS.includes(path.extname(file).toLowerCase());
}

// export function isDirectory(file: string): boolean {
// return fs.lstatSync(file).isDirectory();
// }

export function resolveExtension(file: string): string | undefined {
export function resolveExtension(
file: string,
warnFn?: (message: string, level: 'warn' | 'error', note?: string) => void,
): string | undefined {
if (fs.existsSync(file) && !isDirectory(file)) return file;
const extensions = VALID_FILE_EXTENSIONS.concat(
VALID_FILE_EXTENSIONS.map((ext) => ext.toUpperCase()),
);
return extensions.map((ext) => `${file}${ext}`).find((fileExt) => fs.existsSync(fileExt));
const matches = extensions
.map((ext) => `${file}${ext}`)
.filter((fileExt) => fs.existsSync(fileExt));
if (warnFn) {
const normalizedMatches: string[] = [];
matches.forEach((match) => {
if (!normalizedMatches.map((m) => m.toLowerCase()).includes(match.toLowerCase())) {
normalizedMatches.push(match);
}
});
if (normalizedMatches.length === 0 && path.extname(file)) {
warnFn(`Table of contents entry does not exist: ${file}`, 'error');
} else if (normalizedMatches.length === 0) {
warnFn(
`Unable to resolve table of contents entry: ${file}`,
'error',
`Expected one of:\n - ${VALID_FILE_EXTENSIONS.map((ext) => `${file}${ext}`).join('\n- ')}`,
);
} else if (normalizedMatches.length === 1) {
warnFn(
`Extension inferred for table of contents entry: ${file}`,
'warn',
`To ensure consistent behavior, update the toc entry to ${matches[0]}`,
);
} else {
warnFn(
`Multiple files match table of contents entry: ${file}`,
'error',
`Valid files include: ${normalizedMatches[0]} (currently used in the build), ${normalizedMatches.slice(1).join(', ')}\n Update the toc entry to include the correct extension to ensure consistent behavior.`,
);
}
}
return matches[0];
}

0 comments on commit a665cb1

Please sign in to comment.