diff --git a/CHANGELOG.md b/CHANGELOG.md index 5f050fc..47f4e0f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,12 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), ## Unreleased +### Fixed + +- Server no longer crashes when saving files in nvim + + A bug in the file modification watcher caused the change list to include temporary (non-existent) files when saving with certain editors, such as vim. This is now fixed. + ### Changed - Improve logging diff --git a/core/api.ts b/core/api.ts index f16a71d..7e6bf8d 100644 --- a/core/api.ts +++ b/core/api.ts @@ -52,7 +52,7 @@ import getParser, { stringifyPageContent } from "./parser.ts"; import render from "./renderer.ts"; import getLayoutLoader from "./layout-loader.ts"; import getWalkEntryProcessor from "./walk-entry-processor.ts"; -import { createInputPathsGetter, sanitizeChangesFilter } from "./changes.ts"; +import { createInputPathsGetter, removeDuplicateChanges } from "./changes.ts"; import { getFilesystemChanges } from "./changes-fs.ts"; type ContentGetterCreator = (options: BuildOptions) => ContentGetter; @@ -271,8 +271,10 @@ export const createChangesApplier = (options: BuildOptions): ChangesApplier => { const allChanges = (await Promise.all(changes.map(mapChangeToDependantChanges))) .flat() - .filter(sanitizeChangesFilter); - log?.info(`Applying ${allChanges.length} changes in total including dependants...`); + .filter(removeDuplicateChanges); + log?.info( + `Applying ${allChanges.length} changes in total including dependants...`, + ); log?.debug(JSON.stringify(allChanges)); const startTime = Date.now(); for (const change of allChanges) { diff --git a/core/change-providers/fs-mod.test.ts b/core/change-providers/fs-mod.test.ts new file mode 100644 index 0000000..b8532d2 --- /dev/null +++ b/core/change-providers/fs-mod.test.ts @@ -0,0 +1,104 @@ +import { assertEquals } from "../../deps.ts"; +import { _changesFromFsEvents } from "./fs-mod.ts"; + +Deno.test("basic nvim modification works", () => { + const modifications: Deno.FsEvent[] = [{ + "kind": "create", + "paths": ["content/4913"], + "flag": undefined, + }, { + "kind": "modify", + "paths": ["content/4913"], + "flag": undefined, + }, { + "kind": "access", + "paths": ["content/4913"], + "flag": undefined, + }, { + "kind": "remove", + "paths": ["content/4913"], + "flag": undefined, + }, { + "kind": "modify", + "paths": ["content/index.md"], + "flag": undefined, + }, { + "kind": "modify", + "paths": ["content/index.md~"], + "flag": undefined, + }, { + "kind": "modify", + "paths": [ + "content/index.md", + "content/index.md~", + ], + "flag": undefined, + }, { + "kind": "create", + "paths": ["content/index.md"], + "flag": undefined, + }, { + "kind": "modify", + "paths": ["content/index.md"], + "flag": undefined, + }, { + "kind": "access", + "paths": ["content/index.md"], + "flag": undefined, + }, { + "kind": "modify", + "paths": ["content/index.md"], + "flag": undefined, + }, { + "kind": "remove", + "paths": ["content/index.md~"], + "flag": undefined, + }]; + + const changes = _changesFromFsEvents()(modifications); + assertEquals(changes, [{ type: "modify", inputPath: "content/index.md" }]); +}); + +Deno.test("touching file results in single create", () => { + const modifications: Deno.FsEvent[] = [{ + "kind": "create", + "paths": ["content/index.md"], + "flag": undefined, + }, { + "kind": "modify", + "paths": ["content/index.md"], + "flag": undefined, + }, { + "kind": "access", + "paths": ["content/index.md"], + "flag": undefined, + }]; + + const changes = _changesFromFsEvents()(modifications); + assertEquals(changes, [{ type: "create", inputPath: "content/index.md" }]); +}); + +Deno.test("moving file results in delete and create", () => { + const modifications: Deno.FsEvent[] = [{ + "kind": "modify", + "paths": ["content/test"], + "flag": undefined, + }, { + "kind": "modify", + "paths": ["content/test2"], + "flag": undefined, + }, { + "kind": "modify", + "paths": [ + "content/test", + "content/test2", + ], + "flag": undefined, + }]; + + const changes = _changesFromFsEvents()(modifications); + assertEquals(changes, [ + { type: "delete", inputPath: "content/test" }, + { type: "create", inputPath: "content/test2" }, + ]); +}); diff --git a/core/change-providers/fs-mod.ts b/core/change-providers/fs-mod.ts index 9c71a55..8247820 100644 --- a/core/change-providers/fs-mod.ts +++ b/core/change-providers/fs-mod.ts @@ -1,6 +1,13 @@ import type { BuildOptions, Change } from "../../domain.ts"; import { debounce, path } from "./../../deps.ts"; -import { sanitizeChangesFilter } from "../changes.ts"; +import { removeDuplicateChanges } from "../changes.ts"; + +type ChangeArrayFn = ( + change: Change, + i: number, + changes: Change[], +) => R; +type ChangeArrayFilter = ChangeArrayFn; export default async function changerFileModifications( buildOptions: BuildOptions, @@ -15,7 +22,7 @@ export default async function changerFileModifications( buildOptions.log?.warning("Not updating on layout changes"); const pendingEvents: Deno.FsEvent[] = []; - const getChanges = changesFromFsEvents(buildOptions); + const getChanges = _changesFromFsEvents(buildOptions); const applyEvents = debounce( (events: Deno.FsEvent[]) => applyChanges(getChanges(events)), 100, @@ -29,20 +36,6 @@ export default async function changerFileModifications( } } -/** Set path relative to CWD */ -const _setPathToCwdRelative = ( - change: Change, -): Change => - "inputPath" in change - ? { - ...change, - inputPath: path.relative(Deno.cwd(), change.inputPath), - } - : { - ...change, - outputPath: path.relative(Deno.cwd(), change.outputPath), - }; - /** Create one or many changes based on a filesystem event */ const _eventToChange = (event: Deno.FsEvent): Change | Change[] => { // turn one file modification to modify @@ -80,17 +73,101 @@ const _eventToChange = (event: Deno.FsEvent): Change | Change[] => { throw new Error(`Could not process event ${JSON.stringify(event)}`); }; -const changesFromFsEvents = ({ log }: BuildOptions) => +/** Filter out filest that were first created then immediately removed */ +const _removeTempFiles: ChangeArrayFilter = (change, i, changes) => { + // remove anything followed by delete + if (change.type !== "delete" && change.type !== "orphan") { + const lastDeleteThisFile = changes.findLastIndex((c) => + c.type === "delete" && c.inputPath === change.inputPath + ); + if (lastDeleteThisFile > i) { + return false; + } + } + // remove delete when preceded by create + if (change.type === "delete") { + const firstCreateThisFile = changes.findIndex((c) => + c.type === "create" && c.inputPath === change.inputPath + ); + if (firstCreateThisFile >= 0 && firstCreateThisFile < i) { + return false; + } + } + return true; +}; + +/** Turn delete followed by create into a single modify using reducer */ +const _processFileRefresh = ( + newChanges: Change[], + change: Change, + i: number, + changes: Change[], +): Change[] => { + // do not append delete followed by create + if ( + change.type === "delete" && + changes.findLastIndex((c) => + c.type === "create" && c.inputPath === change.inputPath + ) > i + ) { + return newChanges; + } + // turn create preceded by delete into modify + if (change.type === "create") { + const firstDeleteThisFile = changes.findIndex((c) => + c.type === "delete" && c.inputPath === change.inputPath + ); + if (firstDeleteThisFile >= 0 && firstDeleteThisFile < i) { + return newChanges.concat({ ...change, type: "modify" }); + } + } + return newChanges.concat(change); +}; + +/** Remove modifications of created and deleted files */ +const _removeModificationsWhenCreateOrDelete: ChangeArrayFilter = ( + change, + _i, + changes, +) => { + if ( + change.type === "modify" && + changes.findIndex((c) => + (c.type === "create" || c.type === "delete") && + c.inputPath === change.inputPath + ) >= 0 + ) { + return false; + } + return true; +}; + +/** Set path relative to CWD */ +const _setPathToCwdRelative = (change: Change): Change => + "inputPath" in change + ? { + ...change, + inputPath: path.relative(Deno.cwd(), change.inputPath), + } + : { + ...change, + outputPath: path.relative(Deno.cwd(), change.outputPath), + }; + +export const _changesFromFsEvents = (options?: BuildOptions) => (events: Deno.FsEvent[]): Change[] => { - log?.debug(`Creating changes from fs events ${JSON.stringify(events)}`); + options?.log?.debug( + `Creating changes from fs events ${JSON.stringify(events)}`, + ); const changes: Change[] = events - // filter out access events - .filter((event) => event.kind !== "access") + .filter((event) => event.kind !== "access") // filter out access events .map(_eventToChange) - // above may create nested arrays, so flatten - .flat() - .map(_setPathToCwdRelative) - .filter(sanitizeChangesFilter); + .flat() // above may create nested arrays, so flatten + .filter(_removeTempFiles) + .reduce(_processFileRefresh, []) + .filter(_removeModificationsWhenCreateOrDelete) + .filter(removeDuplicateChanges) + .map(_setPathToCwdRelative); // set paths to cwd-relative // clear array after transforming to changes events.splice(0); return changes; diff --git a/core/changes.test.ts b/core/changes.test.ts index 3e48fc3..281023b 100644 --- a/core/changes.test.ts +++ b/core/changes.test.ts @@ -1,10 +1,8 @@ import type { Change } from "../domain.ts"; import { assertEquals } from "../deps.ts"; import { - _removeDuplicateChanges, - _removeModifiedWhenAlsoCreatedOrDeleted, + removeDuplicateChanges, createInputPathsGetter, - sanitizeChangesFilter, } from "./changes.ts"; Deno.test("content file from public file base case", () => { @@ -36,7 +34,7 @@ Deno.test("content file from public file base case", () => { ); }); -Deno.test("duplicates work", () => { +Deno.test("removing duplicates work", () => { const hasDuplicates: Change[] = [ { type: "orphan", outputPath: "public/orphan/index.html" }, { type: "orphan", outputPath: "public/orphan-2/index.html" }, @@ -46,7 +44,7 @@ Deno.test("duplicates work", () => { { type: "create", inputPath: "content/created.md" }, { type: "create", inputPath: "content/created.md" }, ]; - const dupsRemoved = hasDuplicates.filter(_removeDuplicateChanges); + const dupsRemoved = hasDuplicates.filter(removeDuplicateChanges); assertEquals(dupsRemoved, [ { type: "orphan", outputPath: "public/orphan/index.html" }, { type: "orphan", outputPath: "public/orphan-2/index.html" }, @@ -61,25 +59,6 @@ Deno.test("single change does not get sanitized", () => { type: "orphan", outputPath: "public/testing/index.html", }]; - const dupsRemoved = changes.filter(_removeDuplicateChanges); + const dupsRemoved = changes.filter(removeDuplicateChanges); assertEquals(dupsRemoved, changes); - const modsRemoved = changes.filter(_removeModifiedWhenAlsoCreatedOrDeleted); - assertEquals(modsRemoved, changes); - const sanitized = changes.filter(sanitizeChangesFilter); - assertEquals(sanitized, changes); -}); - -Deno.test("remove modifications for created or deleted", () => { - const hasDuplicates: Change[] = [ - { type: "orphan", outputPath: "public/orphan/index.html" }, - { type: "create", inputPath: "content/modified.md" }, - { type: "modify", inputPath: "content/modified.md" }, - { type: "create", inputPath: "content/created.md" }, - ]; - const dupsRemoved = hasDuplicates.filter(_removeModifiedWhenAlsoCreatedOrDeleted); - assertEquals(dupsRemoved, [ - { type: "orphan", outputPath: "public/orphan/index.html" }, - { type: "create", inputPath: "content/modified.md" }, - { type: "create", inputPath: "content/created.md" }, - ]); }); diff --git a/core/changes.ts b/core/changes.ts index 1873b7c..c64f674 100644 --- a/core/changes.ts +++ b/core/changes.ts @@ -2,7 +2,7 @@ import { path } from "../deps.ts"; import type { Change } from "../domain.ts"; /** Filter duplicates (inefficient algo, but we generally only have a few items in the array) */ -export const _removeDuplicateChanges = ( +export const removeDuplicateChanges = ( change: Change, index: number, changes: Change[], @@ -14,28 +14,6 @@ export const _removeDuplicateChanges = ( c.inputPath === change.inputPath) )) === index; -/** Filter modifications for deleted and created files */ -export const _removeModifiedWhenAlsoCreatedOrDeleted = ( - change: Change, - _index: number, - changes: Change[], -): boolean => - change.type !== "modify" || - changes.findIndex( - (c) => ( - (c.type === "delete" || c.type === "create") && - c.inputPath === change.inputPath - ), - ) < 0; - -/** Filter for `Change[]` that removes unnecessary changes */ -export const sanitizeChangesFilter = ( - change: Change, - index: number, - changes: Change[], -): boolean => - _removeDuplicateChanges(change, index, changes) && - _removeModifiedWhenAlsoCreatedOrDeleted(change, index, changes); /** Get input paths which might have resulted in a specific public (output) path */ export const createInputPathsGetter = ( diff --git a/mod.ts b/mod.ts index 09bf387..521d487 100644 --- a/mod.ts +++ b/mod.ts @@ -26,7 +26,6 @@ export type { ContentImporter, ImportedContent, Page, - Props, } from "./domain.ts"; export type { EdgeComponent } from "./edge/mod.ts"; export type { @@ -42,3 +41,10 @@ export * from "./utils/get-path.ts"; export * from "./utils/read-contents.ts"; export * from "./utils/sort-date.ts"; export * from "./utils/sort-weight.ts"; + +/** + * @deprecated + * + * This type is deprecated and should not be used. + */ +export type Props = Record;