fix: Fix server crash when saving in nvim

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

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) {

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" },
+  ]);
+});

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<R> = (
+  change: Change,
+  i: number,
+  changes: Change[],
+) => R;
+type ChangeArrayFilter = ChangeArrayFn<boolean>;
 
 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<Change[]>(_processFileRefresh, [])
+      .filter(_removeModificationsWhenCreateOrDelete)
+      .filter(removeDuplicateChanges)
+      .map(_setPathToCwdRelative); // set paths to cwd-relative
     // clear array after transforming to changes
     events.splice(0);
     return changes;

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" },
-  ]);
 });

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 = (